From e0014db6fff23ab8f6c3601a97505410bcb9fd33 Mon Sep 17 00:00:00 2001 From: cchaos Date: Fri, 26 Jun 2020 11:42:01 -0400 Subject: [PATCH 1/6] [EuiSkipLink] Fix TS types and Safari click issue --- .../__snapshots__/skip_link.test.tsx.snap | 87 ++++++++++++++++--- .../accessibility/_screen_reader.scss | 3 +- .../accessibility/skip_link.test.tsx | 42 +++++++-- src/components/accessibility/skip_link.tsx | 24 +++-- 4 files changed, 127 insertions(+), 29 deletions(-) diff --git a/src/components/accessibility/__snapshots__/skip_link.test.tsx.snap b/src/components/accessibility/__snapshots__/skip_link.test.tsx.snap index 685870e4087..a0d373191cb 100644 --- a/src/components/accessibility/__snapshots__/skip_link.test.tsx.snap +++ b/src/components/accessibility/__snapshots__/skip_link.test.tsx.snap @@ -1,10 +1,27 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`EuiSkipLink is rendered 1`] = ` - + + + Skip + + + +`; + +exports[`EuiSkipLink props destinationId is rendered 1`] = ` + @@ -18,9 +35,9 @@ exports[`EuiSkipLink is rendered 1`] = ` `; -exports[`EuiSkipLink position absolute is rendered 1`] = ` +exports[`EuiSkipLink props onClick and destinationId is rendered 1`] = ` @@ -34,12 +51,41 @@ exports[`EuiSkipLink position absolute is rendered 1`] = ` `; -exports[`EuiSkipLink position fixed is rendered 1`] = ` - + + + + +`; + +exports[`EuiSkipLink props position absolute is rendered 1`] = ` + +`; + +exports[`EuiSkipLink props position fixed is rendered 1`] = ` + `; -exports[`EuiSkipLink position static is rendered 1`] = ` - - + +`; + +exports[`EuiSkipLink props tabIndex is rendered 1`] = ` + `; diff --git a/src/components/accessibility/_screen_reader.scss b/src/components/accessibility/_screen_reader.scss index 6cc8a8e6407..92fe2a7d8f8 100644 --- a/src/components/accessibility/_screen_reader.scss +++ b/src/components/accessibility/_screen_reader.scss @@ -1,4 +1,5 @@ +// The `:active` selector is necessary for Safari which removes `:focus` when a button is pressed .euiScreenReaderOnly, -.euiScreenReaderOnly--showOnFocus:not(:focus) { +.euiScreenReaderOnly--showOnFocus:not(:focus):not(:active) { @include euiScreenReaderOnly; } diff --git a/src/components/accessibility/skip_link.test.tsx b/src/components/accessibility/skip_link.test.tsx index 655319e29e3..7d51ff36b91 100644 --- a/src/components/accessibility/skip_link.test.tsx +++ b/src/components/accessibility/skip_link.test.tsx @@ -26,20 +26,46 @@ import { EuiSkipLink, POSITIONS } from './skip_link'; describe('EuiSkipLink', () => { test('is rendered', () => { const component = render( - + Skip ); expect(component).toMatchSnapshot(); }); - describe('position', () => { - POSITIONS.forEach(position => { - test(`${position} is rendered`, () => { - const component = render( - - ); + describe('props', () => { + test('destinationId is rendered', () => { + const component = render(); - expect(component).toMatchSnapshot(); + expect(component).toMatchSnapshot(); + }); + + test('tabIndex is rendered', () => { + const component = render(); + + expect(component).toMatchSnapshot(); + }); + + test('onClick is rendered', () => { + const component = render( {}} />); + + expect(component).toMatchSnapshot(); + }); + + test('onClick and destinationId is rendered', () => { + const component = render( + {}} destinationId="somewhere" /> + ); + + expect(component).toMatchSnapshot(); + }); + + describe('position', () => { + POSITIONS.forEach(position => { + test(`${position} is rendered`, () => { + const component = render(); + + expect(component).toMatchSnapshot(); + }); }); }); }); diff --git a/src/components/accessibility/skip_link.tsx b/src/components/accessibility/skip_link.tsx index 7cb97b2f662..6d69bc2120a 100644 --- a/src/components/accessibility/skip_link.tsx +++ b/src/components/accessibility/skip_link.tsx @@ -28,16 +28,18 @@ export const POSITIONS = ['static', 'fixed', 'absolute'] as Positions[]; export interface EuiSkipLinkProps extends EuiButtonProps { /** - * If true, the link will be fixed to the top left of the viewport + * Change the display position of the element when focused. + * If 'fixed', the link will be fixed to the top left of the viewport */ position?: Positions; - /** * Typically an anchor id (e.g. `a11yMainContent`), the value provided * will be prepended with a hash `#` and used as the link `href` */ - destinationId: string; - + destinationId?: string; + /** + * When position is fixed, this is forced to `0` + */ tabIndex?: number; } @@ -55,9 +57,9 @@ type propsForButton = PropsForButton< } >; -export type Props = ExclusiveUnion; +type Props = ExclusiveUnion; -export const EuiSkipLink: FunctionComponent = ({ +export const EuiSkipLink: FunctionComponent = ({ destinationId, tabIndex, position = 'static', @@ -71,14 +73,22 @@ export const EuiSkipLink: FunctionComponent = ({ className ); + // Create the `href` from `destinationId` IF provided + let optionalProps = {}; + if (destinationId) { + optionalProps = { + href: `#${destinationId}`, + }; + } + return ( {children} From 8d4ce7a97a4b22c0ce3b1d6fa351b28137f23da4 Mon Sep 17 00:00:00 2001 From: cchaos Date: Fri, 26 Jun 2020 12:02:12 -0400 Subject: [PATCH 2/6] cl --- CHANGELOG.md | 1 + src-docs/src/views/accessibility/skip_link.js | 41 +++++++++---------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e649171278..2b5c8dce98d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ **Bug fixes** - Fixed `EuiContextMenu` panel `onAnimationEnd` transition bug in Chrome ([#3656](https://github.com/elastic/eui/pull/3656)) +- Fixed `EuiSkipLink` interactive props and Safari click issue ([#3665](https://github.com/elastic/eui/pull/3665)) ## [`26.1.0`](https://github.com/elastic/eui/tree/v26.1.0) diff --git a/src-docs/src/views/accessibility/skip_link.js b/src-docs/src/views/accessibility/skip_link.js index 15cf70e03c1..90dd813c8b7 100644 --- a/src-docs/src/views/accessibility/skip_link.js +++ b/src-docs/src/views/accessibility/skip_link.js @@ -1,16 +1,18 @@ -import React, { Fragment, useState } from 'react'; +import React, { useState } from 'react'; -import { EuiSkipLink } from '../../../../src/components/accessibility/skip_link'; -import { EuiCallOut } from '../../../../src/components/call_out'; -import { EuiText } from '../../../../src/components/text'; -import { EuiSpacer } from '../../../../src/components/spacer'; -import { EuiSwitch } from '../../../../src/components/form/switch'; +import { + EuiSkipLink, + EuiCallOut, + EuiText, + EuiSpacer, + EuiSwitch, +} from '../../../../src/components'; export default () => { const [isFixed, setFixed] = useState(false); return ( - + <> {isFixed ? (

