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

USWDS - Pages: Avoid reordering content in Documentation example #5794

Merged
merged 7 commits into from Mar 6, 2024

Conversation

mahoneycm
Copy link
Contributor

@mahoneycm mahoneycm commented Mar 1, 2024

Summary

Added mobile sidenav to the Documentation page template. Sidenavs will now always follow logical HTML order on mobile and desktop views. Both navs use utility classes to change display settings based on viewport width.

Breaking change

⚠️ This is potentially breaking change.

Workaround

Include a duplicate sidenav after main content. Visibility is toggled using utility classes to avoid accessibility issues related to tab order and users of assistive technologies.

Recommended workaround

<div class="grid-container">
  <div class="grid-row grid-gap">
-   <div class="usa-layout-docs__sidenav">
+   <!-- One of two sidenav's only shown in desktop breakpoints. --> 
+   <div class="usa-layout-docs__sidenav display-none desktop:display-block desktop:grid-col-3">
      {{ SIDENAV_MARKUP }}
    </div>
-   <main class="usa-layout-docs__main desktop:grid-col-9 usa-prose usa-layout-docs" id="main-content">
+   <main class="desktop:grid-col-9 usa-prose usa-layout-docs" id="main-content">
      {{ MAIN_CONTENT }}
    </main>
  </div>
+ <!-- New duplicate section only shown on mobile. -->
+  <div class="usa-layout-docs__sidenav desktop:display-none">
+    {{ SIDENAV_MARKUP }}
+  </div>
</div>

Changed HTML

This solution effectively duplicates the nav HTML and displays only the correctly placed in-page nav.

While this doesn’t affect the in-page nav component, we do offer this page as a template and the component code is changed as a result of this PR. // Need to verify this causes a change

Removed template classes:

  • usa-layout-docs__main: This is the class that had the styles that reordered element.
  • usa-layout-docs: This selector unused in current styles.

Related issue

Closes #4594

Related pull requests

Alternate approach as discussed in #5681
Changelog → (Taken from #5681)

Preview link

Current (develop) Feature branch
Default Preview
Template (Site) Feature branch template

Problem statement

In mobile, tab order doesn't match visual order in the documentation page example.

Solution

Create a second, mobile only in-page nav. Use utility classes to dictate which is displayed.

Testing and review

Visit Documentation page →

In desktop widths

  1. Tab through navigation
  2. Confirm tab order is correct
  3. Confirm screen readers read nav before main content
  4. Confirm second nav is hidden and not tabbed to or read by screen readers

In mobile widths

  1. Confirm nav is still at the bottom of the page
  2. Confirm tab order is correct
  3. Confirm screen readers read main content before nav
  4. Confirm original nav is hidden and not tabbed to or read by screen readers

Testing checklist

  • No visual regressions
  • Hidden navs are not targetted by tabbing
  • Hidden navs are not read by screen readers
  • Screen readers read content in visual order
  • Tab order follows matches expectation.

Copy link
Contributor

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

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

This is looking good. Added a suggestion to remove duplicate code. The suggestion will likely need to be tweaked and tested, but wanted to give you a starting point to let you know what I'm thinking. Let me know if you have questions!

@mahoneycm
Copy link
Contributor Author

mahoneycm commented Mar 1, 2024

Update 3/1/24

Loved @amyleadem's suggestion to implement the side-nav via include. Testing didn't show any regression and the component markup output as expected. Tested via install on site.

@mahoneycm mahoneycm requested a review from amyleadem March 1, 2024 20:57
Copy link
Contributor

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

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

This is looking good! Just a couple small tweaks to the data, detailed in the comments below.

Copy link
Contributor

@amyleadem amyleadem 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!

I tested the following:

  • Confirm no visual regressions from develop
  • Confirm that the HTML order matches the visual order
  • Confirm that the code meets standards

Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

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

Thanks for helping out with this. Small comment for clarity, otherwise LGTM.

Co-authored-by: James Mejia <james.mejia@gsa.gov>
@mejiaj mejiaj requested a review from thisisdano March 4, 2024 15:25
@mejiaj
Copy link
Contributor

mejiaj commented Mar 4, 2024

@mahoneycm I tweaked the summary to avoid any confusion with USA In-page navigation.

Copy link
Member

@thisisdano thisisdano left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@thisisdano thisisdano merged commit 49973df into develop Mar 6, 2024
5 checks passed
@thisisdano thisisdano deleted the cm-feature-doc-layout-order branch March 6, 2024 20:52
@thisisdano thisisdano mentioned this pull request Mar 11, 2024
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.

Documentation template tab order should match visual order
4 participants