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

Inert synchronization issue #2431

Merged
merged 5 commits into from
Sep 20, 2024
Merged

Conversation

DingoEatingFuzz
Copy link
Contributor

@DingoEatingFuzz DingoEatingFuzz commented Sep 19, 2024

πŸ“Œ Summary

If merged this PR will fix two bugs related to the inert state of Hds::SideNav when the viewport changes sizes.

πŸ› οΈ Detailed description

Right now we toggle the inert state of certain elements when the menu is manually minimized or expanded. However, there are two other ways the menu can be minimized/expanded:

  1. Via mobile/desktop breakpoint: If the menu is open and the viewport shrinks, the nav minimized and the inert attributes are not applied. Similarly, if the nav is mimized and the viewport expands, the inert attributes remain.
  2. Via ESC when in mobile mode: If the menu is open while the viewport is mobile, the menu shrinks, but the inert attributes are not applied.

This is simple enough to test today in production:

  1. Go to the HCP Portal or to HCP Terraform
  2. Starting from a desktop viewport:
    1. Toggle the menu closed
    2. Shrink the viewport to mobile
    3. Expand the viewport back to desktop
  3. Observe that the nav automatically expanded, but it is not interactive

πŸ“Έ Screenshots

πŸ”— External links

Jira ticket: HDS-XXX
Figma file: [if it applies]


πŸ‘€ Component checklist

πŸ’¬ Please consider using conventional comments when reviewing this PR.

Copy link

vercel bot commented Sep 19, 2024

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Updated (UTC)
hds-showcase βœ… Ready (Inspect) Visit Preview Sep 20, 2024 2:52pm
hds-website βœ… Ready (Inspect) Visit Preview Sep 20, 2024 2:52pm

Copy link
Member

@alex-ju alex-ju left a comment

Choose a reason for hiding this comment

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

Thanks a lot for contributing this fix! πŸ™Œ
It now works as expected; I left a few nit comments, but otherwise looking great and good to be merged

.changeset/tricky-trains-ring.md Outdated Show resolved Hide resolved
packages/components/src/components/hds/side-nav/index.ts Outdated Show resolved Hide resolved
@DingoEatingFuzz
Copy link
Contributor Author

Thanks for the review @alex-ju! I believe I addressed everything. Sorry about the lingering console.logs and very good to know about property/method/action ordering.

@alex-ju
Copy link
Member

alex-ju commented Sep 20, 2024

very good to know about property/method/action ordering.

to be fair, we're due to update our contribution guidelines, so I'll make sure to include such things

@DingoEatingFuzz DingoEatingFuzz merged commit 52e6916 into main Sep 20, 2024
14 checks passed
@DingoEatingFuzz DingoEatingFuzz deleted the bugfix/inert-synchronization-issue branch September 20, 2024 15:03
@hashibot-hds hashibot-hds mentioned this pull request Sep 20, 2024
@Dhaulagiri
Copy link
Contributor

πŸ™‡ thank you @DingoEatingFuzz!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants