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

Fix on menu highlights #6034

Merged
merged 3 commits into from
Feb 2, 2023
Merged

Fix on menu highlights #6034

merged 3 commits into from
Feb 2, 2023

Conversation

sfc-gh-jgarcia
Copy link
Contributor

@sfc-gh-jgarcia sfc-gh-jgarcia commented Jan 30, 2023

📚 Context

This PR fixes an issue reported by @tvst, where it’s easy to get to a state where there’s a menu item highlighted in both the viewer menu and the dev menu at the same time. We believe this issue could've been introduced recently, after we upgraded BaseWeb.

  • What kind of change does this PR introduce?

    • Bugfix
    • Feature
    • Refactoring
    • Other, please describe:

🧠 Description of Changes

  • Updated backgroundColor styles to apply on :hover for the <StyledMenuItem> and <StyledDevItem> components;

    • This is a breaking API change
    • This is a visible (user-facing) change

Revised:

Kapture.2023-01-30.at.17.29.49.mp4

Current:

Screen.Recording.2023-01-25.at.13.57.04.mov

🧪 Testing Done

Note
Please note that, since highlights now work on hover, updated e2e tests don't include them, as they're rather complex to mimic with Cypress.

  • Screenshots included
  • Added/Updated unit tests
  • Added/Updated e2e tests

🌐 References


Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@sfc-gh-jgarcia sfc-gh-jgarcia marked this pull request as ready for review January 31, 2023 15:57
Copy link
Contributor

@sfc-gh-tszerszen sfc-gh-tszerszen 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, now the behaviour is much more consistent 🥇

@sfc-gh-tszerszen sfc-gh-tszerszen merged commit 097a104 into develop Feb 2, 2023
@vdonato vdonato deleted the fix-menu-highlights branch November 2, 2023 00:02
eric-skydio pushed a commit to eric-skydio/streamlit that referenced this pull request Dec 20, 2023
zyxue pushed a commit to zyxue/streamlit that referenced this pull request Mar 22, 2024
zyxue pushed a commit to zyxue/streamlit that referenced this pull request Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants