From 1fb638b12df290533e6a89e7f8135a7e23c0797e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20H=C3=B8egh?= Date: Thu, 19 Sep 2019 13:08:00 +0200 Subject: [PATCH] fix: enhance NVDA support for #dropdown #button #checkbox #dropdown #input #radio #switch #slider #textarea #date-picker #globa-status --- .../components/slider/slider-properties.md | 4 +- .../src/components/button/Button.js | 7 +- .../__snapshots__/Button.test.js.snap | 8 +- .../src/components/checkbox/Checkbox.js | 3 - .../__snapshots__/Checkbox.test.js.snap | 1 - .../date-picker/DatePickerCalendar.js | 7 +- .../src/components/dropdown/Dropdown.js | 153 ++++--- .../__snapshots__/Dropdown.test.js.snap | 24 +- .../src/components/form-row/FormRow.js | 2 +- .../__snapshots__/FormRow.test.js.snap | 6 +- .../components/global-status/GlobalStatus.js | 42 +- .../__tests__/GlobalStatus.test.js | 2 +- .../__snapshots__/GlobalStatus.test.js.snap | 377 +++++++++--------- .../global-status/style/_global-status.scss | 4 + .../dnb-ui-lib/src/components/input/Input.js | 13 +- .../dnb-ui-lib/src/components/modal/Modal.js | 12 +- .../__snapshots__/Modal.test.js.snap | 31 +- .../dnb-ui-lib/src/components/radio/Radio.js | 28 +- .../src/components/radio/RadioGroup.js | 6 +- .../__snapshots__/Radio.test.js.snap | 14 +- .../src/components/slider/Slider.js | 21 +- .../__snapshots__/Slider.test.js.snap | 31 +- .../src/components/switch/Switch.js | 3 - .../__snapshots__/Switch.test.js.snap | 1 - .../dnb-ui-lib/src/components/tabs/Tabs.js | 61 ++- .../__tests__/__snapshots__/Tabs.test.js.snap | 1 + .../src/components/textarea/Textarea.js | 3 - .../toggle-button/ToggleButtonGroup.js | 2 +- .../__snapshots__/ToggleButton.test.js.snap | 4 +- .../shared/__tests__/custom-element.test.js | 2 +- .../dnb-ui-lib/src/shared/component-helper.js | 2 +- .../dnb-ui-lib/stories/components/Dropdown.js | 14 +- 32 files changed, 481 insertions(+), 408 deletions(-) diff --git a/packages/dnb-design-system-portal/src/docs/uilib/components/slider/slider-properties.md b/packages/dnb-design-system-portal/src/docs/uilib/components/slider/slider-properties.md index bd4d1cee815..e05b56ca21c 100644 --- a/packages/dnb-design-system-portal/src/docs/uilib/components/slider/slider-properties.md +++ b/packages/dnb-design-system-portal/src/docs/uilib/components/slider/slider-properties.md @@ -15,8 +15,8 @@ draft: true | `hide_buttons` | _(optional)_ removes the helper buttons. Defaults to `false`. | | `use_scrollwheel` | _(optional)_ enable mouse scroll-wheel support. Defaults to `false`. | | `thump_title` | _(optional)_ give the slider thump button a title for accessibility reason. Defaults to `null`. | -| `subtract_title` | _(optional)_ give the subtract button a title for accessibility reason. Defaults to `null`. | -| `add_title` | _(optional)_ give the add button a title for accessibility reason. Defaults to `null`. | +| `subtract_title` | _(optional)_ give the subtract button a title for accessibility reason. Defaults to `−`. | +| `add_title` | _(optional)_ give the add button a title for accessibility reason. Defaults to `+`. | | `label` | _(optional)_ prepends the Form Label component. If no ID is provided, a random ID is created. | | `status` | _(optional)_ text with a status message. The style defaults to an error message. | | `status_state` | _(optional)_ defines the state of the status. Currently there are two statuses `[error, info]`. Defaults to `error`. | diff --git a/packages/dnb-ui-lib/src/components/button/Button.js b/packages/dnb-ui-lib/src/components/button/Button.js index d65b511df7e..acd6c401d1d 100644 --- a/packages/dnb-ui-lib/src/components/button/Button.js +++ b/packages/dnb-ui-lib/src/components/button/Button.js @@ -216,9 +216,6 @@ export default class Button extends PureComponent { onMouseOut: this.onMouseOutHandler, // for resetting the button to the default state onClick: this.onClickHandler } - if (!params['aria-label'] && !text && title) { - params['aria-label'] = title - } // also used for code markup simulation validateDOMAttributes(this.props, params) @@ -303,15 +300,13 @@ class Content extends PureComponent { } if (icon) { - const alt = title || text ret.push( ) } diff --git a/packages/dnb-ui-lib/src/components/button/__tests__/__snapshots__/Button.test.js.snap b/packages/dnb-ui-lib/src/components/button/__tests__/__snapshots__/Button.test.js.snap index a13dab6bec9..c6ec7cc1ccc 100644 --- a/packages/dnb-ui-lib/src/components/button/__tests__/__snapshots__/Button.test.js.snap +++ b/packages/dnb-ui-lib/src/components/button/__tests__/__snapshots__/Button.test.js.snap @@ -65,7 +65,7 @@ exports[`Button component have to match default button snapshot 1`] = ` text !day.isLastMonth && @@ -346,7 +345,6 @@ const PrevButton = ({ icon="chevron-left" size="small" aria-label={title} - title={title} onClick={onClick} onKeyDown={onKeyDown} /> @@ -390,7 +388,6 @@ const NextButton = ({ icon="chevron-right" size="small" aria-label={title} - title={title} onClick={onClick} onKeyDown={onKeyDown} /> diff --git a/packages/dnb-ui-lib/src/components/dropdown/Dropdown.js b/packages/dnb-ui-lib/src/components/dropdown/Dropdown.js index 98f66067751..fcb6d95ff4a 100644 --- a/packages/dnb-ui-lib/src/components/dropdown/Dropdown.js +++ b/packages/dnb-ui-lib/src/components/dropdown/Dropdown.js @@ -347,7 +347,7 @@ export default class Dropdown extends PureComponent { attributes }) } - setHidden = () => { + setHidden = ({ setFocus = false } = {}) => { this.setState( { opened: false, @@ -356,10 +356,25 @@ export default class Dropdown extends PureComponent { () => { this._hideTimeout = setTimeout( () => { - this.setState({ - hidden: true, - _listenForPropChanges: false - }) + this.setState( + { + hidden: true, + _listenForPropChanges: false + }, + () => { + if (setFocus) { + let elem = this._refButton.current + try { + elem = this._refButton.current._ref.current + } catch (e) { + // do noting + } + if (elem && elem.focus) { + elem.focus() + } + } + } + ) }, this.props.no_animation ? 1 : Dropdown.blurDelay ) // wait until animation is over @@ -418,7 +433,7 @@ export default class Dropdown extends PureComponent { this.changedOrderFor = value } } catch (e) { - console.log('Dropdown could not findItemByValue:', e) + console.warn('Dropdown could not findItemByValue:', e) } return index @@ -447,17 +462,26 @@ export default class Dropdown extends PureComponent { event, attributes }) - if (ret === false) return + if (ret === false) { + return + } } + + if (!selected_item) { + return + } + // try to scroll to item - if (!this._refUl.current) return + if (!this._refUl.current) { + return + } + try { const ulElement = this._refUl.current const liElement = ulElement.querySelector( `li.dnb-dropdown__option:nth-of-type(${active_item + 1})` ) const top = liElement.offsetTop - // const { parentNode } = liElement if (ulElement.scrollTo) { const params = { top @@ -473,7 +497,7 @@ export default class Dropdown extends PureComponent { liElement.focus() } } catch (e) { - console.log('Dropdown could not scroll into element:', e) + console.warn('Dropdown could not scroll into element:', e) } } ) @@ -525,35 +549,61 @@ export default class Dropdown extends PureComponent { } } + preventTab = e => { + switch (keycode(e)) { + case 'tab': + this.setHidden() + break + } + } + onKeyDownHandler = e => { - let active_item = this.state.active_item + let active_item = parseFloat(this.state.active_item) const total = this.state.data.length - 1 switch (keycode(e)) { + case 'shift': + e.preventDefault() + break + case 'up': e.preventDefault() - active_item-- + if (active_item > -1) { + active_item-- + } else { + active_item = total + } break + case 'down': e.preventDefault() - active_item++ + if (active_item > -1) { + active_item++ + } else { + active_item = 0 + } break + case 'home': e.preventDefault() active_item = 0 break + case 'end': e.preventDefault() active_item = total break + case 'enter': case 'space': e.preventDefault() this.selectItem(active_item, { fireSelectEvent: true, event: e }) this.setHidden() break + case 'esc': - e.preventDefault() + case 'tab': + e.preventDefault() // on edge, we need this prevent to not loose focus after close this.setHidden() break @@ -621,18 +671,10 @@ export default class Dropdown extends PureComponent { if (this._selectTimeout) { clearTimeout(this._selectTimeout) } - this._selectTimeout = setTimeout(() => { - this.setHidden() - let elem = this._refButton.current - try { - elem = this._refButton.current._ref.current - } catch (e) { - // do noting - } - if (elem && elem.focus) { - elem.focus() - } - }, 150) // only for the user experience + this._selectTimeout = setTimeout( + () => this.setHidden({ setFocus: true }), + 150 + ) // only for the user experience } if ( @@ -685,7 +727,7 @@ export default class Dropdown extends PureComponent { this._refUl.current.scrollTop + this._refUl.current.offsetHeight ) closestToTop = findClosest(counts, this._refUl.current.scrollTop) - if (closestToTop !== tmpToTop) { + if (itemSpots[closestToTop] && closestToTop !== tmpToTop) { this.setState({ closestToTop: itemSpots[closestToTop].i, _listenForPropChanges: false @@ -705,7 +747,7 @@ export default class Dropdown extends PureComponent { this._refUl.current.addEventListener('scroll', this.setOnScroll) this.setOnScroll() } catch (e) { - console.log('Dropdown could not set onScroll:', e) + console.warn('Dropdown could not set onScroll:', e) } } @@ -757,7 +799,7 @@ export default class Dropdown extends PureComponent { window.addEventListener('resize', this.setDirection) this.setDirection() } catch (e) { - console.log('Dropdown could not set onResize:', e) + console.warn('Dropdown could not set onResize:', e) } } @@ -777,7 +819,7 @@ export default class Dropdown extends PureComponent { let { icon, icon_position } = props const { - title, + title: titleProp, label, label_direction, icon: _icon, // eslint-disable-line @@ -833,6 +875,12 @@ export default class Dropdown extends PureComponent { const showStatus = status && status !== 'error' const currentOptionData = Dropdown.getOptionData(selected_item, data) + const title = + data && data.length > 0 + ? currentOptionData.selected_value || + Dropdown.parseContentTitle(currentOptionData) || + titleProp + : titleProp const mainParams = { className: classnames( @@ -859,10 +907,6 @@ export default class Dropdown extends PureComponent { ) } - // To link the selected item with the aria-labelledby, use this: - // const selectedId = `option-${id}-${selected_item}` - // But for now we use - const selectedId = `dropdown-${id}-value` const triggerParams = { className: classnames( 'dnb-dropdown__trigger', @@ -870,8 +914,7 @@ export default class Dropdown extends PureComponent { ), id, disabled, - ['aria-haspopup']: 'listbox', - ['aria-labelledby']: selectedId, + ['aria-haspopup']: true, //listbox ['aria-expanded']: opened, ...attributes, onFocus: this.onFocusHandler, @@ -880,7 +923,10 @@ export default class Dropdown extends PureComponent { onKeyDown: this.onTriggerKeyDownHandler } if (typeof title === 'string') { - triggerParams.title = title + triggerParams['title'] = title + } + if (hidden && label) { + triggerParams['aria-labelledby'] = id + '-label' } const listParams = { className: classnames( @@ -889,16 +935,20 @@ export default class Dropdown extends PureComponent { ) } const ulParams = { - className: 'dnb-dropdown__options', + className: 'dnb-dropdown__options', // dnb-no-focus role: 'listbox', - tabIndex: '0', ['aria-labelledby']: id, ref: this._refUl, style: { maxHeight: max_height > 0 ? `${max_height}rem` : null } } - if (selected_item !== null && selected_item > -1) { + if ( + !isPopupMenu && + !hidden && + selected_item !== null && + selected_item > -1 + ) { ulParams['aria-activedescendant'] = `option-${id}-${selected_item}` } @@ -919,6 +969,7 @@ export default class Dropdown extends PureComponent { text={label} direction={label_direction} disabled={disabled} + onMouseDown={this.toggleVisible} /> )) || ( @@ -949,15 +1000,8 @@ export default class Dropdown extends PureComponent { > {!isPopupMenu && ( - - {data && data.length > 0 - ? currentOptionData.selected_value || - Dropdown.parseContentTitle(currentOptionData) || - title - : title} + + {title} )} @@ -987,9 +1031,9 @@ export default class Dropdown extends PureComponent { const isCurrent = i === parseFloat(selected_item) const liParams = { id: `option-${id}-${i}`, - role: 'option', // here we could use "menuitem" + role: 'option', tabIndex: '-1', - title: Dropdown.parseContentTitle(dataItem), + // title: Dropdown.parseContentTitle(dataItem),// freaks out NVDA className: classnames( 'dnb-dropdown__option', isCurrent && 'dnb-dropdown__option--selected', @@ -1003,13 +1047,12 @@ export default class Dropdown extends PureComponent { i === data.length - 1 && 'last-of-type' // because of the triangle element ), onMouseDown: this.selectItemHandler, + onKeyDown: this.preventTab, 'data-item': i } if (isCurrent) { - liParams['aria-selected'] = true - - // use checked if we use role="menuitem" - // liParams['aria-checked'] = true + liParams['aria-current'] = true // has best support on NVDA + liParams['aria-selected'] = true // has best support on VO } return (
  • diff --git a/packages/dnb-ui-lib/src/components/dropdown/__tests__/__snapshots__/Dropdown.test.js.snap b/packages/dnb-ui-lib/src/components/dropdown/__tests__/__snapshots__/Dropdown.test.js.snap index 491451fd36a..62352c75496 100644 --- a/packages/dnb-ui-lib/src/components/dropdown/__tests__/__snapshots__/Dropdown.test.js.snap +++ b/packages/dnb-ui-lib/src/components/dropdown/__tests__/__snapshots__/Dropdown.test.js.snap @@ -96,6 +96,7 @@ exports[`Dropdown component have to match snapshot 1`] = ` for_id="dropdown-id" id="dropdown-id-label" label_direction={null} + onMouseDown={[Function]} render_content={null} text="label" title={null} @@ -106,11 +107,13 @@ exports[`Dropdown component have to match snapshot 1`] = ` htmlFor="dropdown-id" id="dropdown-id-label" is="label" + onMouseDown={[Function]} > @@ -210,8 +213,8 @@ exports[`Dropdown component have to match snapshot 1`] = ` - - -

    -
    -

    - text + + + + + + + + + + + + +

    -
      -
    • - item #1 -
      • -
    • +
        - item #2 - -
      +
    • + item #1 +
      • +
    • + item #2 +
      • +
    +
    - - - + + + `; diff --git a/packages/dnb-ui-lib/src/components/global-status/style/_global-status.scss b/packages/dnb-ui-lib/src/components/global-status/style/_global-status.scss index 94ce011918c..fa9863eb8f8 100644 --- a/packages/dnb-ui-lib/src/components/global-status/style/_global-status.scss +++ b/packages/dnb-ui-lib/src/components/global-status/style/_global-status.scss @@ -40,6 +40,10 @@ } .dnb-global-status { + &__wrapper { + position: relative; + z-index: 1; + } &, &--no-animation#{&}--visible { display: flex; diff --git a/packages/dnb-ui-lib/src/components/input/Input.js b/packages/dnb-ui-lib/src/components/input/Input.js index 9d001e69155..34b13481c03 100644 --- a/packages/dnb-ui-lib/src/components/input/Input.js +++ b/packages/dnb-ui-lib/src/components/input/Input.js @@ -343,10 +343,10 @@ export default class Input extends PureComponent { } // we may considder using: aria-details - if (showStatus) { - inputParams['aria-describedby'] = id + '-status' - } else if (description) { - inputParams['aria-describedby'] = id + '-description' + if (showStatus || description) { + inputParams['aria-describedby'] = `${ + showStatus ? id + '-status' : '' + } ${description ? id + '-description' : ''}` } if (type === 'search') { inputParams.autoComplete = 'off' @@ -354,9 +354,6 @@ export default class Input extends PureComponent { if (readOnly) { inputParams['aria-readonly'] = inputParams.readOnly = true } - if (label) { - inputParams['aria-labelledby'] = id + '-label' - } const shellParams = { 'data-input-state': inputState, @@ -380,7 +377,7 @@ export default class Input extends PureComponent { {label && ( { + clearTimeout(this._setFocusId) + this._setFocusId = setTimeout(() => { try { this._contentRef.current.focus() // in case the button is disabled const focusElement = this._contentRef.current.querySelector( @@ -686,7 +686,7 @@ export const CloseButton = ({ on_click, title, className = null }) => ( className={classnames('dnb-modal__close-button', className)} icon="close" icon_size="medium" - title={title} + aria-label={title} on_click={on_click} /> ) diff --git a/packages/dnb-ui-lib/src/components/modal/__tests__/__snapshots__/Modal.test.js.snap b/packages/dnb-ui-lib/src/components/modal/__tests__/__snapshots__/Modal.test.js.snap index 987faa2214f..c3a382aea85 100644 --- a/packages/dnb-ui-lib/src/components/modal/__tests__/__snapshots__/Modal.test.js.snap +++ b/packages/dnb-ui-lib/src/components/modal/__tests__/__snapshots__/Modal.test.js.snap @@ -104,7 +104,7 @@ exports[`Modal component have to match snapshot 1`] = ` trigger_icon="question" trigger_icon_position="left" trigger_text={null} - trigger_title="Modal" + trigger_title={null} trigger_variant="secondary" >