From 1adc2b2436953351dfb2cf09bd9eea19be450b08 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Fri, 24 Jan 2025 11:49:55 -0500 Subject: [PATCH 1/4] fix(Button): updated rendering of aria-disabled attribute --- .../src/components/Button/Button.tsx | 4 ++- .../Button/__tests__/Button.test.tsx | 31 +++++++++++++------ .../__snapshots__/Button.test.tsx.snap | 3 +- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/packages/react-core/src/components/Button/Button.tsx b/packages/react-core/src/components/Button/Button.tsx index 8345113a2dd..1fc153802ea 100644 --- a/packages/react-core/src/components/Button/Button.tsx +++ b/packages/react-core/src/components/Button/Button.tsx @@ -164,12 +164,14 @@ const ButtonBase: React.FunctionComponent = ({ ); const _children = children && {children}; + // We only want to render the aria-disabled attribute when true, similar to the disabled attribute natively. + const shouldRenderAriaDisabled = isAriaDisabled === true || (!isButtonElement && isDisabled === true); return ( { if (variant !== 'primary') { @@ -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(); - 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( ); + + 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(); + + const button = screen.getByRole('button'); + expect(button).not.toHaveAttribute('aria-disabled'); + expect(button).not.toHaveClass(styles.modifiers.ariaDisabled); +}); + +test('Renders with class pf-m-aria-disabled and aria-disabled attribute when isAriaDisabled = true', () => { render(); - expect(screen.getByRole('button')).toHaveClass('pf-m-aria-disabled'); + const button = screen.getByRole('button'); + + expect(button).toHaveAttribute('aria-disabled', 'true'); + expect(button).toHaveClass('pf-m-aria-disabled'); }); test('Does not disable button when isDisabled = true and component = a', () => { 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 index 1bf9bbee899..43774b7acca 100644 --- 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 @@ -3,10 +3,9 @@ exports[`Renders basic button 1`] = ` ); const button = screen.getByRole('button');