Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

[terra-dropdown]Added aria-haspoup and aria-controls attribute #3936

Merged
merged 32 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
caadb3e
Added aria-properties
Oct 13, 2023
536f8c8
addde changelog
Oct 13, 2023
3044177
changed id to fix wdio issues
Oct 16, 2023
4bab927
snap update
Oct 16, 2023
0f71abe
id updated
Oct 16, 2023
fcb7910
id updated
Oct 16, 2023
08b4966
id updated
Oct 16, 2023
20141c9
id updated
Oct 16, 2023
62206f0
id updated
Oct 16, 2023
5ed5690
updated snap files
Oct 16, 2023
b766425
snap update
Oct 16, 2023
9a8b867
snap update
Oct 17, 2023
a81aa81
snap update
Oct 17, 2023
1172c73
snap update
Oct 17, 2023
fa63590
snap update
Oct 17, 2023
9e8e1bf
snap update
Oct 17, 2023
21ad9ba
snap update
Oct 17, 2023
1bd9327
snap update
Oct 17, 2023
468dbd3
Review Comments Implemented
Oct 19, 2023
0a11b55
Review Comments Implemented
Oct 19, 2023
3344405
Merge branch 'main' of https://github.com/cerner/terra-core into drop…
Oct 19, 2023
e68b94f
Review Comments Implemented
Oct 19, 2023
ea5ae0d
Review Comments Implemented
Oct 19, 2023
f8779db
Merge branch 'main' of https://github.com/cerner/terra-core into drop…
Oct 26, 2023
7cb2fd3
updated as per review comments
Oct 26, 2023
3768685
updated as per review comments
Oct 26, 2023
d5e7bb7
updated as per review comments
Oct 26, 2023
0d7aa0d
added custom Props.classname for dropdown button
Oct 26, 2023
fb23b80
Merge branch 'main' of https://github.com/cerner/terra-core into drop…
Oct 30, 2023
d029e14
Merge branch 'dropdown-aria-property' of https://github.com/cerner/te…
Oct 30, 2023
9c44d50
Merge branch 'main' of https://github.com/cerner/terra-core into drop…
Oct 30, 2023
5ac876d
Merge branch 'main' of https://github.com/cerner/terra-core into drop…
Oct 31, 2023
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
1 change: 1 addition & 0 deletions packages/terra-core-docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

* Updated
* Updated `terra-alert` documentation for custom titles.
* Updated `terra-dropdown-button` documentation for testing uuid details.
* Updated exisiting examples upto a11y standards

* Fixed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ This component requires the following peer dependencies be installed in your app

<!-- AUTO-GENERATED-CONTENT:END -->

## Implementation Notes:
The Form-Field component must be composed inside the [Base][1] component with a locale in order for it to load the correct translation strings.

[1]: https://engineering.cerner.com/terra-ui/application/terra-application/components/application-base

## Usage

```jsx
Expand All @@ -62,3 +67,26 @@ import DropdownButton, { Item } from 'terra-dropdown-button';

## Item Props
<ItemPropsTable />

## Testing

Terra Dropdown button uses `uuid` which changes the component's description id dynamically. To mock the return value with the Jest testing library, `jest.spyOn` can be used.

If Enzyme `shallow` is being used for the tests then the mock may not be required depending on the depth of the returned wrapper. However, if `mount` is used then `uuid` should be mocked as shown below:

```js
import { v4 as uuidv4 } from 'uuid';

let mockSpyUuid;

// using a variable may result in failures. For best results, mock return value.
beforeAll(() => {
mockSpyUuid = jest.spyOn(uuidv4, 'v4').mockReturnValue('00000000-0000-0000-0000-000000000000');
});

// restore the mock
afterAll(() => {
mockSpyUuid.mockRestore();
});

