Desktop: Resolves #12292: Add hover + expanded arrow behavior for Notebook/Tags header#13190
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
Thank you for contributing! For reference, the failing CI check seems unrelated to this pull request: CC @pedr |
|
Hello, we need screenshots or video for this |
hover-effect.movHere is a quick demo of the new hover and expanded arrow behavior for the Notebook/Tags header. Please let me know if you need any further adjustments! thanks |
laurent22
left a comment
There was a problem hiding this comment.
Thank you for clarifying and providing the video, your change makes a lot of sense. I've left some comments in the PR and we can merge once they are fixed
| onMouseEnter={()=>setIsHovered(true)} | ||
| onMouseLeave={()=>setIsHovered(false)} |
There was a problem hiding this comment.
Generally event handlers should be defined using useCallback to avoid uncontrolled re-rendering issues.
| cursor: default; | ||
| transition: background 0.2s; | ||
| &:hover{ | ||
| background: ${(props: StyleProps) => props.theme.backgroundColorHover2}; |
|
Hi Laurent, I’ve pushed the updates you suggested — added useCallback for the hover handlers and fixed the indentation. The checks are running now. Please let me know if you’d like any further adjustments. Thanks! |
|
That looks good now, thank you for adding this @maggie897! |
|
Not sure if this comment under the already merged PR will be noticed, but I think that in the current form, this has accessibility issues. The problem is with the text jumping left and right when you hover and unhover the headers. Ideally, only the icon should change, but the text position should remain in the same place. For myself, the jumping text causes physical dizziness. |


What does this PR do?
How to test
Notes
Related