-
Notifications
You must be signed in to change notification settings - Fork 54
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
Add hover state to Hierarchy toggle background and hover + refactor #915
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like the hover state color persists when the list is expanded but the mouse is off of the toggle. I would expect the hover background color to only be there when the mouse is actually hovering on the icon.
What do you think?
Yes, this is because I added the styles to the active state too. I modified it and now it only applies to hover states. Thanks for catching this up! |
32b8fad
to
00396f0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
It looks like this only adds the hover state to the actual toggle and none of the links within the expanded menu when you hover over them after using that toggle. Should we also add something for that? |
Bug/issue #135891984, if applicable:
Summary
Add hover state to Hierarchy toggle background and border.
Color specs for the toggle in hover state:
figure-blue
Also, I realised that we still had color styles for the nav bar in dark mode that was no longer used since we moved the breadcrumbs into the Hero at #772, so I deleted those styles in a second commit.
Dependencies
NA
Testing
Steps:
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
npm test
, and it succeeded