Skip to content

fix(aria): make aria-* component props consistent #1731

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
redallen wants to merge 11 commits into from
Closed

fix(aria): make aria-* component props consistent #1731

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
eddd52a
fix(aria): make aria-* props consistent
redallen Apr 9, 2019
d77bbab
fix tabs aria label
redallen Apr 9, 2019
2414488
fix typo
redallen Apr 9, 2019
967e2a9
Fix AlertActionCloseButton variant label
redallen Apr 9, 2019
29d5f0a
don't pass title or variant to <button> in DOM
redallen Apr 9, 2019
b7c824a
fix DataListToggle ariaLabelledBy logic
redallen Apr 9, 2019
658381f
Fix aria-labelledby in docs
redallen Apr 9, 2019
d7b6f0a
fix modal aria docs
redallen Apr 9, 2019
ccedbe9
Rename aria-labels that could possibly be extended
redallen Apr 9, 2019
10ec018
update snapshots
redallen Apr 9, 2019
c2ea61a
fix tabs passing aria-label to <nav>
redallen Apr 9, 2019
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
2 changes: 1 addition & 1 deletion packages/patternfly-4/react-core/src/components/Alert/Alert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const propTypes = {
};

const defaultProps = {
'aria-label': undefined,
'aria-label': '',
action: null,
children: '',
className: '',
Expand Down
4 changes: 2 additions & 2 deletions packages/patternfly-4/react-core/src/components/Alert/AlertBody.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const propTypes = {
children: PropTypes.node,
className: PropTypes.string,
onClose: PropTypes.func,
'aria-label': PropTypes.string
'aria-label-button': PropTypes.string
Copy link
Copy Markdown
Contributor

@jgiardino jgiardino Apr 10, 2019

Choose a reason for hiding this comment

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

Related to discussion in slack, let's use ariaLabelClose instead.

};

const defaultProps = {
Expand All @@ -21,7 +21,7 @@ const defaultProps = {
'aria-label': 'Close'
};

const AlertBody = ({ title, className, children, onClose, 'aria-label': ariaLabel, ...props }) => (
const AlertBody = ({ title, className, children, onClose, 'aria-label-button': ariaLabel, ...props }) => (
<div {...props} className={className}>
{onClose && (
<Button variant={ButtonVariant.plain} onClick={onClose} aria-label={ariaLabel}>
Expand Down
6 changes: 3 additions & 3 deletions packages/patternfly-4/react-core/src/components/ApplicationLauncher/ApplicationLauncher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const propTypes = {
/** Indicates where menu will be alligned horizontally */
position: PropTypes.oneOf(Object.values(ApplicationLauncherPosition)),
/** Adds accessible text to the button. Required for plain buttons */
'aria-label': PropTypes.string
'aria-label-button': PropTypes.string
Copy link
Copy Markdown
Contributor

@jgiardino jgiardino Apr 10, 2019

Choose a reason for hiding this comment

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

Let's use ariaLabelToggle instead.

};

export const defaultProps = {
Expand All @@ -36,7 +36,7 @@ export const defaultProps = {
onSelect: Function.prototype,
onToggle: Function.prototype,
position: ApplicationLauncherPosition.left,
'aria-label': 'Actions'
'aria-label-button': 'Actions'
};

class ApplicationLauncher extends React.Component {
Expand All @@ -50,7 +50,7 @@ class ApplicationLauncher extends React.Component {

render() {
const {
'aria-label': ariaLabel,
'aria-label-button': ariaLabel,
dropdownItems,
className,
isOpen,
Expand Down
6 changes: 3 additions & 3 deletions packages/patternfly-4/react-core/src/components/ChipGroup/Chip.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const propTypes = {
/** Content rendered inside the chip text */
children: PropTypes.node,
/** Aria Label for close button */
'aria-label': PropTypes.string,
'aria-label-button': PropTypes.string,
Copy link
Copy Markdown
Contributor

@jgiardino jgiardino Apr 10, 2019

Choose a reason for hiding this comment

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

Let's use ariaLabelClose instead.

/** Additional classes added to the chip item */
className: PropTypes.string,
/** Flag indicating if the chip has overflow */
Expand All @@ -24,7 +24,7 @@ const propTypes = {

const defaultProps = {
children: null,
'aria-label': 'close',
'aria-label-button': 'close',
className: '',
isOverflowChip: false,
tooltipPosition: 'top',
Expand Down Expand Up @@ -53,7 +53,7 @@ class Chip extends React.Component {
};

renderChip = randomId => {
const { children, 'aria-label': ariaLabel, tooltipPosition, className, onClick } = this.props;
const { children, 'aria-label-button': ariaLabel, tooltipPosition, className, onClick } = this.props;
if (this.state.isTooltipVisible) {
return (
<Tooltip position={tooltipPosition} content={children}>
Expand Down
6 changes: 3 additions & 3 deletions packages/patternfly-4/react-core/src/components/ContextSelector/ContextSelector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const propTypes = {
/** Text that appears in the Context Selector Toggle */
toggleText: PropTypes.string,
/** aria-label for the Context Selector Search Button */
'aria-label': PropTypes.string,
'aria-label-search': PropTypes.string,
Copy link
Copy Markdown
Contributor

@jgiardino jgiardino Apr 10, 2019

Choose a reason for hiding this comment

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

Let's use ariaLabelSearch instead.

/** Value in the Search field */
searchInputValue: PropTypes.string,
/** Function callback called when user changes the Search Input */
Expand All @@ -51,7 +51,7 @@ const defaultProps = {
onSelect: () => { },
screenReaderLabel: '',
toggleText: '',
'aria-label': 'Search menu items',
'aria-label-search': 'Search menu items',
searchInputValue: '',
onSearchInputChange: () => { },
searchInputPlaceholder: 'Search',
Expand Down Expand Up @@ -79,7 +79,7 @@ class ContextSelector extends React.Component {
onSelect,
screenReaderLabel,
toggleText,
'aria-label': ariaLabel,
'aria-label-search': ariaLabel,
searchInputValue,
onSearchInputChange,
searchInputPlaceholder,
Expand Down
3 changes: 1 addition & 2 deletions packages/patternfly-4/react-core/src/components/DataList/DataList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@ import { css } from '@patternfly/react-styles';
import PropTypes from 'prop-types';
import styles from '@patternfly/patternfly/components/DataList/data-list.css';

const DataList = ({ children, className, 'aria-label': ariaLabel, ...props }) => (
const DataList = ({ children, className, ...props }) => (
<ul
className={css(styles.dataList, className)}
role="list"
aria-label={ariaLabel}
{...props}
>
{children}
Expand Down
6 changes: 0 additions & 6 deletions packages/patternfly-4/react-core/src/components/DataList/DataListAction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@ const propTypes = {
actions: PropTypes.arrayOf(PropTypes.node).isRequired,
/** Identify the DataList toggle number */
id: PropTypes.string.isRequired,
/** Adds accessible text to the DataList item */
'aria-labelledby': PropTypes.string.isRequired,
/** Adds accessible text to the DataList item */
'aria-label': PropTypes.string.isRequired,
/** Additional props are spread to the container <div> */
'': PropTypes.any
};
Expand Down Expand Up @@ -50,8 +46,6 @@ class DataListAction extends React.Component {
actions,
className,
id,
'aria-label': ariaLabel,
'aria-labelledby': ariaLabelledBy,
...props
} = this.props;

Expand Down
8 changes: 4 additions & 4 deletions packages/patternfly-4/react-core/src/components/DataList/DataListToggle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ const propTypes = {
isExpanded: PropTypes.bool,
/** Identify the DataList toggle number */
id: PropTypes.string.isRequired,
/** Adds accessible text to the DataList toggle */
'aria-labelledby': PropTypes.string,
/** Adds accessible text to the DataList toggle */
'aria-label': PropTypes.string,
/** Allows users of some screen readers to shift focus to the controlled element. Should be used when the controlled contents are not adjacent to the toggle that controls them. */
'aria-controls': PropTypes.string,
/** Adds accessible text to the DataList toggle */
'aria-label': PropTypes.string,
/** Adds accessible text to the DataList toggle */
'aria-labelledby': PropTypes.string,
/** Additional props are spread to the container <div> */
'': PropTypes.any
};
Expand Down
8 changes: 4 additions & 4 deletions packages/patternfly-4/react-core/src/components/Page/PageHeader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ const propTypes = {
onNavToggle: PropTypes.func,
/** True if the side nav is shown */
isNavOpen: PropTypes.bool,
/** Callback function to handle the side nav toggle button */
'aria-label': PropTypes.string,
/** aria-label for the button */
'aria-label-button': PropTypes.string,
Copy link
Copy Markdown
Contributor

@jgiardino jgiardino Apr 10, 2019

Choose a reason for hiding this comment

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

Let's use ariaLabelNavToggle instead.

/** Additional props are spread to the container <header> */
'': PropTypes.any
};
Expand All @@ -40,7 +40,7 @@ const defaultProps = {
showNavToggle: false,
isNavOpen: true,
onNavToggle: () => undefined,
'aria-label': 'Global navigation'
'aria-label-button': 'Global navigation'
};

const PageHeader = ({
Expand All @@ -53,7 +53,7 @@ const PageHeader = ({
isNavOpen,
showNavToggle,
onNavToggle,
'aria-label': ariaLabel,
'aria-label-button': ariaLabel,
...props
}) => (
<header role="banner" className={css(styles.pageHeader, className)} {...props}>
Expand Down
2 changes: 1 addition & 1 deletion packages/patternfly-4/react-core/src/components/Select/Select.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const propTypes = {
placeholderText: PropTypes.oneOfType([PropTypes.string, PropTypes.node]),
/** Selected item */
selections: PropTypes.oneOfType([PropTypes.string, PropTypes.arrayOf(PropTypes.string)]),
/** Adds accessible text to Select */
/** Adds accessible text to all Select elements */
'aria-label': PropTypes.string,
/** Id of label for the Select aria-labelledby */
'aria-labelledby': PropTypes.string,
Expand Down