-
Notifications
You must be signed in to change notification settings - Fork 153
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
Remove localized from partner html template and moved to get_context #12978
base: main
Are you sure you want to change the base?
Conversation
…n.mozilla.org into TP1-104-remove-localized-partner Merge
…/MozillaFoundation/foundation.mozilla.org into TP1-104-remove-localized-partner Merge
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.
Hi @dlopezvsr!
Thank you for taking on this work—everything is looking great and working as expected! 👍
I just have one minor suggestion before we merge, and then it should be good to go.
@@ -11,7 +11,7 @@ <h2 class="tw-h3-heading">{{ page.partner_heading }}</h2> | |||
{{ page.partner_intro_text }} | |||
</p> | |||
{% endif %} | |||
<a href="{{ page.partner_page.localized.url }}" class="text-white tw-cta-link font-weight-bold" id="partner-cta"> | |||
<a href="{{ localized_partner_page.url }}" class="text-white tw-cta-link font-weight-bold" id="partner-cta"> |
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.
<a href="{{ localized_partner_page.url }}" class="text-white tw-cta-link font-weight-bold" id="partner-cta"> | |
{% if localized_partner_page %} | |
<a href="{{ localized_partner_page.url }}" class="text-white tw-cta-link font-weight-bold" id="partner-cta"> | |
{% endif %} |
Hi @dlopezvsr, while I was testing this out, I found that even if the "partner_page" was not set, this <a>
element was still rendering. However, since there was no localized_partner_page
to grab a URL from, it was just linking back to localhost:8000
.
Can we please wrap this in a {% if %}
statement? That way, this button wont render unless the page is set.
Thanks!
if self.partner_page: | ||
context["localized_partner_page"] = self.partner_page.localized |
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.
Kudos Looks good to me. Great job!
Description
This PR removes
.localized
attribute frompartner.html
template as part of thelocalize
cleanup. This also moves the logic to page'sget_context
functioncontext["localized_partner_page"]
returning a localized version of the page.Link to sample test page:
Related PRs/issues: #10011
How to test
See images below.
Checklist
Tests
Changes in Models:
Documentation:
Merge Method
💡❗Remember to use squash merge when merging non-feature branches into
main
┆Issue is synchronized with this Jira Story