```
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Children must be the `Item` subcomponent for proper functionality and appearance
## Implementation Notes:
The SplitButton component must be composed inside the [Base][1] component with a locale in order for it to load the correct translation strings.

[1]: https://engineering.cerner.com/terra-core/components/terra-base/base/base
[1]: https://engineering.cerner.com/terra-ui/application/terra-application/components/application-base

## Usage

Expand All @@ -51,4 +51,27 @@ import { Item, SplitButton } from 'terra-dropdown-button';
<SplitButtonPropsTable />

## Item Props
<ItemPropsTable />
<ItemPropsTable />

## Testing

Terra Split Button uses `uuid` which changes the component's description id dynamically. To mock the return value with the Jest testing library, `jest.spyOn` can be used.

If Enzyme `shallow` is being used for the tests then the mock may not be required depending on the depth of the returned wrapper. However, if `mount` is used then `uuid` should be mocked as shown below:

```js
import { v4 as uuidv4 } from 'uuid';

let mockSpyUuid;

// using a variable may result in failures. For best results, mock return value.
beforeAll(() => {
mockSpyUuid = jest.spyOn(uuidv4, 'v4').mockReturnValue('00000000-0000-0000-0000-000000000000');
});

// restore the mock
afterAll(() => {
mockSpyUuid.mockRestore();
});

