Skip to content
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

fix(Button): updated rendering of aria-disabled attribute #11478

Merged
merged 4 commits into from
Jan 28, 2025
Merged

fix(Button): updated rendering of aria-disabled attribute #11478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1adc2b2
fix(Button): updated rendering of aria-disabled attribute
thatblindgeye Jan 24, 2025
f49269f
Updated other component snapshots
thatblindgeye Jan 24, 2025
e2ecf21
Addressed Austin feedback
thatblindgeye Jan 28, 2025
34c322f
Updated test name to use styles object
thatblindgeye Jan 28, 2025
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: 0 additions & 3 deletions ...act-code-editor/src/components/CodeEditor/__test__/__snapshots__/CodeEditor.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ exports[`Matches snapshot with control buttons enabled 1`] = `
style="display: contents;"
>
<button
aria-disabled="false"
aria-label="Copy code to clipboard"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-1"
Expand Down Expand Up @@ -54,7 +53,6 @@ exports[`Matches snapshot with control buttons enabled 1`] = `
style="display: contents;"
>
<button
aria-disabled="false"
aria-label="Upload code"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-2"
Expand Down Expand Up @@ -85,7 +83,6 @@ exports[`Matches snapshot with control buttons enabled 1`] = `
style="display: contents;"
>
<button
aria-disabled="false"
aria-label="Download code"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-3"
Expand Down
1 change: 0 additions & 1 deletion ...e-editor/src/components/CodeEditor/__test__/__snapshots__/CodeEditorControl.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ exports[`Matches snapshot 1`] = `
style="display: contents;"
>
<button
aria-disabled="false"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-1"
data-ouia-component-type="PF6/Button"
Expand Down
3 changes: 0 additions & 3 deletions .../src/components/AboutModal/__tests__/__snapshots__/AboutModalBoxCloseButton.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ exports[`AboutModalBoxCloseButton Test 1`] = `
class="pf-v6-c-about-modal-box__close"
>
<button
aria-disabled="false"
aria-label="Close Dialog"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-1"
Expand Down Expand Up @@ -42,7 +41,6 @@ exports[`AboutModalBoxCloseButton Test close button aria label 1`] = `
class="pf-v6-c-about-modal-box__close"
>
<button
aria-disabled="false"
aria-label="Klose Daylok"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-3"
Expand Down Expand Up @@ -78,7 +76,6 @@ exports[`AboutModalBoxCloseButton Test onclose 1`] = `
class="pf-v6-c-about-modal-box__close"
>
<button
aria-disabled="false"
aria-label="Close Dialog"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-2"
Expand Down
1 change: 0 additions & 1 deletion ...c/components/Alert/__tests__/Generated/__snapshots__/AlertActionCloseButton.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
exports[`AlertActionCloseButton should match snapshot 1`] = `
<DocumentFragment>
<button
aria-disabled="false"
aria-label="Close some label alert: test title"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-1"
Expand Down
1 change: 0 additions & 1 deletion ...core/src/components/Alert/__tests__/Generated/__snapshots__/AlertActionLink.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
exports[`AlertActionLink should match snapshot (auto-generated) 1`] = `
<DocumentFragment>
<button
aria-disabled="false"
class="pf-v6-c-button pf-m-link pf-m-inline ''"
data-ouia-component-id="OUIA-Generated-Button-link-1"
data-ouia-component-type="PF6/Button"
Expand Down
4 changes: 3 additions & 1 deletion packages/react-core/src/components/Button/Button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,14 @@ const ButtonBase: React.FunctionComponent<ButtonProps> = ({
</span>
);
const _children = children && <span className={css('pf-v6-c-button__text')}>{children}</span>;
// We only want to render the aria-disabled attribute when true, similar to the disabled attribute natively.
const shouldRenderAriaDisabled = isAriaDisabled || (!isButtonElement && isDisabled);

return (
<Component
{...props}
{...(isAriaDisabled ? preventedEvents : null)}
aria-disabled={isAriaDisabled || (!isButtonElement && isDisabled)}
{...(shouldRenderAriaDisabled && { 'aria-disabled': true })}
aria-label={ariaLabel}
className={css(
styles.button,
Expand Down
31 changes: 22 additions & 9 deletions packages/react-core/src/components/Button/__tests__/Button.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import React from 'react';

import { render, screen } from '@testing-library/react';

import { Button, ButtonState, ButtonVariant } from '../Button';
import styles from '@patternfly/react-styles/css/components/Button/button';

Object.values(ButtonVariant).forEach((variant) => {
if (variant !== 'primary') {
Expand Down Expand Up @@ -65,11 +64,6 @@ test('Does not render with class pf-m-disabled by default when isDisabled = true
expect(screen.getByRole('button')).not.toHaveClass('pf-m-disabled');
});

test('aria-disabled is set to false when isDisabled = true', () => {
render(<Button isDisabled>Disabled Button</Button>);
expect(screen.getByRole('button')).toHaveAttribute('aria-disabled', 'false');
});

test('Renders with class pf-m-disabled when isDisabled = true and component is not a button', () => {
render(
<Button isDisabled component="a">
Expand All @@ -79,9 +73,28 @@ test('Renders with class pf-m-disabled when isDisabled = true and component is n
expect(screen.getByText('Disabled Anchor Button').parentElement).toHaveClass('pf-m-disabled');
});

test('Renders with class pf-m-aria-disabled when isAriaDisabled = true', () => {
test(`aria-disabled and class ${styles.modifiers.ariaDisabled} are not rendered when isAriaDisabled is not passed by default`, () => {
render(<Button>Button</Button>);

const button = screen.getByRole('button');
expect(button).not.toHaveAttribute('aria-disabled');
expect(button).not.toHaveClass(styles.modifiers.ariaDisabled);
});

test(`aria-disabled and class ${styles.modifiers.ariaDisabled} are not rendered when isDisabled is true, but isAriaDisabled is not passed`, () => {
render(<Button isDisabled>Disabled Button</Button>);

const button = screen.getByRole('button');
expect(button).not.toHaveAttribute('aria-disabled');
expect(button).not.toHaveClass(styles.modifiers.ariaDisabled);
});

test(`Renders with class ${styles.modifiers.ariaDisabled} and aria-disabled attribute when isAriaDisabled = true`, () => {
render(<Button isAriaDisabled>Disabled yet focusable button</Button>);
expect(screen.getByRole('button')).toHaveClass('pf-m-aria-disabled');
const button = screen.getByRole('button');

expect(button).toHaveAttribute('aria-disabled', 'true');
expect(button).toHaveClass(styles.modifiers.ariaDisabled);
});

test('Does not disable button when isDisabled = true and component = a', () => {
Expand Down
3 changes: 1 addition & 2 deletions packages/react-core/src/components/Button/__tests__/__snapshots__/Button.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@
exports[`Renders basic button 1`] = `
<DocumentFragment>
<button
aria-disabled="false"
aria-label="basic button"
class="pf-v6-c-button pf-m-primary"
data-ouia-component-id="OUIA-Generated-Button-primary-30"
data-ouia-component-id="OUIA-Generated-Button-primary-31"
data-ouia-component-type="PF6/Button"
data-ouia-safe="true"
type="button"
Expand Down
1 change: 0 additions & 1 deletion packages/react-core/src/components/Card/__tests__/__snapshots__/CardHeader.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ exports[`CardHeader onExpand adds the toggle button 1`] = `
class="pf-v6-c-card__header-toggle"
>
<button
aria-disabled="false"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-1"
data-ouia-component-type="PF6/Button"
Expand Down
1 change: 0 additions & 1 deletion ...re/src/components/ClipboardCopy/__tests__/__snapshots__/ClipboardCopyButton.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ exports[`Matches snapshot 1`] = `
onTooltipHidden clicker
</a>
<button
aria-disabled="false"
aria-label="Copyable input"
aria-labelledby="button-id text-id"
class="pf-v6-c-button pf-m-control"
Expand Down
1 change: 0 additions & 1 deletion ...re/src/components/ClipboardCopy/__tests__/__snapshots__/ClipboardCopyToggle.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ exports[`Matches snapshot 1`] = `
<DocumentFragment>
<button
aria-controls="content-id"
aria-disabled="false"
aria-expanded="false"
aria-labelledby="main-id text-id"
class="pf-v6-c-button pf-m-control"
Expand Down
1 change: 0 additions & 1 deletion ...re/src/components/DataList/__tests__/Generated/__snapshots__/DataListToggle.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ exports[`DataListToggle should match snapshot (auto-generated) 1`] = `
>
<button
aria-controls="''"
aria-disabled="false"
aria-expanded="false"
aria-label="'Details'"
class="pf-v6-c-button pf-m-plain"
Expand Down
1 change: 0 additions & 1 deletion ...s/react-core/src/components/DataList/__tests__/__snapshots__/DataListToggle.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ exports[`Renders to match snapshot 1`] = `
>
<button
aria-controls="false"
aria-disabled="false"
aria-expanded="false"
aria-label="Details"
aria-labelledby=" ex-toggle2"
Expand Down
4 changes: 0 additions & 4 deletions ...ges/react-core/src/components/DatePicker/__tests__/__snapshots__/DatePicker.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ exports[`With popover opened 1`] = `
class="pf-v6-c-input-group__item"
>
<button
aria-disabled="false"
aria-haspopup="dialog"
aria-label="Toggle date picker"
class="pf-v6-c-button pf-m-control"
Expand Down Expand Up @@ -95,7 +94,6 @@ exports[`With popover opened 1`] = `
class="pf-v6-c-calendar-month__header-nav-control pf-m-prev-month"
>
<button
aria-disabled="false"
aria-label="Previous month"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-1"
Expand Down Expand Up @@ -201,7 +199,6 @@ exports[`With popover opened 1`] = `
class="pf-v6-c-calendar-month__header-nav-control pf-m-next-month"
>
<button
aria-disabled="false"
aria-label="Next month"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-2"
Expand Down Expand Up @@ -829,7 +826,6 @@ exports[`disabled date picker 1`] = `
class="pf-v6-c-input-group__item"
>
<button
aria-disabled="false"
aria-haspopup="dialog"
aria-label="Toggle date picker"
class="pf-v6-c-button pf-m-control"
Expand Down
6 changes: 0 additions & 6 deletions packages/react-core/src/components/Drawer/__tests__/__snapshots__/Drawer.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ exports[`Drawer expands from bottom 1`] = `
class="pf-v6-c-drawer__close"
>
<button
aria-disabled="false"
aria-label="Close drawer panel"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-4"
Expand Down Expand Up @@ -127,7 +126,6 @@ exports[`Drawer has resizable callback and id 1`] = `
class="pf-v6-c-drawer__close"
>
<button
aria-disabled="false"
aria-label="Close drawer panel"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-6"
Expand Down Expand Up @@ -222,7 +220,6 @@ exports[`Drawer has resizable css and color variants 1`] = `
class="pf-v6-c-drawer__close"
>
<button
aria-disabled="false"
aria-label="Close drawer panel"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-5"
Expand Down Expand Up @@ -351,7 +348,6 @@ exports[`Drawer isExpanded = true and isInline = false and isStatic = false 1`]
class="pf-v6-c-drawer__close"
>
<button
aria-disabled="false"
aria-label="Close drawer panel"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-1"
Expand Down Expand Up @@ -425,7 +421,6 @@ exports[`Drawer isExpanded = true and isInline = false and isStatic = true 1`] =
class="pf-v6-c-drawer__close"
>
<button
aria-disabled="false"
aria-label="Close drawer panel"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-3"
Expand Down Expand Up @@ -499,7 +494,6 @@ exports[`Drawer isExpanded = true and isInline = true and isStatic = false 1`] =
class="pf-v6-c-drawer__close"
>
<button
aria-disabled="false"
aria-label="Close drawer panel"
class="pf-v6-c-button pf-m-plain"
data-ouia-component-id="OUIA-Generated-Button-plain-2"
Expand Down
2 changes: 0 additions & 2 deletions ...ges/react-core/src/components/EmptyState/__tests__/__snapshots__/EmptyState.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ exports[`EmptyState Main 1`] = `
class="pf-v6-c-empty-state__actions"
>
<button
aria-disabled="false"
class="pf-v6-c-button pf-m-primary"
data-ouia-component-id="OUIA-Generated-Button-primary-1"
data-ouia-component-type="PF6/Button"
Expand All @@ -112,7 +111,6 @@ exports[`EmptyState Main 1`] = `
class="pf-v6-c-empty-state__actions"
>
<button
aria-disabled="false"
aria-label="learn more action"
class="pf-v6-c-button pf-m-link"
data-ouia-component-id="OUIA-Generated-Button-link-1"
Expand Down
6 changes: 0 additions & 6 deletions .../src/components/ExpandableSection/__tests__/__snapshots__/ExpandableSection.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ exports[`Detached ExpandableSection renders successfully 1`] = `
>
<button
aria-controls="content-id"
aria-disabled="false"
aria-expanded="true"
class="pf-v6-c-button pf-m-link"
data-ouia-component-id="OUIA-Generated-Button-link-5"
Expand Down Expand Up @@ -73,7 +72,6 @@ exports[`Disclosure ExpandableSection 1`] = `
>
<button
aria-controls="content-id"
aria-disabled="false"
class="pf-v6-c-button pf-m-link"
data-ouia-component-id="OUIA-Generated-Button-link-6"
data-ouia-component-type="PF6/Button"
Expand Down Expand Up @@ -127,7 +125,6 @@ exports[`ExpandableSection 1`] = `
>
<button
aria-controls="content-id"
aria-disabled="false"
class="pf-v6-c-button pf-m-link"
data-ouia-component-id="OUIA-Generated-Button-link-1"
data-ouia-component-type="PF6/Button"
Expand Down Expand Up @@ -181,7 +178,6 @@ exports[`Renders ExpandableSection expanded 1`] = `
>
<button
aria-controls="content-id"
aria-disabled="false"
aria-expanded="true"
class="pf-v6-c-button pf-m-link"
data-ouia-component-id="OUIA-Generated-Button-link-2"
Expand Down Expand Up @@ -235,7 +231,6 @@ exports[`Renders ExpandableSection indented 1`] = `
>
<button
aria-controls="content-id"
aria-disabled="false"
aria-expanded="true"
class="pf-v6-c-button pf-m-link"
data-ouia-component-id="OUIA-Generated-Button-link-7"
Expand Down Expand Up @@ -289,7 +284,6 @@ exports[`Renders Uncontrolled ExpandableSection 1`] = `
>
<button
aria-controls="content-id"
aria-disabled="false"
class="pf-v6-c-button pf-m-link"
data-ouia-component-id="OUIA-Generated-Button-link-4"
data-ouia-component-type="PF6/Button"
Expand Down
2 changes: 0 additions & 2 deletions ...ges/react-core/src/components/FileUpload/__tests__/__snapshots__/FileUpload.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ exports[`simple fileupload 1`] = `
class="pf-v6-c-input-group__item"
>
<button
aria-disabled="false"
class="pf-v6-c-button pf-m-control"
data-ouia-component-id="OUIA-Generated-Button-control-1"
data-ouia-component-type="PF6/Button"
Expand All @@ -55,7 +54,6 @@ exports[`simple fileupload 1`] = `
class="pf-v6-c-input-group__item"
>
<button
aria-disabled="false"
class="pf-v6-c-button pf-m-control"
data-ouia-component-id="OUIA-Generated-Button-control-2"
data-ouia-component-type="PF6/Button"
Expand Down
2 changes: 0 additions & 2 deletions ...eact-core/src/components/FileUpload/__tests__/__snapshots__/FileUploadField.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ exports[`simple fileuploadfield 1`] = `
class="pf-v6-c-input-group__item"
>
<button
aria-disabled="false"
class="pf-v6-c-button pf-m-control"
data-ouia-component-id="OUIA-Generated-Button-control-1"
data-ouia-component-type="PF6/Button"
Expand All @@ -54,7 +53,6 @@ exports[`simple fileuploadfield 1`] = `
class="pf-v6-c-input-group__item"
>
<button
aria-disabled="false"
class="pf-v6-c-button pf-m-control"
data-ouia-component-id="OUIA-Generated-Button-control-2"
data-ouia-component-type="PF6/Button"
Expand Down
3 changes: 0 additions & 3 deletions packages/react-core/src/components/Form/__tests__/__snapshots__/FormFieldGroup.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ exports[`Check expandable form field group example against snapshot 1`] = `
class="pf-v6-c-form__field-group-toggle-button"
>
<button
aria-disabled="false"
aria-expanded="true"
aria-label="toggle"
aria-labelledby="title-text-id2 generated-id"
Expand Down Expand Up @@ -75,7 +74,6 @@ exports[`Check expandable form field group example against snapshot 1`] = `
class="pf-v6-c-form__field-group-header-actions"
>
<button
aria-disabled="false"
class="pf-v6-c-button pf-m-primary"
data-ouia-component-id="OUIA-Generated-Button-primary-2"
data-ouia-component-type="PF6/Button"
Expand Down Expand Up @@ -124,7 +122,6 @@ exports[`Check form field group example against snapshot 1`] = `
class="pf-v6-c-form__field-group-header-actions"
>
<button
aria-disabled="false"
class="pf-v6-c-button pf-m-primary"
data-ouia-component-id="OUIA-Generated-Button-primary-1"
data-ouia-component-type="PF6/Button"
Expand Down
Loading
Loading