Skip to content
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.

Sign up for GitHub

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

[DOCS] Added a11y guidance for EuiTooltip and EuiPopover #7527

Merged
merged 1 commit into from
Feb 14, 2024
Merged

[DOCS] Added a11y guidance for EuiTooltip and EuiPopover #7527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 30 additions & 5 deletions src-docs/src/views/popover/popover_example.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
EuiPopoverTitle,
EuiPopoverFooter,
EuiCallOut,
EuiText,
} from '../../../../src/components';

import { EuiPopoverPanelProps } from './props';
Expand Down Expand Up @@ -157,6 +158,35 @@ const inputPopoverSnippet = `<EuiInputPopover

export const PopoverExample = {
title: 'Popover',
intro: (
<EuiText>
<p>
Use the <strong>EuiPopover</strong> component to hide controls or
options behind a clickable element. A popover is temporary so keep tasks
simple and narrowly focused.
</p>

<EuiCallOut
iconType="accessibility"
color="warning"
Copy link
Contributor

@cee-chen cee-chen Feb 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I really like how you moved the popover intro up here!

One note: We've historically used the blue/informational color for accessibility callouts in the past (example here). I'm not sure how we want to strike the balance of friendliness vs importance, but I think we should consider noting all our callouts around accessibility and ensuring we have some sort of consistent pattern in place and documented for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wondered about that, and I'm glad you brought it up @cee-chen. I'm happy to take another swing at this to modify the a11y callouts to blue and use that as a general guideline for future callouts.

title="Popovers have three accessibility requirements:"
>
<>
<ul>
<li>
Popover triggers <strong>must</strong> be anchored to elements
that accept keyboard focus.
</li>
<li>
Popovers can contain interactive elements. They{' '}
<strong>must</strong> be controlled by a click handler.
</li>
<li>Popovers must not be activated by hover or focus events.</li>
</ul>
</>
</EuiCallOut>
</EuiText>
),
sections: [
{
source: [
Expand All @@ -167,11 +197,6 @@ export const PopoverExample = {
],
text: (
<>
<p>
Use the <strong>EuiPopover</strong> component to hide controls or
options behind a clickable element. A popover is temporary so keep
tasks simple and narrowly focused.
</p>
<p>
While the visibility of the popover is maintained by the consuming
application, popovers will automatically close when clicking outside
Expand Down
38 changes: 20 additions & 18 deletions src-docs/src/views/tool_tip/tool_tip_example.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,26 @@ export const ToolTipExample = {
<EuiCallOut
iconType="accessibility"
color="warning"
title={
<>
Putting anything other than plain text in a tooltip is lost on
screen readers.
</>
}
/>
title="Tooltips have three accessibilty requirements:"
>
<>
<ul>
<li>
Tooltips <strong>must</strong> be anchored to elements that accept
keyboard focus.
</li>
<li>
Put only plain text in tooltips so the content is accessible to
keyboard and screen reader users.
</li>
<li>
{' '}
Do not add links, buttons, or other interactive content inside
tooltips.
</li>
</ul>
</>
</EuiCallOut>
</EuiText>
),
sections: [
Expand All @@ -87,17 +100,6 @@ export const ToolTipExample = {
will change it if the tooltip gets too close to the edge of the
screen.
</p>

<EuiCallOut
iconType="accessibility"
color="warning"
title={
<>
Anchoring a tooltip to a non-interactive element makes it
difficult for keyboard-only and screen reader users to read.
</>
}
/>
</>
),
source: [
Expand Down
Loading