-
-
Notifications
You must be signed in to change notification settings - Fork 182
feat(help): apply updated design to Webhooks Start page #5855
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(help): apply updated design to Webhooks Start page #5855
Conversation
Applied the new visual design to the Webhooks Getting Started page (`v2_webhooks-getting-started.html`) Refs: freelawproject#5353
Adds an attention-grabbing comment to the old version of the template.
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, thanks! However, as with the previous review, this one also doesn't show even when the flag is active. This will probably require the same fix as that one, since both handle template name the same way.
<li>Your application should use the <a class="underline" href="{% url 'webhooks_docs' %}#headers">Idempotency-Key included in the event headers</a> to ensure that it only processes the event once.</li> | ||
</ul> | ||
<p>Below are examples of endpoints configured in Flask and Django</p> | ||
<p><strong>Flask</strong></p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think <h3>
is more semantically correct here than <p><strong>
:
<p><strong>Flask</strong></p> | |
<h3>Flask</h3> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that makes sense, I'll change it.
print(request.json) | ||
return Response(status=200) | ||
</c-code> | ||
<p><strong>Django</strong></p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here:
<p><strong>Django</strong></p> | |
<h3>Django</h3> |
I change middleware to fix it, but to be honest I don't know where to commit, in every branch, or make a PR for this change, because it should be in each PR |
Replaced a <p> with a nested <strong> tag with a more semantically correct <h3> heading.
This sounds great! Could you open a PR to update the middleware to support these views please? Then we can merge that first, and the rest should work :) |
Applied the new visual design to the Webhooks Getting Started page (
v2_webhooks-getting-started.html
)Screenshot of updated design:
Refs: #5353