```
3 changes: 3 additions & 0 deletions packages/terra-dropdown-button/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# Changelog

## Unreleased
* Added
* Added 'aria-haspoup' and 'aria-controls' attributes for dropdown button.
* Added customClass support for dropdown item.

## 1.37.0 - (October 26, 2023)

Expand Down
3 changes: 2 additions & 1 deletion packages/terra-dropdown-button/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
"prop-types": "^15.5.8",
"terra-hookshot": "^5.0.0",
"terra-mixins": "^1.40.0",
"terra-theme-context": "^1.0.0"
"terra-theme-context": "^1.0.0",
"uuid": "3.4.0"
},
"scripts": {
"compile": "babel --root-mode upward src --out-dir lib --copy-files",
Expand Down
8 changes: 8 additions & 0 deletions packages/terra-dropdown-button/src/DropdownButton.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import PropTypes from 'prop-types';
import classNamesBind from 'classnames/bind';
import ThemeContext from 'terra-theme-context';
import * as KeyCode from 'keycode-js';
// eslint-disable-next-line import/no-extraneous-dependencies
import { v4 as uuidv4 } from 'uuid';
import { injectIntl } from 'react-intl';
import DropdownButtonBase from './_DropdownButtonBase';
import styles from './DropdownButton.module.scss';
Expand Down Expand Up @@ -185,6 +187,9 @@ class DropdownButton extends React.Component {
const customLabel = (selectText) ? `${selectText}, ${selectedLabel}, ${label}` : label;
buttonAriaLabel = `${customLabel}${buttonAriaLabel ? `, ${buttonAriaLabel}` : ''}`;

const dropDownMenuId = uuidv4();
const dropDownMenuListId = `dropdown-menu-list-${dropDownMenuId}`;

return (
<DropdownButtonBase
{...customProps}
Expand All @@ -197,6 +202,7 @@ class DropdownButton extends React.Component {
refCallback={this.setListNode}
buttonRef={this.getButtonNode}
getSelectedOptionText={this.getSelectedOptionText}
menuId={dropDownMenuListId}
>
<button
{...modifiedButtonAttrs}
Expand All @@ -211,6 +217,8 @@ class DropdownButton extends React.Component {
ref={this.setButtonNode}
aria-expanded={isOpen}
aria-label={buttonAriaLabel}
aria-haspopup="true"
aria-controls={dropDownMenuListId}
Comment on lines +220 to +221
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to add aria-controls for split button as well :

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented.

onBlur={this.handleBlur}
>
<span className={cx('dropdown-button-text')}>{label}</span>
Expand Down
15 changes: 10 additions & 5 deletions packages/terra-dropdown-button/src/Item.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
import classNamesBind from 'classnames/bind';
import ThemeContext from 'terra-theme-context';
import styles from './Item.module.scss';
Expand Down Expand Up @@ -39,6 +40,14 @@ const Item = ({
label, onSelect, isActive, metaData, requestClose, ...customProps
}) => {
const theme = React.useContext(ThemeContext);
const itemClasses = classNames(
cx([
'item',
{ active: isActive },
theme.className,
]),
customProps.className,
);
/*
Having the li element with tabIndex -1 is important for VoiceOver in Safari, without it pressing VO + left/right will
navigate outside the dropdown with the dropdown still open if nothing else is pressed after opening the menu.
Expand All @@ -53,11 +62,7 @@ const Item = ({
}}
tabIndex="0"
role="menuitem"
className={cx(
'item',
{ active: isActive },
theme.className,
)}
className={itemClasses}
>
{label}
</li>
Expand Down
7 changes: 7 additions & 0 deletions packages/terra-dropdown-button/src/SplitButton.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import PropTypes from 'prop-types';
import classNamesBind from 'classnames/bind';
import ThemeContext from 'terra-theme-context';
import * as KeyCode from 'keycode-js';
// eslint-disable-next-line import/no-extraneous-dependencies
import { v4 as uuidv4 } from 'uuid';
import { injectIntl } from 'react-intl';
import DropdownButtonBase from './_DropdownButtonBase';
import styles from './SplitButton.module.scss';
Expand Down Expand Up @@ -242,6 +244,9 @@ class SplitButton extends React.Component {
const customLabel = (selectText) ? `${selectText}, ${selectedLabel}, ${caretLabel}` : caretLabel;
buttonAriaLabel = `${customLabel}${buttonAriaLabel ? `, ${buttonAriaLabel}` : ''}`;

const dropDownMenuId = uuidv4();
const dropDownMenuListId = `dropdown-menu-list-${dropDownMenuId}`;

return (
<DropdownButtonBase
{...customProps}
Expand All @@ -254,6 +259,7 @@ class SplitButton extends React.Component {
buttonRef={this.getButtonNode}
refCallback={this.setListNode}
getSelectedOptionText={this.getSelectedOptionText}
menuId={dropDownMenuListId}
>
<button
type="button"
Expand Down Expand Up @@ -282,6 +288,7 @@ class SplitButton extends React.Component {
aria-label={buttonAriaLabel}
onFocus={this.handleFocus}
onBlur={this.handleBlur}
aria-controls={dropDownMenuListId}
ref={this.setButtonNode}
>
<span className={cx('caret-icon')} />
Expand Down
4 changes: 3 additions & 1 deletion packages/terra-dropdown-button/src/_Dropdown.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,10 @@ const propTypes = {
};

const Dropdown = ({
requestClose, isOpen, targetRef, children, width, refCallback, buttonRef, getSelectedOptionText,
requestClose, isOpen, targetRef, children, width, refCallback, buttonRef, getSelectedOptionText, ...customProps
}) => {
const buttonFocused = useRef(false);
const { menuId } = customProps;
useEffect(() => {
// added this change to bring focus back to button when dropdown list is closed.
if (buttonFocused.current && buttonRef) {
Expand Down Expand Up @@ -76,6 +77,7 @@ const Dropdown = ({
width={width}
refCallback={refCallback}
getSelectedOptionText={getSelectedOptionText}
menuId={menuId}
>
{children}
</DropdownList>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ class DropdownButtonBase extends React.Component {
buttonRef={buttonRef}
refCallback={refCallback}
getSelectedOptionText={getSelectedOptionText}
menuId={customProps.menuId}
>
{items}
</Dropdown>
Expand Down
2 changes: 2 additions & 0 deletions packages/terra-dropdown-button/src/_DropdownList.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ class DropdownList extends React.Component {
const {
width,
refCallback,
...customProps
} = this.props;

const theme = this.context;
Expand All @@ -225,6 +226,7 @@ class DropdownList extends React.Component {
onKeyDown={this.handleKeyDown}
onKeyUp={this.handleKeyUp}
role="menu"
id={customProps.menuId}
>
{this.cloneChildren()}
</ul>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,21 @@ import ThemeContextProvider from 'terra-theme-context/lib/ThemeContextProvider';
import { IntlProvider } from 'react-intl';
/* eslint-disable-next-line import/no-extraneous-dependencies */
import { shallowWithIntl, mountWithIntl } from 'terra-enzyme-intl';
// eslint-disable-next-line import/no-extraneous-dependencies
import { v4 as uuidv4 } from 'uuid';
import translationsFile from '../../translations/en.json';
/* eslint-disable-next-line import/no-extraneous-dependencies */
import DropdownButton, { Item, Variants } from '../../src/DropdownButton';

let mockSpyUuid;
beforeAll(() => {
mockSpyUuid = jest.spyOn(uuidv4, 'v4').mockReturnValue('00000000-0000-0000-0000-000000000000');
});

afterAll(() => {
mockSpyUuid.mockRestore();
Comment on lines +13 to +18
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to add similar spy for splitbutton test as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

});

describe('Dropdown Button', () => {
it('should render a default dropdown type', () => {
const wrapper = shallowWithIntl(
Expand Down Expand Up @@ -87,7 +98,7 @@ describe('Dropdown Button', () => {
it('correctly applies the theme context className', () => {
const wrapper = mountWithIntl(
<ThemeContextProvider theme={{ className: 'orion-fusion-theme' }}>
<DropdownButton label="Primary Option">
<DropdownButton label="Primary Option" id="dropDown">
Copy link
Contributor

Choose a reason for hiding this comment

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

can we have jest test to ensure button is assigned with value passed by consumer as ID instead of the auto generated ID.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

<Item label="1st Option" onSelect={() => {}} />
</DropdownButton>
</ThemeContextProvider>,
Expand Down
11 changes: 11 additions & 0 deletions packages/terra-dropdown-button/tests/jest/SplitButton.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,22 @@ import ThemeContextProvider from 'terra-theme-context/lib/ThemeContextProvider';
import { IntlProvider } from 'react-intl';
/* eslint-disable-next-line import/no-extraneous-dependencies */
import { shallowWithIntl, mountWithIntl } from 'terra-enzyme-intl';
// eslint-disable-next-line import/no-extraneous-dependencies
import { v4 as uuidv4 } from 'uuid';
import translationsFile from '../../translations/en.json';

/* eslint-disable-next-line import/no-extraneous-dependencies */
import SplitButton, { Item } from '../../src/SplitButton';

let mockSpyUuid;
beforeAll(() => {
mockSpyUuid = jest.spyOn(uuidv4, 'v4').mockReturnValue('00000000-0000-0000-0000-000000000000');
});

afterAll(() => {
mockSpyUuid.mockRestore();
});

describe('Dropdown Button', () => {
it('should render a default split type', () => {
const wrapper = shallowWithIntl(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,12 @@ exports[`Dropdown Button correctly applies the theme context className 1`] = `
}
>
<InjectIntl(DropdownButton)
id="dropDown"
label="Primary Option"
>
<DropdownButton
buttonAttrs={Object {}}
id="dropDown"
intl={
Object {
"defaultFormats": Object {},
Expand Down Expand Up @@ -76,6 +78,7 @@ exports[`Dropdown Button correctly applies the theme context className 1`] = `
<DropdownButtonBase
buttonRef={[Function]}
getSelectedOptionText={[Function]}
id="dropDown"
isBlock={false}
isCompact={false}
isDisabled={false}
Expand All @@ -86,15 +89,20 @@ exports[`Dropdown Button correctly applies the theme context className 1`] = `
onSelect={[Function]}
/>
}
menuId="dropdown-menu-list-00000000-0000-0000-0000-000000000000"
refCallback={[Function]}
requestClose={[Function]}
>
<div
className="dropdown-button-base orion-fusion-theme"
id="dropDown"
menuId="dropdown-menu-list-00000000-0000-0000-0000-000000000000"
>
<button
aria-controls="dropdown-menu-list-00000000-0000-0000-0000-000000000000"
aria-disabled={false}
aria-expanded={false}
aria-haspopup="true"
aria-label="Primary Option"
className="dropdown-button neutral orion-fusion-theme"
disabled={false}
Expand All @@ -117,6 +125,7 @@ exports[`Dropdown Button correctly applies the theme context className 1`] = `
buttonRef={[Function]}
getSelectedOptionText={[Function]}
isOpen={false}
menuId="dropdown-menu-list-00000000-0000-0000-0000-000000000000"
refCallback={[Function]}
requestClose={[Function]}
targetRef={[Function]}
Expand Down
Loading
Loading