Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion packages/react-core/src/components/Accordion/Accordion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export interface AccordionProps extends React.HTMLProps<HTMLDListElement> {
export const Accordion: React.FunctionComponent<AccordionProps> = ({
children = null,
className = '',
'aria-label': ariaLabel = '',
'aria-label': ariaLabel,
headingLevel = 'h3',
asDefinitionList = true,
isBordered = false,
Expand All @@ -40,6 +40,7 @@ export const Accordion: React.FunctionComponent<AccordionProps> = ({
className
)}
aria-label={ariaLabel}
{...(!asDefinitionList && ariaLabel && { role: 'region' })}
{...props}
>
<AccordionContext.Provider
Expand Down
4 changes: 0 additions & 4 deletions packages/react-core/src/components/Alert/Alert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ export interface AlertProps extends Omit<React.HTMLProps<HTMLDivElement>, 'actio
* or React.Fragment.
*/
actionLinks?: React.ReactNode;
/** Adds accessible text to the alert. */
'aria-label'?: string;
/** Content rendered inside the alert. */
children?: React.ReactNode;
/** Additional classes to add to the alert. */
Expand Down Expand Up @@ -97,7 +95,6 @@ export const Alert: React.FunctionComponent<AlertProps> = ({
isPlain = false,
isLiveRegion = false,
variantLabel = `${capitalize(variant)} alert:`,
'aria-label': ariaLabel = `${capitalize(variant)} Alert`,
actionClose,
actionLinks,
title,
Expand Down Expand Up @@ -229,7 +226,6 @@ export const Alert: React.FunctionComponent<AlertProps> = ({
styles.modifiers[variant as 'success' | 'danger' | 'warning' | 'info'],
className
)}
aria-label={ariaLabel}
{...ouiaProps}
{...(isLiveRegion && {
'aria-live': 'polite',
Expand Down
58 changes: 33 additions & 25 deletions packages/react-core/src/components/Alert/__tests__/Alert.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,9 @@ test('Renders with class pf-c-alert__title on the div containing the title', ()
expect(screen.getByRole('heading', { name: 'Default alert: Some title' })).toHaveClass('pf-c-alert__title');
});

test('Renders with a default aria label of Default Alert', () => {
render(
<Alert title="Some title" data-testid="Alert-test-id">
Some alert
</Alert>
);
expect(screen.getByTestId('Alert-test-id')).toHaveAccessibleName('Default Alert');
test('Renders with default hidden text of "Default alert:"', () => {
render(<Alert title="Some title">Some alert</Alert>);
expect(screen.getByText('Default alert:')).toBeInTheDocument();
});

['success', 'danger', 'warning', 'info'].forEach(variant => {
Expand All @@ -76,13 +72,13 @@ test('Renders with a default aria label of Default Alert', () => {
expect(screen.getByTestId('Alert-test-id')).toHaveClass(`pf-m-${variant}`);
});

test(`Renders with aria label ${capitalize(variant)} Alert when variant = ${variant}`, () => {
test(`Renders with hidden text "${capitalize(variant)} alert:" when variant = ${variant}`, () => {
render(
<Alert variant={`${variant as AlertVariant}`} title="Some title" data-testid="Alert-test-id">
Some alert
</Alert>
);
expect(screen.getByTestId('Alert-test-id')).toHaveAccessibleName(`${capitalize(variant)} Alert`);
expect(screen.getByText(`${capitalize(variant)} alert:`)).toBeInTheDocument();
});

test(`Renders the title with an accessible name of '${capitalize(
Expand Down Expand Up @@ -136,31 +132,39 @@ test('Renders with a default aria label of Default Alert', () => {
});

test('Does not render with class pf-m-inline by default', () => {
render(<Alert title="Some title">Some alert</Alert>);
expect(screen.getByLabelText('Default Alert')).not.toHaveClass('pf-m-inline');
render(
<Alert title="Some title" data-testid="Alert-test-id">
Some alert
</Alert>
);
expect(screen.getByTestId('Alert-test-id')).not.toHaveClass('pf-m-inline');
});

test('Renders with class pf-m-inline when isInline = true', () => {
render(
<Alert isInline title="Some title">
<Alert isInline title="Some title" data-testid="Alert-test-id">
Some alert
</Alert>
);
expect(screen.getByLabelText('Default Alert')).toHaveClass('pf-m-inline');
expect(screen.getByTestId('Alert-test-id')).toHaveClass('pf-m-inline');
});

test('Does not render with class pf-m-plain by default', () => {
render(<Alert title="Some title">Some alert</Alert>);
expect(screen.getByLabelText('Default Alert')).not.toHaveClass('pf-m-plain');
render(
<Alert title="Some title" data-testid="Alert-test-id">
Some alert
</Alert>
);
expect(screen.getByTestId('Alert-test-id')).not.toHaveClass('pf-m-plain');
});

test('Renders with class pf-m-plain when isPlain = true', () => {
render(
<Alert isPlain title="Some title">
<Alert isPlain title="Some title" data-testid="Alert-test-id">
Some alert
</Alert>
);
expect(screen.getByLabelText('Default Alert')).toHaveClass('pf-m-plain');
expect(screen.getByTestId('Alert-test-id')).toHaveClass('pf-m-plain');
});

test('Renders the title', () => {
Expand Down Expand Up @@ -588,17 +592,21 @@ test('Passes customIcon value to AlertIcon', () => {
});

test('Does not render with class pf-m-expandable by default', () => {
render(<Alert title="Some title">Some alert</Alert>);
expect(screen.getByLabelText('Default Alert')).not.toHaveClass('pf-m-expandable');
render(
<Alert title="Some title" data-testid="Alert-test-id">
Some alert
</Alert>
);
expect(screen.getByTestId('Alert-test-id')).not.toHaveClass('pf-m-expandable');
});

test('Renders with class pf-m-expandable when isExpandable = true', () => {
render(
<Alert isExpandable title="Some title">
<Alert isExpandable title="Some title" data-testid="Alert-test-id">
Some alert
</Alert>
);
expect(screen.getByLabelText('Default Alert')).toHaveClass('pf-m-expandable');
expect(screen.getByTestId('Alert-test-id')).toHaveClass('pf-m-expandable');
});

test('Renders AlertToggleExpandButton inside pf-c-alert__toggle', () => {
Expand All @@ -613,26 +621,26 @@ test('Renders AlertToggleExpandButton inside pf-c-alert__toggle', () => {

test('Does not render with class pf-m-expanded when AlertToggleExpandButton has not been clicked', () => {
render(
<Alert isExpandable title="Some title">
<Alert isExpandable title="Some title" data-testid="Alert-test-id">
Some alert
</Alert>
);

expect(screen.getByLabelText('Default Alert')).not.toHaveClass('pf-m-expanded');
expect(screen.getByTestId('Alert-test-id')).not.toHaveClass('pf-m-expanded');
});

test('Renders with class pf-m-expanded once the AlertToggleExpandButton is clicked', async () => {
const user = userEvent.setup();

render(
<Alert isExpandable title="Some title">
<Alert isExpandable title="Some title" data-testid="Alert-test-id">
Some alert
</Alert>
);

await user.click(screen.getByRole('button'));

expect(screen.getByLabelText('Default Alert')).toHaveClass('pf-m-expanded');
expect(screen.getByTestId('Alert-test-id')).toHaveClass('pf-m-expanded');
});

test('Does not render children when isExpandable = true and AlertToggleExpandButton has not been clicked', () => {
Expand Down
4 changes: 0 additions & 4 deletions packages/react-core/src/components/Menu/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ export interface MenuProps extends Omit<React.HTMLAttributes<HTMLDivElement>, 'r
event: React.FormEvent<HTMLInputElement> | React.SyntheticEvent<HTMLButtonElement>,
value: string
) => void;
/** Accessibility label */
'aria-label'?: string;
/** @beta Indicates if menu contains a flyout menu */
containsFlyout?: boolean;
/** @beta Indicating that the menu should have nav flyout styling */
Expand Down Expand Up @@ -250,7 +248,6 @@ class MenuBase extends React.Component<MenuProps, MenuState> {

render() {
const {
'aria-label': ariaLabel,
id,
children,
className,
Expand Down Expand Up @@ -339,7 +336,6 @@ class MenuBase extends React.Component<MenuProps, MenuState> {
_isMenuDrilledIn && styles.modifiers.drilledIn,
className
)}
aria-label={ariaLabel}
ref={this.menuRef}
{...getOUIAProps(Menu.displayName, ouiaId !== undefined ? ouiaId : this.state.ouiaStateId, ouiaSafe)}
{...props}
Expand Down
6 changes: 4 additions & 2 deletions packages/react-core/src/components/Menu/MenuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export interface MenuItemProps extends Omit<React.HTMLProps<HTMLLIElement>, 'onC
direction?: 'down' | 'up';
/** @beta True if item is on current selection path */
isOnPath?: boolean;
/** Accessibility label */
/** Adds an accessible name to the menu item. */
'aria-label'?: string;
/** @hide Forwarded ref */
innerRef?: React.Ref<HTMLAnchorElement | HTMLButtonElement>;
Expand Down Expand Up @@ -103,6 +103,7 @@ const MenuItemBase: React.FunctionComponent<MenuItemProps> = ({
isOnPath,
innerRef,
id,
'aria-label': ariaLabel,
...props
}: MenuItemProps) => {
const {
Expand Down Expand Up @@ -312,6 +313,7 @@ const MenuItemBase: React.FunctionComponent<MenuItemProps> = ({
{...(flyoutMenu && { onKeyDown: handleFlyout })}
ref={ref}
role={!hasCheckbox ? 'none' : 'menuitem'}
{...(hasCheckbox && { 'aria-label': ariaLabel })}
{...props}
>
<GenerateId>
Expand All @@ -321,7 +323,7 @@ const MenuItemBase: React.FunctionComponent<MenuItemProps> = ({
tabIndex={-1}
className={css(styles.menuItem, getIsSelected() && !hasCheckbox && styles.modifiers.selected, className)}
aria-current={getAriaCurrent()}
{...(!hasCheckbox && { disabled: isDisabled })}
{...(!hasCheckbox && { disabled: isDisabled, 'aria-label': ariaLabel })}
{...(!hasCheckbox && !flyoutMenu && { role: isSelectMenu ? 'option' : 'menuitem' })}
{...(!hasCheckbox && !flyoutMenu && isSelectMenu && { 'aria-selected': getIsSelected() })}
ref={innerRef}
Expand Down
4 changes: 4 additions & 0 deletions packages/react-core/src/components/Menu/MenuList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ export interface MenuListProps extends React.HTMLProps<HTMLUListElement> {
* for a non-checkbox menu. Only applies when the menu's role is "listbox".
*/
isAriaMultiselectable?: boolean;
/** Adds an accessible name to the menu. */
'aria-label'?: string;
}

export const MenuList: React.FunctionComponent<MenuListProps> = ({
children = null,
className,
isAriaMultiselectable = false,
'aria-label': ariaLabel,
...props
}: MenuListProps) => {
const { role } = React.useContext(MenuContext);
Expand All @@ -27,6 +30,7 @@ export const MenuList: React.FunctionComponent<MenuListProps> = ({
role={role}
{...(role === 'listbox' && { 'aria-multiselectable': isAriaMultiselectable })}
className={css(styles.menuList, className)}
aria-label={ariaLabel}
{...props}
>
{children}
Expand Down
5 changes: 2 additions & 3 deletions packages/react-core/src/components/Page/PageGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export interface PageGroupProps extends React.HTMLProps<HTMLDivElement> {
hasShadowBottom?: boolean;
/** Flag indicating if the PageGroup has a scrolling overflow */
hasOverflowScroll?: boolean;
/** Adds an accessible name to the page group. Required when the hasOverflowScroll prop is set to true. */
/** Adds an accessible name to the page group when the hasOverflowScroll prop is set to true. */
'aria-label'?: string;
}

Expand Down Expand Up @@ -57,8 +57,7 @@ export const PageGroup = ({
hasOverflowScroll && styles.modifiers.overflowScroll,
className
)}
{...(hasOverflowScroll && { tabIndex: 0 })}
aria-label={ariaLabel}
{...(hasOverflowScroll && { tabIndex: 0, role: 'region', 'aria-label': ariaLabel })}
>
{children}
</div>
Expand Down
5 changes: 2 additions & 3 deletions packages/react-core/src/components/Page/PageNavigation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export interface PageNavigationProps extends React.HTMLProps<HTMLDivElement> {
hasShadowBottom?: boolean;
/** Flag indicating if the PageNavigation has a scrolling overflow */
hasOverflowScroll?: boolean;
/** Adds an accessible name to the page navigation. Required when the hasOverflowScroll prop is set to true. */
/** Adds an accessible name to the page navigation when the hasOverflowScroll prop is set to true. */
'aria-label'?: string;
}

Expand Down Expand Up @@ -61,8 +61,7 @@ export const PageNavigation = ({
hasOverflowScroll && styles.modifiers.overflowScroll,
className
)}
{...(hasOverflowScroll && { tabIndex: 0 })}
aria-label={ariaLabel}
{...(hasOverflowScroll && { tabIndex: 0, role: 'region', 'aria-label': ariaLabel })}
{...props}
>
{isWidthLimited && <div className={css(styles.pageMainBody)}>{children}</div>}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,12 @@ describe('Page', () => {
tertiaryNav={nav}
isBreadcrumbGrouped
isTertiaryNavGrouped
groupProps={{ stickyOnBreakpoint: { default: 'bottom' }, hasShadowTop: true, 'aria-label': 'test' }}
groupProps={{
stickyOnBreakpoint: { default: 'bottom' },
hasShadowTop: true,
hasOverflowScroll: true,
'aria-label': 'test'
}}
>
<PageSection variant="default">Section with default background</PageSection>
<PageSection variant="light">Section with light background</PageSection>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ describe('page group', () => {
});

test('Renders with the passed aria-label applied', () => {
render(<PageGroup aria-label="Test label">test</PageGroup>);
render(
<PageGroup aria-label="Test label" hasOverflowScroll>
test
</PageGroup>
);

expect(screen.getByText('test')).toHaveAccessibleName('Test label');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@ describe('page navigation', () => {
});

test('Renders with the passed aria-label applied', () => {
render(<PageNavigation aria-label="Test label">test</PageNavigation>);
render(
<PageNavigation aria-label="Test label" hasOverflowScroll>
test
</PageNavigation>
);

expect(screen.getByText('test')).toHaveAccessibleName('Test label');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,9 @@ exports[`Page Check page to verify grouped nav and breadcrumb - old / props synt
>
<div
aria-label="test"
class="pf-c-page__main-group pf-m-sticky-bottom pf-m-shadow-top"
class="pf-c-page__main-group pf-m-sticky-bottom pf-m-shadow-top pf-m-overflow-scroll"
role="region"
tabindex="0"
>
<div
class="pf-c-page__main-nav"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ exports[`page group Verify overflow scroll 1`] = `
<DocumentFragment>
<div
class="pf-c-page__main-group pf-m-overflow-scroll"
role="region"
tabindex="0"
>
test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ exports[`page navigation Verify overflow scroll 1`] = `
<DocumentFragment>
<div
class="pf-c-page__main-nav pf-m-overflow-scroll"
role="region"
tabindex="0"
>
test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ export const ProgressStep: React.FunctionComponent<ProgressStepProps> = ({
id={titleId}
ref={stepRef}
{...(popoverRender && { type: 'button' })}
{...(props.id !== undefined && titleId !== undefined && { 'aria-labelledby': `${props.id} ${titleId}` })}
{...(props.id !== undefined &&
titleId !== undefined &&
popoverRender && { 'aria-labelledby': `${props.id} ${titleId}` })}
>
{children}
{popoverRender && popoverRender(stepRef)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,11 @@ test('Does not renders with aria-labelledBy by default on Component element', ()

test('Renders with an accessible name that matches children', () => {
render(
<ProgressStep id="test-id" titleId="title-id">
<ProgressStep popoverRender={stepRef => <div>Popover content</div>} id="test-id" titleId="title-id">
Test
</ProgressStep>
);
expect(screen.getByText('Test')).toHaveAccessibleName('Test');
expect(screen.getByText('Test')).toHaveAccessibleName('Test Popover content');
});

test('Renders with popoverRender element', () => {
Expand Down
Loading