Skip to content

Conversation

mahoneycm
Copy link
Contributor

@mahoneycm mahoneycm commented Nov 15, 2023

Summary

Resolved a compilation error when $theme-max-header-width was set to none.

Breaking change

This is not a breaking change

Related issue

Closes #5579

Related pull requests

Changelog →

USWDS: Header: Add fix for Safari-only header bug #5443

Preview link

Header →

Documentation page →

Problem statement

A compilation error occurs when setting $theme-max-header-width to none.

This is a regression which stems from our Safari header navigation bug fix #5543. The calc() method cannot take a value of none so it breaks the calculation.

Solution

Use sass @if at-rule to check if $theme-max-header-width value is none before enabling the Safari bug fix logic.

  • Since there is no max width, the Safari header bug does not occur
  • Therefore, we can bypass the additional logic when max-width is set to none

Testing and review

Test compilation error

  1. Set $theme-header-max-width to "none"
  2. Ensure there are no compilation errors

Safari header bug

Follow Amy’s testing steps from #5543 to ensure there are no regressions:

Use Storybook to test that the mobile menu opens in Safari:

  1. Open the Documentation page in Safari
  2. Resize the window to somewhere between 1009px-1024px
  3. Click the "Menu" button to open the mobile navigation panel
  4. Confirm that the mobile header successfully opens as expected

Use Storybook to test that this does not affect other browsers:

  1. Open the Documentation page in other browsers
  2. Resize the window to somewhere between 1009px-1024px
  3. Click the "Menu" button to open the mobile navigation panel
  4. Confirm that the main page content is not scrollable

Testing checklist

  • Confirm there are no compilation errors when $theme-header-max-width is set to "none".
  • Confirm use of @if is appropriate in this situation.
  • Confirm there are no regressions with header in Safari after this change.

@amyleadem
Copy link
Contributor

Closing this PR in favor of the fix in #5624

@amyleadem amyleadem closed this Nov 17, 2023
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.

USWDS - Bug: Setting $theme-header-max-width: "none" causes error when building usa-nav styles
2 participants