-
Notifications
You must be signed in to change notification settings - Fork 873
[EuiToolTip] Show tooltip only on keyboard focus #9624
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
Changes from all commits
cf641a4
56e803b
23b6524
cbf1e04
4226cf3
0441436
1928b01
e427b60
0e0414c
7e680b5
8816375
1ea39d2
e763d64
2ee6df2
77e7167
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -182,6 +182,24 @@ Ensure the `EuiIcon` includes appropriate accessibility attributes. | |
| - `EuiIcon` has an accessible name via `title`, `aria-label`, or `aria-labelledby`; otherwise mark it decorative with `aria-hidden={true}` | ||
| - Do not combine `tabIndex` with `aria-hidden` | ||
|
|
||
| ### `@elastic/eui/prefer-tooltip-trigger-focus-test-utility` | ||
|
|
||
| Flags `fireEvent.focus()` inside `it`/`test` blocks that also query for a tooltip element (`getByRole('tooltip')`, `queryByRole('tooltip')`, `findByRole('tooltip')` or any selector containing `euiToolTip`). Plain `fireEvent.focus` does not simulate `:focus-visible` in jsdom and will not trigger `EuiToolTip`, so tooltip focus tests will silently pass without actually showing the tooltip. | ||
|
|
||
| Use `focusEuiToolTipTrigger` from EUI's RTL test utilities instead, which correctly mocks `:focus-visible` before firing the focus event: | ||
|
|
||
| ```tsx | ||
| import { focusEuiToolTipTrigger } from '@elastic/eui/test/rtl'; | ||
|
|
||
| it('shows tooltip on focus', async () => { | ||
| const { getByRole } = render(<MyComponent />); | ||
| const cleanup = focusEuiToolTipTrigger(getByRole('button')); | ||
| expect(getByRole('tooltip')).toBeInTheDocument(); | ||
| cleanup(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on #9624 (comment) - We should probably highlight using try...finally here to ensure the cleanup is always called (independent of potential assertion failures).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We agreed to update this after merging. |
||
| }); | ||
| ``` | ||
|
|
||
|
|
||
| ## Testing | ||
|
|
||
| ### Running unit tests | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Added a new `prefer-tooltip-trigger-focus-test-utility` rule that flags `fireEvent.focus()` inside `it`/`test` blocks that also query for a tooltip. The rule auto-fixes to `focusEuiToolTipTrigger` from EUI's RTL test utilities. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,236 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
| * in compliance with, at your election, the Elastic License 2.0 or the Server | ||
| * Side Public License, v 1. | ||
| */ | ||
|
|
||
| import dedent from 'dedent'; | ||
| import { RuleTester } from '@typescript-eslint/rule-tester'; | ||
|
|
||
| import { PreferTooltipTriggerFocusTestUtility } from './prefer_tooltip_trigger_focus_test_utility'; | ||
|
|
||
| const languageOptions = { | ||
| parserOptions: { | ||
| ecmaFeatures: { | ||
| jsx: true, | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| const ruleTester = new RuleTester(); | ||
|
|
||
| ruleTester.run( | ||
| 'prefer-tooltip-trigger-focus-test-utility', | ||
| PreferTooltipTriggerFocusTestUtility, | ||
| { | ||
| valid: [ | ||
| { | ||
| // No tooltip signal in the it block, rule does not apply | ||
| code: dedent` | ||
| it('focuses', () => { | ||
| fireEvent.focus(element); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| }, | ||
| { | ||
| // Uses the correct helper, no violation | ||
| code: dedent` | ||
| it('shows tooltip on focus', () => { | ||
| focusEuiToolTipTrigger(element); | ||
| queryByRole('tooltip'); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| }, | ||
| { | ||
| // `fireEvent.blur` is not flagged | ||
| code: dedent` | ||
| it('hides tooltip on blur', () => { | ||
| fireEvent.blur(element); | ||
| queryByRole('tooltip'); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| }, | ||
| { | ||
| // `someOtherObj.focus()` is not flagged | ||
| code: dedent` | ||
| it('shows tooltip', () => { | ||
| someOtherObj.focus(element); | ||
| queryByRole('tooltip'); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| }, | ||
| { | ||
| // `fireEvent.focus` outside any it block, not tracked | ||
| code: dedent` | ||
| queryByRole('tooltip'); | ||
| fireEvent.focus(element); | ||
| `, | ||
| languageOptions, | ||
| }, | ||
| { | ||
| // `fireEvent.focus` in an it block without tooltip signal, not flagged | ||
| code: dedent` | ||
| it('focuses an input', () => { | ||
| fireEvent.focus(input); | ||
| getByRole('textbox'); | ||
| }); | ||
| it('shows tooltip on hover', () => { | ||
| fireEvent.mouseOver(trigger); | ||
| queryByRole('tooltip'); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| }, | ||
| { | ||
| // `.euiToolTip` class selector present but no `fireEvent.focus` in that block, valid | ||
| code: dedent` | ||
| it('finds tooltip element', () => { | ||
| document.querySelector('.euiToolTip'); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| }, | ||
| { | ||
| // `.euiToolTipAnchor` in one block does not affect `fireEvent.focus` in a separate block | ||
| code: dedent` | ||
| it('renders tooltip when showTooltip=true', () => { | ||
| const { container } = render(<ExtraActionsButton onClick={() => {}} showTooltip />); | ||
| expect(container.querySelector('.euiToolTipAnchor')).not.toBeNull(); | ||
| }); | ||
| it('focuses the button', () => { | ||
| const { getByTestId } = render(<ExtraActionsButton onClick={() => {}} />); | ||
| fireEvent.focus(getByTestId('showExtraActionsButton')); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| }, | ||
| ], | ||
|
|
||
| invalid: [ | ||
| { | ||
| // `queryByRole('tooltip')` in same it block, `fireEvent.focus` should be flagged | ||
| code: dedent` | ||
| it('shows tooltip on focus', () => { | ||
| fireEvent.focus(element); | ||
| queryByRole('tooltip'); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| errors: [{ messageId: 'preferTooltipTriggerFocusTestUtility' }], | ||
| }, | ||
| { | ||
| // `getByRole('tooltip')` in same it block, `fireEvent.focus` should be flagged | ||
| code: dedent` | ||
| it('shows tooltip on focus', () => { | ||
| fireEvent.focus(element); | ||
| getByRole('tooltip'); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| errors: [{ messageId: 'preferTooltipTriggerFocusTestUtility' }], | ||
| }, | ||
| { | ||
| // `screen.findByRole('tooltip')` in same it block, `fireEvent.focus` should be flagged | ||
| code: dedent` | ||
| it('shows tooltip on focus', async () => { | ||
| fireEvent.focus(element); | ||
| await screen.findByRole('tooltip'); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| errors: [{ messageId: 'preferTooltipTriggerFocusTestUtility' }], | ||
| }, | ||
| { | ||
| // test() block (alias for it), `fireEvent.focus` should be flagged | ||
| code: dedent` | ||
| test('shows tooltip on focus', () => { | ||
| fireEvent.focus(element); | ||
| queryByRole('tooltip'); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| errors: [{ messageId: 'preferTooltipTriggerFocusTestUtility' }], | ||
| }, | ||
| { | ||
| // tooltip query appears before `fireEvent.focus`, still flagged (whole-block analysis) | ||
| code: dedent` | ||
| it('shows tooltip on focus', () => { | ||
| queryByRole('tooltip'); | ||
| fireEvent.focus(element); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| errors: [{ messageId: 'preferTooltipTriggerFocusTestUtility' }], | ||
| }, | ||
| { | ||
| // Only the it block with a tooltip signal is flagged, not unrelated blocks | ||
| code: dedent` | ||
| it('focuses an input', () => { | ||
| fireEvent.focus(input); | ||
| getByRole('textbox'); | ||
| }); | ||
| it('shows tooltip on focus', () => { | ||
| fireEvent.focus(trigger); | ||
| queryByRole('tooltip'); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| errors: [{ messageId: 'preferTooltipTriggerFocusTestUtility' }], | ||
| }, | ||
| { | ||
| // Multiple `fireEvent.focus` calls in same tooltip it block, each flagged | ||
| code: dedent` | ||
| it('shows tooltip on focus', () => { | ||
| queryByRole('tooltip'); | ||
| fireEvent.focus(triggerA); | ||
| fireEvent.focus(triggerB); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| errors: [ | ||
| { messageId: 'preferTooltipTriggerFocusTestUtility' }, | ||
| { messageId: 'preferTooltipTriggerFocusTestUtility' }, | ||
| ], | ||
| }, | ||
| { | ||
| // `.euiToolTip` class selector in same it block, `fireEvent.focus` should be flagged | ||
| code: dedent` | ||
| it('shows tooltip on focus', () => { | ||
| fireEvent.focus(trigger); | ||
| document.querySelector('.euiToolTip'); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| errors: [{ messageId: 'preferTooltipTriggerFocusTestUtility' }], | ||
| }, | ||
| { | ||
| // `.euiToolTipAnchor` through `closest()` in same it block, `fireEvent.focus` should be flagged | ||
| code: dedent` | ||
| it('shows tooltip on focus', () => { | ||
| fireEvent.focus(trigger); | ||
| trigger.closest('.euiToolTipAnchor'); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| errors: [{ messageId: 'preferTooltipTriggerFocusTestUtility' }], | ||
| }, | ||
| { | ||
| // `[class*="euiToolTip"]` wildcard selector in same it block, `fireEvent.focus` should be flagged | ||
| code: dedent` | ||
| it('shows tooltip on focus', () => { | ||
| fireEvent.focus(trigger); | ||
| trigger.closest('[class*="euiToolTip"]'); | ||
| }); | ||
| `, | ||
| languageOptions, | ||
| errors: [{ messageId: 'preferTooltipTriggerFocusTestUtility' }], | ||
| }, | ||
| ], | ||
| } | ||
| ); |
Uh oh!
There was an error while loading. Please reload this page.