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

Secondary Nav: refactor into new design #8011

Merged
merged 8 commits into from
Nov 8, 2023

Conversation

anselmbradford
Copy link
Member

Changes

  • Change translations from "In this section" to "Navigate this section"
  • Refactor secondary nav to latest designs (see screenshots).

How to test this PR

  1. Visit some pages with a secondary navigation, such as

http://localhost:8000/data-research/small-business-lending/filing-instructions-guide/2024-guide/
http://localhost:8000/rules-policy/regulations/1002/
http://localhost:8000/paying-for-college/your-financial-path-to-graduation/
http://localhost:8000/enforcement/payments-harmed-consumers/
http://localhost:8000/es/herramientas-del-consumidor/prestamos-para-vehiculos/respuestas/conozca-sus-derechos/

Resize to mobile. These pages should have a new secondary nav appearance (see screenshots).

Screenshots

Before:
Screenshot 2023-11-07 at 10 10 17 AM

After:
Screenshot 2023-11-07 at 10 10 01 AM

@jenn-franklin
Copy link
Member

Exciting! One small thing I'm seeing in the above screenshot is that there's a slash before the parent link "Strategy, Budget and Performance." On Beta—on a Coronavirus page for example—we don't include a slash before the first link of the path.

Screenshot 2023-11-07 at 4 14 41 PM

I'd say either approach is technically correct. I wasn't thinking we'd include the first slash but then again it does indicate that there's something (Home) beyond the breadcrumb showing, and I guess the chevron that's currently on cf.gov currently fills that same purpose. I glanced at what USWDS does, and they actually include a "Home" link in every breadcrumb. So, I'm honestly a little torn on if we should include it or not. The main thing I'm feeling is that there should be consistency in approach across pages.

@anselmbradford anselmbradford force-pushed the ans_breadcrumb_nav_beta_demo4 branch from 7385422 to 3e51d43 Compare November 7, 2023 21:29
@anselmbradford
Copy link
Member Author

@chosak @willbarton Do you have guidance on what is happening with the breadcrumbs on pages that have the caret at the beginning? http://localhost:8000/es/coronavirus/maneje-sus-finanzas/ is another page with the leading slash (caret in the old design).


{% for crumb in breadcrumbs %}

{% if loop.index == loop.length %}
Copy link
Member

Choose a reason for hiding this comment

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

@anselmbradford in response to your question about the leading slash: in Jinja for syntax the loop.index variable is 1-based, not 0-based. So if the length of the breadcrumbs is 1, this will render the divider the first time through the loop. See docs here, you can use loop.index0 instead.

I'm a bit unclear from your comments above with @jenn-franklin -- what is the desired behavior? If you don't want to render the slash if there's only one parent breadcrumb, you could remove this clause, but will it be obvious that this is a breadcrumb then?

Copy link
Member

Choose a reason for hiding this comment

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

We didn't include the first slash during usability testing, and participants didn't struggle to understand that it was a breadcrumb, so that's good! But... ugh... I'm waffling. Haha. I guess ultimately I may lean a little more toward including the first slash.

Copy link
Member Author

Choose a reason for hiding this comment

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

As is, http://localhost:8000/es/herramientas-del-consumidor/prestamos-para-vehiculos/respuestas/conozca-sus-derechos/ would not have a leading slash, while http://localhost:8000/es/coronavirus/maneje-sus-finanzas/ would. I guess we either always want the slash or always not have it?

Copy link
Member

Choose a reason for hiding this comment

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

Yah, I'd say always slash or never slash.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yah, I'd say always slash or never slash.

Which one though?

Copy link
Member

Choose a reason for hiding this comment

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

If it's helpful, Jinja has a random filter if we want to show slashes to some visitors but not others. Keep the cf.gov experience dynamic and interesting!

Copy link
Member

Choose a reason for hiding this comment

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

Let's go with slash!

@anselmbradford anselmbradford added this pull request to the merge queue Nov 8, 2023
Merged via the queue into main with commit 225e2ad Nov 8, 2023
15 checks passed
@anselmbradford anselmbradford deleted the ans_breadcrumb_nav_beta_demo4 branch November 8, 2023 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants