Skip to content

[DSCO] Adds a Keyboard Only Tooltip#74018

Open
hannahbergam wants to merge 1 commit into
stagingfrom
hbergam/accessible-tooltip
Open

[DSCO] Adds a Keyboard Only Tooltip#74018
hannahbergam wants to merge 1 commit into
stagingfrom
hbergam/accessible-tooltip

Conversation

@hannahbergam

Copy link
Copy Markdown
Contributor

Links

  • Jira:

Testing story

Deployment notes

Privacy and security

@hannahbergam
hannahbergam requested a review from a team as a code owner July 21, 2026 23:24
@github-actions

Copy link
Copy Markdown

🖼️ Storybook Visual Comparison Report

✅ No Storybook eyes differences detected!

@levadadenys levadadenys left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: let's call it Design System - Tooltip - WithKeyboardTooltip (to highlight it's a part of existing tooltip, not a standalone component)

Comment on lines +17 to +18
const TAIL_OFFSET = 4;
const TAIL_LENGTHS = {l: 12, m: 9, s: 6, xs: 6};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants