Skip to content

Commit

Permalink
[Dashboard Navigation] Fix a11y concerns (elastic#174772)
Browse files Browse the repository at this point in the history
## Summary

This PR addresses the a11y issues that were brought up as part of the
[a11y review](elastic#163802) via the
following:

- **Added `aria-current` attribute to the current dashboard link**
  | Before | After |
  |--------|--------|
| ![Screenshot 2024-01-12 at 9 49
54 AM](https://github.com/elastic/kibana/assets/8698078/975f6acf-fd87-47f3-a12f-02abeb2e5dce)
| ![Screenshot 2024-01-12 at 10 03
11 AM](https://github.com/elastic/kibana/assets/8698078/102b3e12-f98e-4f17-9d51-ac8a70d2f34e)
|

- **Returned focus to the create flyout after adding/editing a link**
  | Before | After |
  |--------|--------|
|
![image](https://github.com/elastic/kibana/assets/8698078/4d74f3b3-788b-47e7-9dcb-d54a6383026b)
|
![image](https://github.com/elastic/kibana/assets/8698078/6b057ee5-094a-49c5-92c2-10deba15b7db)
|

- **Added `aria-label` attribute to `Technical preview` badge**
  | Before | After |
  |--------|--------|
| ![Screenshot 2024-01-12 at 11 16
14 AM](https://github.com/elastic/kibana/assets/8698078/06b454ce-07e9-438e-9d41-1d8505ca0e28)
| ![Screenshot 2024-01-12 at 11 13
07 AM](https://github.com/elastic/kibana/assets/8698078/44ee37c6-18cc-4918-b127-0b9583630612)
|



- **Added more context to the `aria-label`s for the edit/delete link
buttons**
  | Before | After |
  |--------|--------|
| ![Screenshot 2024-01-12 at 11 43
45 AM](https://github.com/elastic/kibana/assets/8698078/f90db90f-3da6-4ab6-807d-fba73315ee66)
| ![Screenshot 2024-01-12 at 11 43
19 AM](https://github.com/elastic/kibana/assets/8698078/2d9e6ae2-6bd4-4957-ab6d-93fda0708471)
|



### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
([Link](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4856))

![image](https://github.com/elastic/kibana/assets/8698078/74c173dc-2b59-43e8-bc9b-33a1ae8297b0)
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
  • Loading branch information
Heenawter authored and CoenWarmer committed Feb 15, 2024
1 parent 99098a5 commit 98ab6b8
Show file tree
Hide file tree
Showing 14 changed files with 130 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ export const legacyEmbeddableToApi = (
(embeddable as VisualizeEmbeddable).getOutput().visTypeName === 'markdown';

const isImage = embeddable.type === 'image';
const isNavigation = embeddable.type === 'navigation';
return !isInputControl && !isMarkdown && !isImage && !isNavigation;
const isLinks = embeddable.type === 'links';
return !isInputControl && !isMarkdown && !isImage && !isLinks;
};

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ export const DashboardLinkComponent = ({
label={linkLabel}
external={link.options?.openInNewTab}
data-test-subj={error ? `${id}--error` : `${id}`}
aria-current={link.destination === parentDashboardId}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ describe('LinksEditor', () => {
onAddToDashboard: jest.fn(),
onClose: jest.fn(),
isByReference: false,
flyoutId: 'test-id',
};

const someLinks: Link[] = [
Expand Down
17 changes: 13 additions & 4 deletions src/plugins/links/public/components/editor/links_editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import {
LINKS_HORIZONTAL_LAYOUT,
LINKS_VERTICAL_LAYOUT,
} from '../../../common/content_management';
import { memoizedGetOrderedLinkList } from '../../editor/links_editor_tools';
import { focusMainFlyout, memoizedGetOrderedLinkList } from '../../editor/links_editor_tools';
import { openLinkEditorFlyout } from '../../editor/open_link_editor_flyout';
import { LinksLayoutInfo } from '../../embeddable/types';
import { coreServices } from '../../services/kibana_services';
Expand Down Expand Up @@ -71,6 +71,7 @@ const LinksEditor = ({
initialLayout,
parentDashboard,
isByReference,
flyoutId,
}: {
onSaveToLibrary: (newLinks: Link[], newLayout: LinksLayoutType) => Promise<void>;
onAddToDashboard: (newLinks: Link[], newLayout: LinksLayoutType) => void;
Expand All @@ -79,6 +80,7 @@ const LinksEditor = ({
initialLayout?: LinksLayoutType;
parentDashboard?: DashboardContainer;
isByReference: boolean;
flyoutId: string; // used to manage the focus of this flyout after individual link editor flyout is closed
}) => {
const toasts = coreServices.notifications.toasts;
const isMounted = useMountedState();
Expand Down Expand Up @@ -120,6 +122,7 @@ const LinksEditor = ({
const newLink = await openLinkEditorFlyout({
parentDashboard,
link: linkToEdit,
mainFlyoutId: flyoutId,
ref: editLinkFlyoutRef,
});
if (newLink) {
Expand All @@ -137,7 +140,7 @@ const LinksEditor = ({
}
}
},
[editLinkFlyoutRef, orderedLinks, parentDashboard]
[editLinkFlyoutRef, orderedLinks, parentDashboard, flyoutId]
);

const hasZeroLinks = useMemo(() => {
Expand All @@ -151,8 +154,9 @@ const LinksEditor = ({
return link.id !== linkId;
})
);
focusMainFlyout(flyoutId);
},
[orderedLinks]
[orderedLinks, flyoutId]
);

return (
Expand All @@ -172,7 +176,12 @@ const LinksEditor = ({
<EuiFlexItem grow={false}>
<EuiToolTip content={LinksStrings.editor.panelEditor.getTechnicalPreviewTooltip()}>
{/* The EuiBadge needs an empty title to prevent the default tooltip */}
<EuiBadge color="hollow" tabIndex={0} title="">
<EuiBadge
color="hollow"
tabIndex={0}
title=""
aria-label={LinksStrings.editor.panelEditor.getTechnicalPreviewTooltip()}
>
{LinksStrings.editor.panelEditor.getTechnicalPreviewLabel()}
</EuiBadge>
</EuiToolTip>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export const LinksEditorSingleLink = ({
size="xs"
iconType="pencil"
onClick={editLink}
aria-label={LinksStrings.editor.getEditLinkTitle()}
aria-label={LinksStrings.editor.getEditLinkTitle(linkLabel)}
data-test-subj="panelEditorLink--editBtn"
/>
</EuiToolTip>
Expand All @@ -170,7 +170,7 @@ export const LinksEditorSingleLink = ({
<EuiButtonIcon
size="xs"
iconType="trash"
aria-label={LinksStrings.editor.getDeleteLinkTitle()}
aria-label={LinksStrings.editor.getDeleteLinkTitle(linkLabel)}
color="danger"
onClick={deleteLink}
data-test-subj="panelEditorLink--deleteBtn"
Expand Down
12 changes: 7 additions & 5 deletions src/plugins/links/public/components/links_strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@ export const LinksStrings = {
i18n.translate('links.editor.updateButtonLabel', {
defaultMessage: 'Update link',
}),
getEditLinkTitle: () =>
i18n.translate('links.editor.editLinkTitle', {
defaultMessage: 'Edit link',
getEditLinkTitle: (label?: string) =>
i18n.translate('links.editor.editLinkTitle.hasLabel', {
defaultMessage: 'Edit {label} link',
values: { label: label ?? '' },
}),
getDeleteLinkTitle: () =>
getDeleteLinkTitle: (label?: string) =>
i18n.translate('links.editor.deleteLinkTitle', {
defaultMessage: 'Delete link',
defaultMessage: 'Delete {label} link',
values: { label: label ?? '' },
}),
getCancelButtonLabel: () =>
i18n.translate('links.editor.cancelButtonLabel', {
Expand Down
11 changes: 11 additions & 0 deletions src/plugins/links/public/editor/links_editor_tools.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,14 @@ export const memoizedGetOrderedLinkList = memoize(
return links;
}
);

/**
* Return focus to the main flyout div to align with a11y standards
* @param flyoutId ID of the main flyout div element
*/
export const focusMainFlyout = (flyoutId: string) => {
const flyoutElement = document.getElementById(flyoutId);
if (flyoutElement) {
flyoutElement.focus();
}
};
7 changes: 6 additions & 1 deletion src/plugins/links/public/editor/open_editor_flyout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import React from 'react';
import { skip, take } from 'rxjs/operators';
import { v4 as uuidv4 } from 'uuid';

import { EuiLoadingSpinner, EuiPanel } from '@elastic/eui';
import { DashboardContainer } from '@kbn/dashboard-plugin/public/dashboard_container';
Expand Down Expand Up @@ -58,6 +59,8 @@ export async function openEditorFlyout(
}

return new Promise((resolve, reject) => {
const flyoutId = `linksEditorFlyout-${uuidv4()}`;

const closeEditorFlyout = (editorFlyout: OverlayRef) => {
if (overlayTracker) {
overlayTracker.clearOverlays();
Expand Down Expand Up @@ -124,6 +127,7 @@ export async function openEditorFlyout(
const editorFlyout = coreServices.overlays.openFlyout(
toMountPoint(
<LinksEditor
flyoutId={flyoutId}
initialLinks={initialLinks}
initialLayout={attributes?.layout}
onClose={onCancel}
Expand All @@ -135,10 +139,11 @@ export async function openEditorFlyout(
{ theme: coreServices.theme, i18n: coreServices.i18n }
),
{
id: flyoutId,
maxWidth: 720,
ownFocus: true,
outsideClickCloses: false,
onClose: onCancel,
outsideClickCloses: false,
className: 'linksPanelEditor',
'data-test-subj': 'links--panelEditor--flyout',
}
Expand Down
4 changes: 4 additions & 0 deletions src/plugins/links/public/editor/open_link_editor_flyout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ import { DashboardContainer } from '@kbn/dashboard-plugin/public/dashboard_conta
import { coreServices } from '../services/kibana_services';
import { Link } from '../../common/content_management';
import { LinkEditor } from '../components/editor/link_editor';
import { focusMainFlyout } from './links_editor_tools';

export interface LinksEditorProps {
link?: Link;
parentDashboard?: DashboardContainer;
mainFlyoutId: string;
ref: React.RefObject<HTMLDivElement>;
}

Expand All @@ -34,6 +36,7 @@ export type UnorderedLink = Omit<Link, 'order'>;
export async function openLinkEditorFlyout({
ref,
link,
mainFlyoutId, // used to manage the focus of this flyout after inidividual link editor flyout is closed
parentDashboard,
}: LinksEditorProps): Promise<UnorderedLink | undefined> {
const unmountFlyout = async () => {
Expand All @@ -44,6 +47,7 @@ export async function openLinkEditorFlyout({
// wait for close animation before unmounting
setTimeout(() => {
if (ref.current) ReactDOM.unmountComponentAtNode(ref.current);
focusMainFlyout(mainFlyoutId);
}, 180);
});
};
Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugins/translations/translations/fr-FR.json
Original file line number Diff line number Diff line change
Expand Up @@ -4771,8 +4771,6 @@
"links.description": "Utiliser des liens pour accéder aux tableaux de bord et aux sites web couramment utilisés.",
"links.editor.addButtonLabel": "Ajouter un lien",
"links.editor.cancelButtonLabel": "Fermer",
"links.editor.deleteLinkTitle": "Supprimer le lien",
"links.editor.editLinkTitle": "Modifier le lien",
"links.editor.horizontalLayout": "Horizontal",
"links.editor.unableToSaveToastTitle": "Erreur lors de l'enregistrement du Panneau de liens",
"links.editor.updateButtonLabel": "Mettre à jour le lien",
Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugins/translations/translations/ja-JP.json
Original file line number Diff line number Diff line change
Expand Up @@ -4786,8 +4786,6 @@
"links.description": "リンクを使用して、よく使用されるダッシュボードや Web サイトに移動します。",
"links.editor.addButtonLabel": "リンクを追加",
"links.editor.cancelButtonLabel": "閉じる",
"links.editor.deleteLinkTitle": "リンクを削除",
"links.editor.editLinkTitle": "リンクを編集",
"links.editor.horizontalLayout": "横",
"links.editor.unableToSaveToastTitle": "リンクパネルの保存エラー",
"links.editor.updateButtonLabel": "リンクを更新",
Expand Down
2 changes: 0 additions & 2 deletions x-pack/plugins/translations/translations/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -4879,8 +4879,6 @@
"links.description": "使用链接以导航到常用仪表板和网站。",
"links.editor.addButtonLabel": "添加链接",
"links.editor.cancelButtonLabel": "关闭",
"links.editor.deleteLinkTitle": "删除链接",
"links.editor.editLinkTitle": "编辑链接",
"links.editor.horizontalLayout": "水平",
"links.editor.unableToSaveToastTitle": "保存链接面板时出错",
"links.editor.updateButtonLabel": "更新链接",
Expand Down
81 changes: 81 additions & 0 deletions x-pack/test/accessibility/apps/group1/dashboard_links.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { FtrProviderContext } from '../../ftr_provider_context';

export default function ({ getService, getPageObjects }: FtrProviderContext) {
const a11y = getService('a11y');
const deployment = getService('deployment');
const testSubjects = getService('testSubjects');
const kibanaServer = getService('kibanaServer');
const dashboardAddPanel = getService('dashboardAddPanel');
const { common, dashboard, home, dashboardLinks } = getPageObjects([
'common',
'dashboard',
'home',
'dashboardLinks',
]);

const DASHBOARD_NAME = 'Test Links Panel A11y';

describe('Dashboard links a11y tests', () => {
before(async () => {
await kibanaServer.savedObjects.cleanStandardList();
await common.navigateToUrl('home', '/tutorial_directory/sampleData', {
useActualUrl: true,
});
await home.addSampleDataSet('flights');
await common.navigateToApp('dashboard');
await dashboard.gotoDashboardLandingPage();
await dashboard.clickNewDashboard();
await dashboard.saveDashboard(DASHBOARD_NAME, { exitFromEditMode: false });
});

after(async () => {
await dashboard.clickQuickSave();
await common.navigateToUrl('home', '/tutorial_directory/sampleData', {
useActualUrl: true,
});
await home.removeSampleDataSet('flights');
await kibanaServer.savedObjects.cleanStandardList();
});

it('Empty links editor flyout', async () => {
await dashboardAddPanel.clickEditorMenuButton();
await dashboardAddPanel.clickAddNewEmbeddableLink('links');
await a11y.testAppSnapshot();
});

it('Add dashboard link flyout', async () => {
await testSubjects.click('links--panelEditor--addLinkBtn');
await testSubjects.exists('links--linkEditor--flyout');
await a11y.testAppSnapshot();
});

it('Add external link flyout', async () => {
const radioOption = await testSubjects.find('links--linkEditor--externalLink--radioBtn');
const label = await radioOption.findByCssSelector('label[for="externalLink"]');
await label.click();
await a11y.testAppSnapshot();
await dashboardLinks.clickLinkEditorCloseButton();
});

it('Non-empty links editor flyout', async () => {
await dashboardLinks.addDashboardLink('[Flights] Global Flight Dashboard');
await dashboardLinks.addDashboardLink(DASHBOARD_NAME);
await dashboardLinks.addExternalLink(`${deployment.getHostPort()}/app/bar`);
await a11y.testAppSnapshot();
});

it('Links panel', async () => {
await dashboardLinks.toggleSaveByReference(false);
await dashboardLinks.clickPanelEditorSaveButton();
await testSubjects.existOrFail('links--component');
await a11y.testAppSnapshot();
});
});
}
3 changes: 2 additions & 1 deletion x-pack/test/accessibility/apps/group1/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ export default ({ loadTestFile }: FtrProviderContext): void => {
loadTestFile(require.resolve('./uptime'));
loadTestFile(require.resolve('./spaces'));
loadTestFile(require.resolve('./advanced_settings'));
loadTestFile(require.resolve('./dashboard_panel_options'));
loadTestFile(require.resolve('./dashboard_controls'));
loadTestFile(require.resolve('./dashboard_links'));
loadTestFile(require.resolve('./dashboard_panel_options'));
loadTestFile(require.resolve('./users'));
loadTestFile(require.resolve('./roles'));
loadTestFile(require.resolve('./ingest_node_pipelines'));
Expand Down

0 comments on commit 98ab6b8

Please sign in to comment.