add tooltips to editor tabs #456
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
Screen.Recording.2024-08-07.at.12.43.59.movsmall change to close tooltip on tab click incoming shortly |
|
I have read the CLA Document and I hereby sign the CLA |
|
I don't love that I am manually setting the "openTabKey" to the first item in the array on load but have not found a smarter way to determine whats currently open. Open to ideas! |
| title: string | ||
| children: React.ReactNode | ||
| } | ||
| export const IconWrapper = ({ tabKey, openTabKey, title, children }: IconWrapperProps): React.JSX.Element => { |
There was a problem hiding this comment.
Can we also add the tooltip, when the Tab has focus via keyboard? :)
Just want make sure that we meet the a11y requirements, if possible.
There was a problem hiding this comment.
Screen.Recording.2024-08-08.at.16.46.22.mov
There was a problem hiding this comment.
As I am not able to access each tab element separately (because of Tabs.TabPane soon being deprecated)
I have made do with the onFocus/ onBlur combined with some event.target.id text matching.
When someone is using the Keyboard to tab through the tabs and then switches to navigate via mouse, two tooltips can show at once which I think makes sense (see vid).
There was a problem hiding this comment.
Yes, I'd agree that it makes sense to show 2 tooltips in that case...
|
|
||
| const onTabClick = (key: string): void => { |
There was a problem hiding this comment.
Does this also work with the keyboard? Otherwise we maybe could use the onChange listener instead?
There was a problem hiding this comment.
I had tested it and it also work for keyboard navigation. I do think the onChange handler seems more fitting though. Changed, Thanks!
| @@ -30,11 +31,27 @@ export interface IEditorTabsProps { | |||
| showLabelIfActive?: boolean | |||
| } | |||
|
|
|||
| export interface IconWrapperProps { | |||
There was a problem hiding this comment.
I think this interface can be removed? Seems like it was moved to the tooltip component already? :)
|
|
@xIrusux, very nice first PR 🥇 nice to have you in the team 🥳 🎉 |
Changes in this pull request
Add tooltips to editor tabs
Resolves #435
Additional info
Resorted to writing a custom icon wrapper to add and remove the "open" property on hover.
Unfortunately Antd does not have an out of the box tooltip solution for the way we use their Tabs component (i.e. we remove the label text on click).
I explored the option to add the tooltip on Tabs.TabPane but later read that it's (soon to be?) deprecated.