@@ -36,26 +38,21 @@ export default () => { onChange={e => setFixed(e.target.checked)} /> - {isFixed ? ( - - - Skip to main content - + + Skip to {isFixed && 'main '}content + + {isFixed && ( + <> - - ) : ( - - Skip to content - + )} - + ); }; From 14510bba7d09580c20a62e6c22beddb1eb204116 Mon Sep 17 00:00:00 2001 From: cchaos Date: Fri, 26 Jun 2020 12:17:30 -0400 Subject: [PATCH 3/6] Fixing type names and exports --- src/components/accessibility/index.ts | 2 +- src/components/accessibility/skip_link.tsx | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/accessibility/index.ts b/src/components/accessibility/index.ts index fcd8db30e13..d849d79552e 100644 --- a/src/components/accessibility/index.ts +++ b/src/components/accessibility/index.ts @@ -19,4 +19,4 @@ export { EuiKeyboardAccessible } from './keyboard_accessible'; export { EuiScreenReaderOnly } from './screen_reader'; -export { EuiSkipLink } from './skip_link'; +export { EuiSkipLink, EuiSkipLinkProps } from './skip_link'; diff --git a/src/components/accessibility/skip_link.tsx b/src/components/accessibility/skip_link.tsx index 6d69bc2120a..bc4171dca66 100644 --- a/src/components/accessibility/skip_link.tsx +++ b/src/components/accessibility/skip_link.tsx @@ -26,7 +26,7 @@ import { PropsForAnchor, PropsForButton, ExclusiveUnion } from '../common'; type Positions = 'static' | 'fixed' | 'absolute'; export const POSITIONS = ['static', 'fixed', 'absolute'] as Positions[]; -export interface EuiSkipLinkProps extends EuiButtonProps { +interface EuiSkipLinkInterface extends EuiButtonProps { /** * Change the display position of the element when focused. * If 'fixed', the link will be fixed to the top left of the viewport @@ -44,22 +44,22 @@ export interface EuiSkipLinkProps extends EuiButtonProps { } type propsForAnchor = PropsForAnchor< - EuiSkipLinkProps, + EuiSkipLinkInterface, { buttonRef?: Ref; } >; type propsForButton = PropsForButton< - EuiSkipLinkProps, + EuiSkipLinkInterface, { buttonRef?: Ref; } >; -type Props = ExclusiveUnion; +export type EuiSkipLinkProps = ExclusiveUnion; -export const EuiSkipLink: FunctionComponent = ({ +export const EuiSkipLink: FunctionComponent = ({ destinationId, tabIndex, position = 'static', From f744b579dfcaac0c7d7433d69a16cdf5d7588e0b Mon Sep 17 00:00:00 2001 From: cchaos Date: Fri, 26 Jun 2020 14:20:14 -0400 Subject: [PATCH 4/6] cl --- .../accessibility/skip_link.test.tsx | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/components/accessibility/skip_link.test.tsx b/src/components/accessibility/skip_link.test.tsx index 7d51ff36b91..7aee614cf3d 100644 --- a/src/components/accessibility/skip_link.test.tsx +++ b/src/components/accessibility/skip_link.test.tsx @@ -26,34 +26,26 @@ import { EuiSkipLink, POSITIONS } from './skip_link'; describe('EuiSkipLink', () => { test('is rendered', () => { const component = render( - Skip + + Skip + ); expect(component).toMatchSnapshot(); }); describe('props', () => { - test('destinationId is rendered', () => { - const component = render(); - - expect(component).toMatchSnapshot(); - }); - test('tabIndex is rendered', () => { - const component = render(); + const component = render( + + ); expect(component).toMatchSnapshot(); }); test('onClick is rendered', () => { - const component = render( {}} />); - - expect(component).toMatchSnapshot(); - }); - - test('onClick and destinationId is rendered', () => { const component = render( - {}} destinationId="somewhere" /> + {}} /> ); expect(component).toMatchSnapshot(); @@ -62,7 +54,9 @@ describe('EuiSkipLink', () => { describe('position', () => { POSITIONS.forEach(position => { test(`${position} is rendered`, () => { - const component = render(); + const component = render( + + ); expect(component).toMatchSnapshot(); }); From 6c93b402fc6daad2e90bebc153afef76c96b96cd Mon Sep 17 00:00:00 2001 From: cchaos Date: Fri, 26 Jun 2020 14:20:39 -0400 Subject: [PATCH 5/6] Snaps --- .../__snapshots__/skip_link.test.tsx.snap | 64 ++++++------------- 1 file changed, 19 insertions(+), 45 deletions(-) diff --git a/src/components/accessibility/__snapshots__/skip_link.test.tsx.snap b/src/components/accessibility/__snapshots__/skip_link.test.tsx.snap index a0d373191cb..8d73fa7dd02 100644 --- a/src/components/accessibility/__snapshots__/skip_link.test.tsx.snap +++ b/src/components/accessibility/__snapshots__/skip_link.test.tsx.snap @@ -1,11 +1,12 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`EuiSkipLink is rendered 1`] = ` - + `; -exports[`EuiSkipLink props destinationId is rendered 1`] = ` +exports[`EuiSkipLink props onClick is rendered 1`] = ` `; -exports[`EuiSkipLink props onClick and destinationId is rendered 1`] = ` +exports[`EuiSkipLink props position absolute is rendered 1`] = ` @@ -51,41 +52,12 @@ exports[`EuiSkipLink props onClick and destinationId is rendered 1`] = ` `; -exports[`EuiSkipLink props onClick is rendered 1`] = ` - -`; - -exports[`EuiSkipLink props position absolute is rendered 1`] = ` - -`; - exports[`EuiSkipLink props position fixed is rendered 1`] = ` - + `; exports[`EuiSkipLink props position static is rendered 1`] = ` - + `; exports[`EuiSkipLink props tabIndex is rendered 1`] = ` - + `; From 99ce6836930f964aaeb40392e84b7ffd55bf8745 Mon Sep 17 00:00:00 2001 From: cchaos Date: Fri, 26 Jun 2020 14:25:40 -0400 Subject: [PATCH 6/6] Require destinationId --- src/components/accessibility/skip_link.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/accessibility/skip_link.tsx b/src/components/accessibility/skip_link.tsx index bc4171dca66..0c2b2fca4c6 100644 --- a/src/components/accessibility/skip_link.tsx +++ b/src/components/accessibility/skip_link.tsx @@ -36,7 +36,7 @@ interface EuiSkipLinkInterface extends EuiButtonProps { * Typically an anchor id (e.g. `a11yMainContent`), the value provided * will be prepended with a hash `#` and used as the link `href` */ - destinationId?: string; + destinationId: string; /** * When position is fixed, this is forced to `0` */ @@ -73,7 +73,7 @@ export const EuiSkipLink: FunctionComponent = ({ className ); - // Create the `href` from `destinationId` IF provided + // Create the `href` from `destinationId` let optionalProps = {}; if (destinationId) { optionalProps = {