[DSCO] Adds a Keyboard Only Tooltip - #74018
Conversation
🖼️ Storybook Visual Comparison ReportA difference was found in our Storybook front-end visual comparison testing against the Remediation steps:
|
levadadenys
left a comment
levadadenys
left a comment
There was a problem hiding this comment.
This actually looks closer to what i mentioned in slack than what I though. (https://codeai.slack.com/archives/C0532LCK125/p1784704210063279?thread_ts=1784676197.629769&cid=C0532LCK125)
I still think long term we use Mui tooltip, but this also works. Few comments before we merge this though
| import Tooltip, {TooltipProps, WithKeyboardTooltip} from '../index'; | ||
|
|
||
| export default { | ||
| title: 'DesignSystem/KeyboardTooltip', |
There was a problem hiding this comment.
let's please call it DesignSystem/Tooltip/KeyboardTooltip (to highlight it's a part of existing tooltip, not a standalone component)
|
|
||
| import {WithKeyboardTooltip, TooltipProps} from './../index'; | ||
|
|
||
| describe('Design System - WithKeyboardTooltip', () => { |
There was a problem hiding this comment.
nit: let's call it Design System - Tooltip - WithKeyboardTooltip (to highlight it's a part of existing tooltip, not a standalone component)
| const TAIL_OFFSET = 4; | ||
| const TAIL_LENGTHS = {l: 12, m: 9, s: 6, xs: 6}; |
There was a problem hiding this comment.
this is a direct copy from WithTooltip, so we should be able to decalre these variables in one place and export them where needed instead of declaring same variables several times
There was a problem hiding this comment.
I'm still somewhat worried about this component partially copying existing component logic and 'missing' part of the logic there (e.g. hideDelayMs logic for 'smoother' tooltip hide)
If possible - would be nice if we could extend the existing WithTooltip by adding boolean prop (e.g. isKeyboardOnly or something like that)
If not possible - currrent option also works, but please let's make sure we reuse logic instead of copying where possible/as much as possible
|
Closing to use MUI instead #74565 |
2 participants
Links
Testing story
Deployment notes
Privacy and security