From 2e8630f470500ed35b4d688d4dc80e2c1ce6a716 Mon Sep 17 00:00:00 2001 From: Michael Spaxman Date: Fri, 28 Feb 2020 15:30:07 -0500 Subject: [PATCH] feat(focusable): add focusable component, cypress-tab-plugin add cypress tests add prop for dynamically controlling focusability/classname --- .../src/components/Focusable/Focusable.tsx | 50 +++++ .../Focusable/__tests__/Focusable.test.tsx | 37 ++++ .../__snapshots__/Focusable.test.tsx.snap | 43 +++++ .../Focusable/examples/Focusable.md | 171 ++++++++++++++++++ .../src/components/Focusable/index.ts | 1 + .../components/Tooltip/examples/Tooltip.md | 14 +- packages/react-core/src/components/index.ts | 1 + .../cypress/integration/focusable.spec.ts | 113 ++++++++++++ .../cypress/support/index.js | 1 + .../demo-app-ts/src/Demos.ts | 7 +- .../demos/FocusableDemo/FocusableDemo.tsx | 169 +++++++++++++++++ .../demo-app-ts/src/components/demos/index.ts | 1 + packages/react-integration/package.json | 1 + yarn.lock | 25 +++ 14 files changed, 632 insertions(+), 2 deletions(-) create mode 100644 packages/react-core/src/components/Focusable/Focusable.tsx create mode 100644 packages/react-core/src/components/Focusable/__tests__/Focusable.test.tsx create mode 100644 packages/react-core/src/components/Focusable/__tests__/__snapshots__/Focusable.test.tsx.snap create mode 100644 packages/react-core/src/components/Focusable/examples/Focusable.md create mode 100644 packages/react-core/src/components/Focusable/index.ts create mode 100644 packages/react-integration/cypress/integration/focusable.spec.ts create mode 100644 packages/react-integration/demo-app-ts/src/components/demos/FocusableDemo/FocusableDemo.tsx diff --git a/packages/react-core/src/components/Focusable/Focusable.tsx b/packages/react-core/src/components/Focusable/Focusable.tsx new file mode 100644 index 00000000000..420c1907bd3 --- /dev/null +++ b/packages/react-core/src/components/Focusable/Focusable.tsx @@ -0,0 +1,50 @@ +import * as React from 'react'; +import { css } from '@patternfly/react-styles'; + +export interface FocusableProps { + children?: string | React.ReactElement; + /** Any additional classes to apply to the focusable element */ + className?: string; + /** An HTML element to use as the container */ + component?: React.ElementType; + /** Sets the tabindex value */ + tabIndex?: number; + /** Whether an intermediary DOM node container is needed */ + withContainer?: boolean; + /** A prop for dynamically controlling focusability */ + isFocusable?: boolean; +} + +const renderElement = ({ children, tabIndex, className, component: Component, ...props }: FocusableProps) => + typeof children === 'object' + ? React.cloneElement(children as React.ReactElement, { + tabIndex, + className: css(className), + ...props + }) + : React.createElement(Component, { tabIndex, className: css(className), ...props }, children); + +const Focusable: React.FunctionComponent = ({ + children, + className, + component: Component = 'div', + tabIndex = 0, + isFocusable = true, + withContainer = false, + ...props +}) => { + const focusableElement = children as React.ReactElement; + + const getDefaultTabIndex = () => + !isFocusable || focusableElement.props.isDisabled || focusableElement.props.disabled ? tabIndex : null; + + return withContainer ? ( + + {children} + + ) : ( + renderElement({ children, tabIndex, className, component: Component, ...props }) + ); +}; + +export { Focusable }; diff --git a/packages/react-core/src/components/Focusable/__tests__/Focusable.test.tsx b/packages/react-core/src/components/Focusable/__tests__/Focusable.test.tsx new file mode 100644 index 00000000000..d2a84d22fd6 --- /dev/null +++ b/packages/react-core/src/components/Focusable/__tests__/Focusable.test.tsx @@ -0,0 +1,37 @@ +import * as React from 'react'; +import { shallow } from 'enzyme'; +import { Card } from '../../Card/Card'; +import { Button } from '../../Button/Button'; +import { Focusable } from '../Focusable'; + +test('Focusable', () => { + const view = shallow(test); + expect(view).toMatchSnapshot(); +}); + +test('Focus non-interactive html', () => { + const view = shallow( + +
Article element
+ + ); + expect(view).toMatchSnapshot(); +}); + +test('Focus non-interactive component', () => { + const view = shallow( + + Card element + + ); + expect(view).toMatchSnapshot(); +}); + +test('Focus disabled button', () => { + const view = shallow( + + + + ); + expect(view).toMatchSnapshot(); +}); diff --git a/packages/react-core/src/components/Focusable/__tests__/__snapshots__/Focusable.test.tsx.snap b/packages/react-core/src/components/Focusable/__tests__/__snapshots__/Focusable.test.tsx.snap new file mode 100644 index 00000000000..f54e897127f --- /dev/null +++ b/packages/react-core/src/components/Focusable/__tests__/__snapshots__/Focusable.test.tsx.snap @@ -0,0 +1,43 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Focus disabled button 1`] = ` + + + Button text + + +`; + +exports[`Focus non-interactive component 1`] = ` + + Card element + +`; + +exports[`Focus non-interactive html 1`] = ` +
+ Article element +
+`; + +exports[`Focusable 1`] = ` +
+ test +
+`; diff --git a/packages/react-core/src/components/Focusable/examples/Focusable.md b/packages/react-core/src/components/Focusable/examples/Focusable.md new file mode 100644 index 00000000000..e01b1944d43 --- /dev/null +++ b/packages/react-core/src/components/Focusable/examples/Focusable.md @@ -0,0 +1,171 @@ +--- +title: 'Focusable' +section: components +cssPrefix: 'pf-c-focusable' +typescript: true +propComponents: ['Focusable'] +beta: true +--- + +import { Focusable, Button, Tooltip, Radio, Card, CardHeader, CardBody, CardFooter } from '@patternfly/react-core'; +import { BeerIcon } from '@patternfly/react-icons'; + +## Examples +```js title=Focus-text +import React from 'react'; +import { Focusable } from '@patternfly/react-core'; + +class FocusableText extends React.Component { + constructor(props) { + super(props); + } + render() { + return ( + + This text is focusable + + ); + } +} +``` + +```js title=Focus-non-interactive-html-children +import React from 'react'; +import { Focusable, Button } from '@patternfly/react-core'; + +class FocusNonInteractiveHtmlChildren extends React.Component { + constructor(props) { + super(props); + } + render() { + return ( + +
Article element
+ + ); + } +} +``` + +```js title=Focus-non-interactive-component-children +import React from 'react'; +import { Focusable, Button, Card, CardHeader, CardBody, CardFooter } from '@patternfly/react-core'; + +class FocusNonInteractiveComponentChildren extends React.Component { + constructor(props) { + super(props); + } + render() { + return ( + + + Header + Body + Footer + + + ); + } +} +``` + +```js title=Focus-with-positive-tabindex +import React from 'react'; +import { Focusable } from '@patternfly/react-core'; + +class FocusPositiveTabindex extends React.Component { + constructor(props) { + super(props); + } + render() { + return ( + + + + ); + } +} +``` + +```js title=Focus-an-icon +import React from 'react'; +import { Focusable, Button, Tooltip } from '@patternfly/react-core'; +import { BeerIcon } from '@patternfly/react-icons'; + +class FocusIcon extends React.Component { + constructor(props) { + super(props); + } + render() { + return ( + + + + + + ); + } +} +``` + +```js title=Wrapping-disabled-button-with-tooltip +import React from 'react'; +import { Focusable, Button, Tooltip } from '@patternfly/react-core'; + +class WrapDisabledButtonTooltip extends React.Component { + constructor(props) { + super(props); + } + + render() { + return ( + + + + + + ); + } +} +``` + +```js title=Wrapping-disabled-html-button-with-tooltip +import React from 'react'; +import { Focusable, Button, Tooltip } from '@patternfly/react-core'; + +class WrapDisabledButtonTooltip extends React.Component { + constructor(props) { + super(props); + } + + render() { + return ( + + + + + + ); + } +} +``` + +```js title=Wrapping-disabled-radio-with-tooltip +import React from 'react'; +import { Focusable, Radio, Tooltip } from '@patternfly/react-core'; + +class WrapDisabledRadioTooltip extends React.Component { + constructor(props) { + super(props); + } + render() { + return ( + + + + + + ); + } +} +``` diff --git a/packages/react-core/src/components/Focusable/index.ts b/packages/react-core/src/components/Focusable/index.ts new file mode 100644 index 00000000000..d3ca639e400 --- /dev/null +++ b/packages/react-core/src/components/Focusable/index.ts @@ -0,0 +1 @@ +export * from './Focusable'; diff --git a/packages/react-core/src/components/Tooltip/examples/Tooltip.md b/packages/react-core/src/components/Tooltip/examples/Tooltip.md index 4b2b2ac5675..1542590f091 100644 --- a/packages/react-core/src/components/Tooltip/examples/Tooltip.md +++ b/packages/react-core/src/components/Tooltip/examples/Tooltip.md @@ -6,7 +6,7 @@ typescript: true propComponents: ['Tooltip'] --- -import { Button, Tooltip, TooltipPosition, Checkbox } from '@patternfly/react-core'; +import { Button, Tooltip, TooltipPosition, Checkbox, Focusable } from '@patternfly/react-core'; import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons'; ## Examples @@ -54,6 +54,18 @@ import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons'; ``` +```js title=On-disabled-element +import React from 'react'; +import { Tooltip, TooltipPosition, Button, Focusable } from '@patternfly/react-core'; +import { OutlinedQuestionCircleIcon } from '@patternfly/react-icons'; + + + + + + +``` + ```js title=Positions import React from 'react'; import { Button, Tooltip, TooltipPosition, Checkbox } from '@patternfly/react-core'; diff --git a/packages/react-core/src/components/index.ts b/packages/react-core/src/components/index.ts index 8727fd25019..94db8430e11 100644 --- a/packages/react-core/src/components/index.ts +++ b/packages/react-core/src/components/index.ts @@ -20,6 +20,7 @@ export * from './DataList'; export * from './Dropdown'; export * from './EmptyState'; export * from './Expandable'; +export * from './Focusable'; export * from './Form'; export * from './FormSelect'; export * from './InputGroup'; diff --git a/packages/react-integration/cypress/integration/focusable.spec.ts b/packages/react-integration/cypress/integration/focusable.spec.ts new file mode 100644 index 00000000000..fddb207f1ca --- /dev/null +++ b/packages/react-integration/cypress/integration/focusable.spec.ts @@ -0,0 +1,113 @@ +describe('Focusable Simple Test', () => { + it('Navigate to demo section', () => { + cy.visit('http://localhost:3000/'); + cy.get('#focusable-demo-nav-item-link').click(); + cy.url().should('eq', 'http://localhost:3000/focusable-demo-nav-link'); + }); + + it('Checks that incrementing tabindex values send focus to the correct elements', () => { + // eslint-disable-next-line @typescript-eslint/ban-ts-ignore + // @ts-ignore + cy.get('body').tab(); + cy.focused() + .should('have.attr', 'tabindex', '1') + .and('have.attr', 'data-positive-tabindex-example'); + + // eslint-disable-next-line @typescript-eslint/ban-ts-ignore + // @ts-ignore + cy.focused().tab(); + cy.focused() + .should('have.attr', 'tabindex', '3') + .and('have.attr', 'data-disabled-textinput-tooltip-example'); + }); + + it('Check that you can tab through non-interactive elements', () => { + // eslint-disable-next-line @typescript-eslint/ban-ts-ignore + // @ts-ignore // goal is to focus a section that has tabindex="-1" and press tab from there, currently you have to supply tabindex="0" + cy.get('.non-interactive-elements') + .focus() + .tab() + .should('have.attr', 'tabindex', '0') + .and('have.attr', 'data-text-example'); + + // eslint-disable-next-line @typescript-eslint/ban-ts-ignore + // @ts-ignore + cy.focused() + .tab() + .should('have.attr', 'tabindex', '0') + .and('have.attr', 'data-non-interactive-html-example'); + + // eslint-disable-next-line @typescript-eslint/ban-ts-ignore + // @ts-ignore + cy.focused() + .tab() + .should('have.attr', 'tabindex', '0') + .and('have.attr', 'data-non-interactive-component-example'); + + // eslint-disable-next-line @typescript-eslint/ban-ts-ignore + // @ts-ignore + cy.focused() + .tab() + .should('have.attr', 'tabindex', '0') + .and('have.attr', 'data-icon-example'); + }); + + it('Check that focusing disabled element wrappers displays the tooltip', () => { + cy.get('[data-disabled-button-tooltip-example]') + .focus() + .should('have.attr', 'aria-describedby', 'tippy-2'); + + cy.get('[data-disabled-html-button-tooltip-example]') + .focus() + .should('have.attr', 'aria-describedby', 'tippy-3'); + + cy.get('[data-disabled-radio-tooltip-example]') + .focus() + .should('have.attr', 'aria-describedby', 'tippy-4'); + + cy.get('[data-disabled-textinput-tooltip-example]') + .focus() + .should('have.attr', 'aria-describedby', 'tippy-5'); + }); + + it('Check that you can tab through interactive elements', () => { + // eslint-disable-next-line @typescript-eslint/ban-ts-ignore + // @ts-ignore + cy.get('.interactive-elements-enabled') + .focus() + .within(() => { + // goal is to focus a section that has tabindex="-1" and press tab from there + // eslint-disable-next-line @typescript-eslint/ban-ts-ignore + // @ts-ignore + cy.tab() + .focused() + .should('have.attr', 'tabindex', '0') + .and('have.attr', 'data-enabled-button-tooltip-example'); + }); + + cy.get('.interactive-elements-enabled') + .focus() + .within(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-ignore + // @ts-ignore + cy.tab() + .tab() + .focused() + .should('have.attr', 'tabindex', '0') + .and('have.attr', 'data-enabled-radio-tooltip-example'); + }); + + cy.get('.interactive-elements-enabled') + .focus() + .within(() => { + // eslint-disable-next-line @typescript-eslint/ban-ts-ignore + // @ts-ignore + cy.tab() + .tab() + .tab() + .focused() + .should('have.attr', 'tabindex', '0') + .and('have.attr', 'data-redundantly-undisabled-button-tooltip-example'); + }); + }); +}); diff --git a/packages/react-integration/cypress/support/index.js b/packages/react-integration/cypress/support/index.js index 37a498fb5bf..9d8c04ad277 100644 --- a/packages/react-integration/cypress/support/index.js +++ b/packages/react-integration/cypress/support/index.js @@ -12,6 +12,7 @@ // You can read more here: // https://on.cypress.io/configuration // *********************************************************** +import 'cypress-plugin-tab'; // Import commands.js using ES2015 syntax: import './commands'; diff --git a/packages/react-integration/demo-app-ts/src/Demos.ts b/packages/react-integration/demo-app-ts/src/Demos.ts index fa4512cd961..3643f2c72e4 100644 --- a/packages/react-integration/demo-app-ts/src/Demos.ts +++ b/packages/react-integration/demo-app-ts/src/Demos.ts @@ -6,7 +6,7 @@ interface DemoInterface { /** The name of the demo */ name: string; /** Demo component associated with the demo */ - componentType: any; + componentType: React.ComponentType; } /** Add the name of the demo and it's component here to have them show up in the demo app */ export const Demos: DemoInterface[] = [ @@ -301,6 +301,11 @@ export const Demos: DemoInterface[] = [ name: 'Expandable Demo', componentType: Examples.ExpandableDemo }, + { + id: 'focusable-demo', + name: 'Focusable Demo', + componentType: Examples.FocusableDemo + }, { id: 'flex-demo', name: 'Flex Demo', diff --git a/packages/react-integration/demo-app-ts/src/components/demos/FocusableDemo/FocusableDemo.tsx b/packages/react-integration/demo-app-ts/src/components/demos/FocusableDemo/FocusableDemo.tsx new file mode 100644 index 00000000000..e295961ee32 --- /dev/null +++ b/packages/react-integration/demo-app-ts/src/components/demos/FocusableDemo/FocusableDemo.tsx @@ -0,0 +1,169 @@ +import React from 'react'; +import { + Focusable, + Card, + CardHeader, + Tooltip, + Button, + Radio, + Gallery, + GalleryItem, + Title, + TextInput +} from '@patternfly/react-core'; +import { css } from '@patternfly/react-styles'; +import alignmentStyles from '@patternfly/react-styles/css/utilities/Alignment/alignment'; +import spacingStyles from '@patternfly/react-styles/css/utilities/Spacing/spacing'; +import { BeerIcon } from '@patternfly/react-icons'; + +export class FocusableDemo extends React.Component { + render() { + return ( + + + Non-interactive elements + + + + + + Simple text + + + + +
Article element
+ + + + + + Header + + + + + + + + + + + + + + + Interactive elements (disabled) + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Interactive elements (enabled) + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + ); + } +} diff --git a/packages/react-integration/demo-app-ts/src/components/demos/index.ts b/packages/react-integration/demo-app-ts/src/components/demos/index.ts index 5e36aa3d9d3..0a88bb5588e 100644 --- a/packages/react-integration/demo-app-ts/src/components/demos/index.ts +++ b/packages/react-integration/demo-app-ts/src/components/demos/index.ts @@ -56,6 +56,7 @@ export * from './DrawerDemo/DrawerDemo'; export * from './DropdownDemo/DropdownDemo'; export * from './EmptyStateDemo/EmptyStateDemo'; export * from './ExpandableDemo/ExpandableDemo'; +export * from './FocusableDemo/FocusableDemo'; export * from './FlexDemo/FlexDemo'; export * from './FormDemo/FormDemo'; export * from './FormSelectDemo/FormSelectDemo'; diff --git a/packages/react-integration/package.json b/packages/react-integration/package.json index 6ee8178b64c..0b3e4bd8dbd 100644 --- a/packages/react-integration/package.json +++ b/packages/react-integration/package.json @@ -33,6 +33,7 @@ "devDependencies": { "@cypress/webpack-preprocessor": "^4.0.3", "cypress": "^3.8.2", + "cypress-plugin-tab": "^1.0.5", "jasmine-xml2html-converter": "^0.0.2", "junit-merge": "^2.0.0", "local-web-server": "^2.6.1", diff --git a/yarn.lock b/yarn.lock index 6ed9110bb2b..8291e2620b6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4605,6 +4605,14 @@ ajv@^6.0.1, ajv@^6.1.0, ajv@^6.10.0, ajv@^6.10.2, ajv@^6.5.3, ajv@^6.5.5: json-schema-traverse "^0.4.1" uri-js "^4.2.2" +ally.js@^1.4.1: + version "1.4.1" + resolved "https://registry.yarnpkg.com/ally.js/-/ally.js-1.4.1.tgz#9fb7e6ba58efac4ee9131cb29aa9ee3b540bcf1e" + integrity sha1-n7fmuljvrE7pExyymqnuO1QLzx4= + dependencies: + css.escape "^1.5.0" + platform "1.3.3" + alphanum-sort@^1.0.0: version "1.0.2" resolved "https://registry.yarnpkg.com/alphanum-sort/-/alphanum-sort-1.0.2.tgz#97a1119649b211ad33691d9f9f486a8ec9fbe0a3" @@ -8229,6 +8237,11 @@ css-what@2.1, css-what@^2.1.2: version "2.1.3" resolved "https://registry.yarnpkg.com/css-what/-/css-what-2.1.3.tgz#a6d7604573365fe74686c3f311c56513d88285f2" +css.escape@^1.5.0: + version "1.5.1" + resolved "https://registry.yarnpkg.com/css.escape/-/css.escape-1.5.1.tgz#42e27d4fa04ae32f931a4b4d4191fa9cddee97cb" + integrity sha1-QuJ9T6BK4y+TGktNQZH6nN3ul8s= + css@^2.2.3: version "2.2.4" resolved "https://registry.yarnpkg.com/css/-/css-2.2.4.tgz#c646755c73971f2bba6a601e2cf2fd71b1298929" @@ -8369,6 +8382,13 @@ cyclist@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/cyclist/-/cyclist-1.0.1.tgz#596e9698fd0c80e12038c2b82d6eb1b35b6224d9" +cypress-plugin-tab@^1.0.5: + version "1.0.5" + resolved "https://registry.yarnpkg.com/cypress-plugin-tab/-/cypress-plugin-tab-1.0.5.tgz#a40714148104004bb05ed62b1bf46bb544f8eb4a" + integrity sha512-QtTJcifOVwwbeMP3hsOzQOKf3EqKsLyjtg9ZAGlYDntrCRXrsQhe4ZQGIthRMRLKpnP6/tTk6G0gJ2sZUfRliQ== + dependencies: + ally.js "^1.4.1" + cypress@^3.8.2: version "3.8.2" resolved "https://registry.yarnpkg.com/cypress/-/cypress-3.8.2.tgz#58fa96e1e7dae712403b0f4e8af1efe35442ff7a" @@ -18007,6 +18027,11 @@ pkginfo@0.x.x: resolved "https://registry.yarnpkg.com/pkginfo/-/pkginfo-0.4.1.tgz#b5418ef0439de5425fc4995042dced14fb2a84ff" integrity sha1-tUGO8EOd5UJfxJlQQtztFPsqhP8= +platform@1.3.3: + version "1.3.3" + resolved "https://registry.yarnpkg.com/platform/-/platform-1.3.3.tgz#646c77011899870b6a0903e75e997e8e51da7461" + integrity sha1-ZGx3ARiZhwtqCQPnXpl+jlHadGE= + plop@^2.0.0: version "2.5.2" resolved "https://registry.yarnpkg.com/plop/-/plop-2.5.2.tgz#52433ce18abe859f4b4f97a2ea9b2125c4129d98"