Skip to content
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 custom_hero_guts template #12925

Merged
merged 4 commits into from
Sep 30, 2024

Conversation

dlopezvsr
Copy link
Collaborator

@dlopezvsr dlopezvsr commented Sep 25, 2024

Description

This PR removes .localized from custom_hero_guts.html template, reducing the number of .localized attributes in the codebase, and still keeping the entry in self.breadcrumb_list translated and displayed properly.

How to test

  • Pull and checkout this branch into your local env, and start app.
  • Go to the CMS > open a publication page that contains Article pages where the breadcrumb can be shown once the article is opened.
  • Open the Article page and change the locale slug from "en" to any other locale available.
  • Make sure the breadcrumb description is still displayed properly and translated accordingly.

Caveats:

In case there is not any current translation, the parent page url name will remain the same. So you can go to the CMS and add the translation to test.

image

Link to sample test page: https://foundation-s-tp1-101-re-3vx1kx.herokuapp.com/en/
Related PRs/issues: #10008

Checklist

Tests

  • Is the code I'm adding covered by tests?

Changes in Models:

  • Did I update or add new fake data?
  • Did I squash my migration?
  • Are my changes backward-compatible. If not, did I schedule a deploy with the rest of the team?

Documentation:

  • Is my code documented?
  • Did I update the READMEs or wagtail documentation?

Merge Method
💡❗Remember to use squash merge when merging non-feature branches into main

@dlopezvsr dlopezvsr self-assigned this Sep 25, 2024
@dlopezvsr dlopezvsr temporarily deployed to foundation-s-tp1-101-re-vtodms September 25, 2024 20:11 Inactive
@dlopezvsr dlopezvsr temporarily deployed to foundation-s-tp1-101-re-3vx1kx September 25, 2024 20:26 Inactive
@dlopezvsr dlopezvsr marked this pull request as ready for review September 25, 2024 20:44
@dlopezvsr dlopezvsr temporarily deployed to foundation-s-tp1-101-re-itfpvd September 25, 2024 20:45 Inactive
Copy link
Collaborator

@mmmavis mmmavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks @dlopezvsr !

@dlopezvsr dlopezvsr temporarily deployed to foundation-s-tp1-101-re-kbe9l6 September 30, 2024 15:32 Inactive
@dlopezvsr dlopezvsr merged commit c9851bb into main Sep 30, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants