feat(Button): change behaviour of isDisabled prop (#10255)

* feat(Button): isDisabled won't set aria-disabled and pf-m-disabled class on <button> * test(Button): update tests to new isDisabled behaviour * test: update snapshots
patternfly · May 3, 2024 · fe6683f · fe6683f
1 parent 868041f
commit fe6683f
Show file tree

Hide file tree

Showing 9 changed files with 168 additions and 152 deletions.
diff --git a/packages/react-core/src/components/Button/Button.tsx b/packages/react-core/src/components/Button/Button.tsx
@@ -161,13 +161,13 @@ const ButtonBase: React.FunctionComponent<ButtonProps> = ({
     <Component
       {...props}
       {...(isAriaDisabled ? preventedEvents : null)}
-      aria-disabled={isDisabled || isAriaDisabled}
+      aria-disabled={isAriaDisabled || (!isButtonElement && isDisabled)}
       aria-label={ariaLabel}
       className={css(
         styles.button,
         styles.modifiers[variant],
         isBlock && styles.modifiers.block,
-        isDisabled && styles.modifiers.disabled,
+        isDisabled && !isButtonElement && styles.modifiers.disabled,
         isAriaDisabled && styles.modifiers.ariaDisabled,
         isClicked && styles.modifiers.clicked,
         isInline && variant === ButtonVariant.link && styles.modifiers.inline,

diff --git a/packages/react-core/src/components/Button/__tests__/Button.test.tsx b/packages/react-core/src/components/Button/__tests__/Button.test.tsx
@@ -60,9 +60,23 @@ test('Renders with class pf-m-clicked when isClicked = true', () => {
   expect(screen.getByRole('button')).toHaveClass('pf-m-clicked');
 });
 
-test('Renders with class pf-m-disabled when isDisabled = true', () => {
+test('Does not render with class pf-m-disabled by default when isDisabled = true', () => {
   render(<Button isDisabled>Disabled Button</Button>);
-  expect(screen.getByRole('button')).toHaveClass('pf-m-disabled');
+  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">
+      Disabled Anchor Button
+    </Button>
+  );
+  expect(screen.getByText('Disabled Anchor Button')).toHaveClass('pf-m-disabled');
 });
 
 test('Renders with class pf-m-aria-disabled when isAriaDisabled = true', () => {
@@ -208,7 +222,9 @@ test('aria-disabled is set to true and tabIndex to -1 if component is not a butt
       Disabled Anchor Button
     </Button>
   );
-  expect(screen.getByText('Disabled Anchor Button')).toHaveAttribute('tabindex', '-1');
+  const anchor = screen.getByText('Disabled Anchor Button');
+  expect(anchor).toHaveAttribute('tabindex', '-1');
+  expect(anchor).toHaveAttribute('aria-disabled', 'true');
 });
 
 test('setting tab index through props', () => {

diff --git a/packages/react-core/src/components/Button/__tests__/__snapshots__/Button.test.tsx.snap b/packages/react-core/src/components/Button/__tests__/__snapshots__/Button.test.tsx.snap
@@ -6,7 +6,7 @@ exports[`Renders basic button 1`] = `
     aria-disabled="false"
     aria-label="basic button"
     class="pf-v6-c-button pf-m-primary"
-    data-ouia-component-id="OUIA-Generated-Button-primary-28"
+    data-ouia-component-id="OUIA-Generated-Button-primary-30"
     data-ouia-component-type="PF6/Button"
     data-ouia-safe="true"
     type="button"

diff --git a/...re/src/components/DualListSelector/__tests__/__snapshots__/DualListSelector.test.tsx.snap b/...re/src/components/DualListSelector/__tests__/__snapshots__/DualListSelector.test.tsx.snap
@@ -106,9 +106,9 @@ exports[`DualListSelector basic 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Add selected"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-1"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -163,9 +163,9 @@ exports[`DualListSelector basic 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Remove all"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-3"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -192,9 +192,9 @@ exports[`DualListSelector basic 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Remove selected"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-4"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -364,9 +364,9 @@ exports[`DualListSelector basic with disabled controls 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Add selected"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-13"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -393,9 +393,9 @@ exports[`DualListSelector basic with disabled controls 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Add all"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-14"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -422,9 +422,9 @@ exports[`DualListSelector basic with disabled controls 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Remove all"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-15"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -451,9 +451,9 @@ exports[`DualListSelector basic with disabled controls 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Remove selected"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-16"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -634,9 +634,9 @@ exports[`DualListSelector with actions 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Add selected"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-21"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -719,9 +719,9 @@ exports[`DualListSelector with actions 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Remove selected"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-24"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -956,9 +956,9 @@ exports[`DualListSelector with custom status 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Add selected"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-9"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -1013,9 +1013,9 @@ exports[`DualListSelector with custom status 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Remove all"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-11"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -1042,9 +1042,9 @@ exports[`DualListSelector with custom status 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Remove selected"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-12"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -1256,9 +1256,9 @@ exports[`DualListSelector with search inputs 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Add selected"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-5"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -1313,9 +1313,9 @@ exports[`DualListSelector with search inputs 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Remove all"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-7"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -1342,9 +1342,9 @@ exports[`DualListSelector with search inputs 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Remove selected"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-8"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -1633,9 +1633,9 @@ exports[`DualListSelector with tree 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Add selected"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-17"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -1690,9 +1690,9 @@ exports[`DualListSelector with tree 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Remove all"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-19"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -1719,9 +1719,9 @@ exports[`DualListSelector with tree 1`] = `
         class="pf-v6-c-dual-list-selector__controls-item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Remove selected"
-          class="pf-v6-c-button pf-m-plain pf-m-disabled"
+          class="pf-v6-c-button pf-m-plain"
           data-ouia-component-id="OUIA-Generated-Button-plain-20"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"

diff --git a/...ges/react-core/src/components/FileUpload/__tests__/__snapshots__/FileUpload.test.tsx.snap b/...ges/react-core/src/components/FileUpload/__tests__/__snapshots__/FileUpload.test.tsx.snap
@@ -51,8 +51,8 @@ exports[`simple fileupload 1`] = `
           class="pf-v6-c-input-group__item"
         >
           <button
-            aria-disabled="true"
-            class="pf-v6-c-button pf-m-control pf-m-disabled"
+            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"
             data-ouia-safe="true"

diff --git a/...s/react-core/src/components/NumberInput/__tests__/__snapshots__/NumberInput.test.tsx.snap b/...s/react-core/src/components/NumberInput/__tests__/__snapshots__/NumberInput.test.tsx.snap
@@ -12,9 +12,9 @@ exports[`numberInput disables lower threshold 1`] = `
         class="pf-v6-c-input-group__item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Minus"
-          class="pf-v6-c-button pf-m-control pf-m-disabled"
+          class="pf-v6-c-button pf-m-control"
           data-ouia-component-id="OUIA-Generated-Button-control-13"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -155,9 +155,9 @@ exports[`numberInput disables upper threshold 1`] = `
         class="pf-v6-c-input-group__item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Plus"
-          class="pf-v6-c-button pf-m-control pf-m-disabled"
+          class="pf-v6-c-button pf-m-control"
           data-ouia-component-id="OUIA-Generated-Button-control-16"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -576,9 +576,9 @@ exports[`numberInput renders disabled 1`] = `
         class="pf-v6-c-input-group__item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Minus"
-          class="pf-v6-c-button pf-m-control pf-m-disabled"
+          class="pf-v6-c-button pf-m-control"
           data-ouia-component-id="OUIA-Generated-Button-control-11"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"
@@ -627,9 +627,9 @@ exports[`numberInput renders disabled 1`] = `
         class="pf-v6-c-input-group__item"
       >
         <button
-          aria-disabled="true"
+          aria-disabled="false"
           aria-label="Plus"
-          class="pf-v6-c-button pf-m-control pf-m-disabled"
+          class="pf-v6-c-button pf-m-control"
           data-ouia-component-id="OUIA-Generated-Button-control-12"
           data-ouia-component-type="PF6/Button"
           data-ouia-safe="true"