From 3aacd2d9278bb16bfe57ec5a4fc33c7e5337cc49 Mon Sep 17 00:00:00 2001 From: Titani Date: Mon, 23 Mar 2026 17:33:47 -0400 Subject: [PATCH 01/13] add logic to check if glass theme is on --- .../src/components/Accordion/Accordion.tsx | 5 ++ .../Accordion/__tests__/Accordion.test.tsx | 48 ++++++++++++++++--- packages/react-core/src/helpers/util.ts | 9 ++++ 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/packages/react-core/src/components/Accordion/Accordion.tsx b/packages/react-core/src/components/Accordion/Accordion.tsx index 53096f97c2e..51cd9972765 100644 --- a/packages/react-core/src/components/Accordion/Accordion.tsx +++ b/packages/react-core/src/components/Accordion/Accordion.tsx @@ -1,5 +1,6 @@ import { css } from '@patternfly/react-styles'; import styles from '@patternfly/react-styles/css/components/Accordion/accordion'; +import { hasGlassTheme } from '../../helpers/util'; import { AccordionContext } from './AccordionContext'; export interface AccordionProps extends React.HTMLProps { @@ -15,6 +16,8 @@ export interface AccordionProps extends React.HTMLProps { asDefinitionList?: boolean; /** Flag to indicate the accordion had a border */ isBordered?: boolean; + /** Flag to indicate if the accordion is plain */ + isPlain?: boolean; /** Display size variant. */ displaySize?: 'default' | 'lg'; /** Sets the toggle icon position for all accordion toggles. */ @@ -28,6 +31,7 @@ export const Accordion: React.FunctionComponent = ({ headingLevel = 'h3', asDefinitionList = true, isBordered = false, + isPlain = false, displaySize = 'default', togglePosition = 'end', ...props @@ -38,6 +42,7 @@ export const Accordion: React.FunctionComponent = ({ className={css( styles.accordion, isBordered && styles.modifiers.bordered, + !isPlain && hasGlassTheme() && styles.modifiers.noPlain, togglePosition === 'start' && styles.modifiers.toggleStart, displaySize === 'lg' && styles.modifiers.displayLg, className diff --git a/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx b/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx index c6260e3f8cd..78fbad14653 100644 --- a/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx +++ b/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx @@ -1,7 +1,11 @@ +import '@testing-library/jest-dom'; +// eslint-disable-next-line no-restricted-imports -- React in scope required for TS (test file) +import React from 'react'; import { render, screen } from '@testing-library/react'; import { Accordion } from '../Accordion'; import { AccordionContext } from '../AccordionContext'; +// @ts-ignore - react-styles subpath module resolution import styles from '@patternfly/react-styles/css/components/Accordion/accordion'; test('Renders without children', () => { @@ -33,7 +37,7 @@ test('Renders with inherited element props spread to the component', () => { expect(screen.getByText('Test')).toHaveAccessibleName('Label'); }); -test(`Renders with class name ${styles.accordion}`, () => { +test(`Renders with class ${styles.accordion}`, () => { render(Test); expect(screen.getByText('Test')).toHaveClass(styles.accordion); @@ -62,7 +66,7 @@ test('Renders Accordion as a "div" when asDefinitionList is false', () => { test('Provides a ContentContainer of "dd" in a context by default', () => { render( - {({ ContentContainer }) => ContentContainer} + {({ ContentContainer }) => String(ContentContainer)} ); @@ -72,7 +76,7 @@ test('Provides a ContentContainer of "dd" in a context by default', () => { test('Provides a ContentContainer of "div" in a context when asDefinitionList is false', () => { render( - {({ ContentContainer }) => ContentContainer} + {({ ContentContainer }) => String(ContentContainer)} ); @@ -82,7 +86,7 @@ test('Provides a ContentContainer of "div" in a context when asDefinitionList is test('Provides a ToggleContainer of "dt" in a context by default', () => { render( - {({ ToggleContainer }) => ToggleContainer} + {({ ToggleContainer }) => String(ToggleContainer)} ); @@ -92,7 +96,7 @@ test('Provides a ToggleContainer of "dt" in a context by default', () => { test('Provides a ToggleContainer of "h3" in a context when asDefinitionList is false', () => { render( - {({ ToggleContainer }) => ToggleContainer} + {({ ToggleContainer }) => String(ToggleContainer)} ); @@ -102,7 +106,7 @@ test('Provides a ToggleContainer of "h3" in a context when asDefinitionList is f test('Provides a ToggleContainer of "h2" in a context when asDefinitionList is false and headingLevel is "h2"', () => { render( - {({ ToggleContainer }) => ToggleContainer} + {({ ToggleContainer }) => String(ToggleContainer)} ); @@ -121,6 +125,38 @@ test('Renders with pf-m-bordered when isBordered=true', () => { expect(screen.getByText('Test')).toHaveClass('pf-m-bordered'); }); +test(`Renders without class ${styles.modifiers.noPlain} by default`, () => { + render(Test); + + expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.noPlain); +}); + +test(`Renders without class ${styles.modifiers.noPlain} when isPlain is undefined`, () => { + render(Test); + + expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.noPlain); +}); + +test(`Renders without class ${styles.modifiers.noPlain} when isPlain=false and glass theme is not applied`, () => { + render(Test); + + expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.noPlain); +}); + +test(`Renders with class ${styles.modifiers.noPlain} when isPlain=false and glass theme is applied`, () => { + document.documentElement.classList.add('pf-v6-theme-glass'); + render(Test); + + expect(screen.getByText('Test')).toHaveClass(styles.modifiers.noPlain); + document.documentElement.classList.remove('pf-v6-theme-glass'); +}); + +test(`Renders without class ${styles.modifiers.noPlain} when isPlain=true`, () => { + render(Test); + + expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.noPlain); +}); + test('Renders without pf-m-display-lg by default', () => { render(Test); diff --git a/packages/react-core/src/helpers/util.ts b/packages/react-core/src/helpers/util.ts index ac63bbaeade..9dd32f7c0d2 100644 --- a/packages/react-core/src/helpers/util.ts +++ b/packages/react-core/src/helpers/util.ts @@ -395,6 +395,15 @@ export const toCamel = (s: string) => s.replace(/([-_][a-z])/gi, camelize); */ export const canUseDOM = !!(typeof window !== 'undefined' && window.document && window.document.createElement); +/** + * Checks if the PatternFly glass theme is applied at the document root. + * Used by components that adapt styling for glass theme (e.g. Accordion). + * + * @returns {boolean} - True if the glass theme class is present on the html element + */ +export const hasGlassTheme = (): boolean => + typeof document !== 'undefined' && document.documentElement.classList.contains('pf-v6-theme-glass'); + /** * Calculate the width of the text * Example: From 5eb7016ed1446462c2a81a0c0370c11b44f67bb1 Mon Sep 17 00:00:00 2001 From: Titani Date: Tue, 24 Mar 2026 08:53:02 -0400 Subject: [PATCH 02/13] fix build issue --- packages/react-core/src/helpers/util.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/react-core/src/helpers/util.ts b/packages/react-core/src/helpers/util.ts index 9dd32f7c0d2..24312a56657 100644 --- a/packages/react-core/src/helpers/util.ts +++ b/packages/react-core/src/helpers/util.ts @@ -401,8 +401,13 @@ export const canUseDOM = !!(typeof window !== 'undefined' && window.document && * * @returns {boolean} - True if the glass theme class is present on the html element */ -export const hasGlassTheme = (): boolean => - typeof document !== 'undefined' && document.documentElement.classList.contains('pf-v6-theme-glass'); +export const hasGlassTheme = (): boolean => { + if (typeof document === 'undefined') { + return false; + } + const classList = document.documentElement?.classList; + return classList ? classList.contains('pf-v6-theme-glass') : false; +}; /** * Calculate the width of the text From ca4a3e71905960b6e8f48c4a131f3782660ee23f Mon Sep 17 00:00:00 2001 From: Titani Date: Tue, 24 Mar 2026 09:03:39 -0400 Subject: [PATCH 03/13] fix test file --- .../src/components/Accordion/__tests__/Accordion.test.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx b/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx index 78fbad14653..0486403c047 100644 --- a/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx +++ b/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx @@ -1,11 +1,9 @@ import '@testing-library/jest-dom'; -// eslint-disable-next-line no-restricted-imports -- React in scope required for TS (test file) -import React from 'react'; +import { Fragment } from 'react'; import { render, screen } from '@testing-library/react'; import { Accordion } from '../Accordion'; import { AccordionContext } from '../AccordionContext'; -// @ts-ignore - react-styles subpath module resolution import styles from '@patternfly/react-styles/css/components/Accordion/accordion'; test('Renders without children', () => { @@ -28,10 +26,10 @@ test('Renders with the passed aria label', () => { test('Renders with inherited element props spread to the component', () => { render( - <> + Test

Label

- + ); expect(screen.getByText('Test')).toHaveAccessibleName('Label'); From 3aa1c5cf91f422c3de4fab1603f251193e992d85 Mon Sep 17 00:00:00 2001 From: Titani Date: Tue, 24 Mar 2026 10:59:18 -0400 Subject: [PATCH 04/13] fix test based of coderabbitai comment --- .../components/Accordion/__tests__/Accordion.test.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx b/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx index 0486403c047..23de19ad548 100644 --- a/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx +++ b/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx @@ -143,10 +143,13 @@ test(`Renders without class ${styles.modifiers.noPlain} when isPlain=false and g test(`Renders with class ${styles.modifiers.noPlain} when isPlain=false and glass theme is applied`, () => { document.documentElement.classList.add('pf-v6-theme-glass'); - render(Test); + try { + render(Test); - expect(screen.getByText('Test')).toHaveClass(styles.modifiers.noPlain); - document.documentElement.classList.remove('pf-v6-theme-glass'); + expect(screen.getByText('Test')).toHaveClass(styles.modifiers.noPlain); + } finally { + document.documentElement.classList.remove('pf-v6-theme-glass'); + } }); test(`Renders without class ${styles.modifiers.noPlain} when isPlain=true`, () => { From 0a48e8b49ad7ca21d681da945d125a55b3043e41 Mon Sep 17 00:00:00 2001 From: Titani Date: Tue, 24 Mar 2026 11:09:27 -0400 Subject: [PATCH 05/13] update isPlain prop comment --- packages/react-core/src/components/Accordion/Accordion.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/react-core/src/components/Accordion/Accordion.tsx b/packages/react-core/src/components/Accordion/Accordion.tsx index 51cd9972765..99b486d4ba4 100644 --- a/packages/react-core/src/components/Accordion/Accordion.tsx +++ b/packages/react-core/src/components/Accordion/Accordion.tsx @@ -16,7 +16,11 @@ export interface AccordionProps extends React.HTMLProps { asDefinitionList?: boolean; /** Flag to indicate the accordion had a border */ isBordered?: boolean; - /** Flag to indicate if the accordion is plain */ + /** + * Flag to indicate if the accordion uses plain styling. Only applicable when the PatternFly glass + * theme is active (e.g. `pf-v6-theme-glass` on the document root); when the glass theme is not + * active, this prop has no effect. + */ isPlain?: boolean; /** Display size variant. */ displaySize?: 'default' | 'lg'; From f486954635ddd30312a069b3aaae4fd498130dd0 Mon Sep 17 00:00:00 2001 From: Titani Date: Wed, 25 Mar 2026 09:26:04 -0400 Subject: [PATCH 06/13] updates to add prop and remove util class --- .../src/components/Accordion/Accordion.tsx | 13 ++++++------- packages/react-core/src/helpers/util.ts | 14 -------------- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/packages/react-core/src/components/Accordion/Accordion.tsx b/packages/react-core/src/components/Accordion/Accordion.tsx index 99b486d4ba4..ea5c0c04d37 100644 --- a/packages/react-core/src/components/Accordion/Accordion.tsx +++ b/packages/react-core/src/components/Accordion/Accordion.tsx @@ -1,6 +1,5 @@ import { css } from '@patternfly/react-styles'; import styles from '@patternfly/react-styles/css/components/Accordion/accordion'; -import { hasGlassTheme } from '../../helpers/util'; import { AccordionContext } from './AccordionContext'; export interface AccordionProps extends React.HTMLProps { @@ -16,11 +15,9 @@ export interface AccordionProps extends React.HTMLProps { asDefinitionList?: boolean; /** Flag to indicate the accordion had a border */ isBordered?: boolean; - /** - * Flag to indicate if the accordion uses plain styling. Only applicable when the PatternFly glass - * theme is active (e.g. `pf-v6-theme-glass` on the document root); when the glass theme is not - * active, this prop has no effect. - */ + /** Flag to prevent the accordion from automatically applying plain styling when glass theme is enabled. */ + isNoPlainOnGlass?: boolean; + /** Flag to add plain styling to the accordion. */ isPlain?: boolean; /** Display size variant. */ displaySize?: 'default' | 'lg'; @@ -35,6 +32,7 @@ export const Accordion: React.FunctionComponent = ({ headingLevel = 'h3', asDefinitionList = true, isBordered = false, + isNoPlainOnGlass = false, isPlain = false, displaySize = 'default', togglePosition = 'end', @@ -46,7 +44,8 @@ export const Accordion: React.FunctionComponent = ({ className={css( styles.accordion, isBordered && styles.modifiers.bordered, - !isPlain && hasGlassTheme() && styles.modifiers.noPlain, + isNoPlainOnGlass && styles.modifiers.noPlain, + isPlain && styles.modifiers.plain, togglePosition === 'start' && styles.modifiers.toggleStart, displaySize === 'lg' && styles.modifiers.displayLg, className diff --git a/packages/react-core/src/helpers/util.ts b/packages/react-core/src/helpers/util.ts index 24312a56657..ac63bbaeade 100644 --- a/packages/react-core/src/helpers/util.ts +++ b/packages/react-core/src/helpers/util.ts @@ -395,20 +395,6 @@ export const toCamel = (s: string) => s.replace(/([-_][a-z])/gi, camelize); */ export const canUseDOM = !!(typeof window !== 'undefined' && window.document && window.document.createElement); -/** - * Checks if the PatternFly glass theme is applied at the document root. - * Used by components that adapt styling for glass theme (e.g. Accordion). - * - * @returns {boolean} - True if the glass theme class is present on the html element - */ -export const hasGlassTheme = (): boolean => { - if (typeof document === 'undefined') { - return false; - } - const classList = document.documentElement?.classList; - return classList ? classList.contains('pf-v6-theme-glass') : false; -}; - /** * Calculate the width of the text * Example: From cd8fc1a6692da73eb5c0235e05a17f4962bfc097 Mon Sep 17 00:00:00 2001 From: Titani Date: Wed, 25 Mar 2026 09:36:09 -0400 Subject: [PATCH 07/13] update unit test --- .../Accordion/__tests__/Accordion.test.tsx | 27 ++++++------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx b/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx index 23de19ad548..1dc4420e102 100644 --- a/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx +++ b/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx @@ -129,33 +129,22 @@ test(`Renders without class ${styles.modifiers.noPlain} by default`, () => { expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.noPlain); }); -test(`Renders without class ${styles.modifiers.noPlain} when isPlain is undefined`, () => { - render(Test); +test(`Renders with class ${styles.modifiers.noPlain} when isNoPlainOnGlass`, () => { + render(Test); - expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.noPlain); + expect(screen.getByText('Test')).toHaveClass(styles.modifiers.noPlain); }); -test(`Renders without class ${styles.modifiers.noPlain} when isPlain=false and glass theme is not applied`, () => { - render(Test); - - expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.noPlain); -}); - -test(`Renders with class ${styles.modifiers.noPlain} when isPlain=false and glass theme is applied`, () => { - document.documentElement.classList.add('pf-v6-theme-glass'); - try { - render(Test); +test(`Renders without class ${styles.modifiers.plain} by default`, () => { + render(Test); - expect(screen.getByText('Test')).toHaveClass(styles.modifiers.noPlain); - } finally { - document.documentElement.classList.remove('pf-v6-theme-glass'); - } + expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.plain); }); -test(`Renders without class ${styles.modifiers.noPlain} when isPlain=true`, () => { +test(`Renders with class ${styles.modifiers.plain} when isPlain`, () => { render(Test); - expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.noPlain); + expect(screen.getByText('Test')).toHaveClass(styles.modifiers.plain); }); test('Renders without pf-m-display-lg by default', () => { From a4a37be2d6cb02cda837567f7357293bafef2472 Mon Sep 17 00:00:00 2001 From: Titani Date: Wed, 25 Mar 2026 14:28:24 -0400 Subject: [PATCH 08/13] update tests --- .../Accordion/__tests__/Accordion.test.tsx | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx b/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx index 1dc4420e102..5d9c6fa3c9a 100644 --- a/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx +++ b/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx @@ -1,5 +1,4 @@ import '@testing-library/jest-dom'; -import { Fragment } from 'react'; import { render, screen } from '@testing-library/react'; import { Accordion } from '../Accordion'; @@ -26,10 +25,10 @@ test('Renders with the passed aria label', () => { test('Renders with inherited element props spread to the component', () => { render( - + <> Test

Label

- + ); expect(screen.getByText('Test')).toHaveAccessibleName('Label'); @@ -111,16 +110,16 @@ test('Provides a ToggleContainer of "h2" in a context when asDefinitionList is f expect(screen.getByText('h2')).toBeVisible(); }); -test('Renders without pf-m-bordered by default', () => { +test(`Renders without class ${styles.modifiers.bordered} by default`, () => { render(Test); - expect(screen.getByText('Test')).not.toHaveClass('pf-m-bordered'); + expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.bordered); }); -test('Renders with pf-m-bordered when isBordered=true', () => { +test(`Renders with class ${styles.modifiers.bordered} when isBordered=true`, () => { render(Test); - expect(screen.getByText('Test')).toHaveClass('pf-m-bordered'); + expect(screen.getByText('Test')).toHaveClass(styles.modifiers.bordered); }); test(`Renders without class ${styles.modifiers.noPlain} by default`, () => { @@ -147,16 +146,16 @@ test(`Renders with class ${styles.modifiers.plain} when isPlain`, () => { expect(screen.getByText('Test')).toHaveClass(styles.modifiers.plain); }); -test('Renders without pf-m-display-lg by default', () => { +test(`Renders without class ${styles.modifiers.displayLg} by default`, () => { render(Test); - expect(screen.getByText('Test')).not.toHaveClass('pf-m-display-lg'); + expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.displayLg); }); -test('Renders with pf-m-display-lg when displaySize="lg"', () => { +test(`Renders with class ${styles.modifiers.displayLg} when displaySize="lg"`, () => { render(Test); - expect(screen.getByText('Test')).toHaveClass('pf-m-display-lg'); + expect(screen.getByText('Test')).toHaveClass(styles.modifiers.displayLg); }); test(`Renders without class ${styles.modifiers.toggleStart} by default`, () => { From e94966c9f6f70daf3ee9495232b480f2ca88ad56 Mon Sep 17 00:00:00 2001 From: Titani Date: Wed, 25 Mar 2026 14:39:19 -0400 Subject: [PATCH 09/13] update tests --- .../Accordion/__tests__/Accordion.test.tsx | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx b/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx index 5d9c6fa3c9a..71de9daf24c 100644 --- a/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx +++ b/packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx @@ -34,7 +34,7 @@ test('Renders with inherited element props spread to the component', () => { expect(screen.getByText('Test')).toHaveAccessibleName('Label'); }); -test(`Renders with class ${styles.accordion}`, () => { +test(`Renders with class name ${styles.accordion}`, () => { render(Test); expect(screen.getByText('Test')).toHaveClass(styles.accordion); @@ -63,7 +63,7 @@ test('Renders Accordion as a "div" when asDefinitionList is false', () => { test('Provides a ContentContainer of "dd" in a context by default', () => { render( - {({ ContentContainer }) => String(ContentContainer)} + {({ ContentContainer }) => ContentContainer} ); @@ -73,7 +73,7 @@ test('Provides a ContentContainer of "dd" in a context by default', () => { test('Provides a ContentContainer of "div" in a context when asDefinitionList is false', () => { render( - {({ ContentContainer }) => String(ContentContainer)} + {({ ContentContainer }) => ContentContainer} ); @@ -83,7 +83,7 @@ test('Provides a ContentContainer of "div" in a context when asDefinitionList is test('Provides a ToggleContainer of "dt" in a context by default', () => { render( - {({ ToggleContainer }) => String(ToggleContainer)} + {({ ToggleContainer }) => ToggleContainer} ); @@ -93,7 +93,7 @@ test('Provides a ToggleContainer of "dt" in a context by default', () => { test('Provides a ToggleContainer of "h3" in a context when asDefinitionList is false', () => { render( - {({ ToggleContainer }) => String(ToggleContainer)} + {({ ToggleContainer }) => ToggleContainer} ); @@ -103,59 +103,59 @@ test('Provides a ToggleContainer of "h3" in a context when asDefinitionList is f test('Provides a ToggleContainer of "h2" in a context when asDefinitionList is false and headingLevel is "h2"', () => { render( - {({ ToggleContainer }) => String(ToggleContainer)} + {({ ToggleContainer }) => ToggleContainer} ); expect(screen.getByText('h2')).toBeVisible(); }); -test(`Renders without class ${styles.modifiers.bordered} by default`, () => { +test('Renders without pf-m-bordered by default', () => { render(Test); - expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.bordered); + expect(screen.getByText('Test')).not.toHaveClass('pf-m-bordered'); }); -test(`Renders with class ${styles.modifiers.bordered} when isBordered=true`, () => { +test('Renders with pf-m-bordered when isBordered=true', () => { render(Test); - expect(screen.getByText('Test')).toHaveClass(styles.modifiers.bordered); + expect(screen.getByText('Test')).toHaveClass('pf-m-bordered'); }); -test(`Renders without class ${styles.modifiers.noPlain} by default`, () => { +test('Renders without class pf-m-no-plain by default', () => { render(Test); - expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.noPlain); + expect(screen.getByText('Test')).not.toHaveClass('pf-m-no-plain'); }); -test(`Renders with class ${styles.modifiers.noPlain} when isNoPlainOnGlass`, () => { +test('Renders with class pf-m-no-plain when isNoPlainOnGlass', () => { render(Test); - expect(screen.getByText('Test')).toHaveClass(styles.modifiers.noPlain); + expect(screen.getByText('Test')).toHaveClass('pf-m-no-plain'); }); -test(`Renders without class ${styles.modifiers.plain} by default`, () => { +test('Renders without class pf-m-plain by default', () => { render(Test); - expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.plain); + expect(screen.getByText('Test')).not.toHaveClass('pf-m-plain'); }); -test(`Renders with class ${styles.modifiers.plain} when isPlain`, () => { +test('Renders with class pf-m-plain when isPlain', () => { render(Test); - expect(screen.getByText('Test')).toHaveClass(styles.modifiers.plain); + expect(screen.getByText('Test')).toHaveClass('pf-m-plain'); }); -test(`Renders without class ${styles.modifiers.displayLg} by default`, () => { +test('Renders without pf-m-display-lg by default', () => { render(Test); - expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.displayLg); + expect(screen.getByText('Test')).not.toHaveClass('pf-m-display-lg'); }); -test(`Renders with class ${styles.modifiers.displayLg} when displaySize="lg"`, () => { +test('Renders with pf-m-display-lg when displaySize="lg"', () => { render(Test); - expect(screen.getByText('Test')).toHaveClass(styles.modifiers.displayLg); + expect(screen.getByText('Test')).toHaveClass('pf-m-display-lg'); }); test(`Renders without class ${styles.modifiers.toggleStart} by default`, () => { From ece529eb6f4f2c2b53bbdbd36ee35e4a1ec7f83c Mon Sep 17 00:00:00 2001 From: Titani Date: Thu, 26 Mar 2026 11:58:02 -0400 Subject: [PATCH 10/13] add warning --- .../src/components/Accordion/Accordion.tsx | 11 ++- .../Accordion/__tests__/Accordion.test.tsx | 67 ++++++++++++++++--- 2 files changed, 68 insertions(+), 10 deletions(-) diff --git a/packages/react-core/src/components/Accordion/Accordion.tsx b/packages/react-core/src/components/Accordion/Accordion.tsx index ea5c0c04d37..6919d575d66 100644 --- a/packages/react-core/src/components/Accordion/Accordion.tsx +++ b/packages/react-core/src/components/Accordion/Accordion.tsx @@ -15,9 +15,9 @@ export interface AccordionProps extends React.HTMLProps { asDefinitionList?: boolean; /** Flag to indicate the accordion had a border */ isBordered?: boolean; - /** Flag to prevent the accordion from automatically applying plain styling when glass theme is enabled. */ + /** @beta Flag to prevent the accordion from automatically applying plain styling when glass theme is enabled. */ isNoPlainOnGlass?: boolean; - /** Flag to add plain styling to the accordion. */ + /** @beta Flag to add plain styling to the accordion. */ isPlain?: boolean; /** Display size variant. */ displaySize?: 'default' | 'lg'; @@ -38,6 +38,13 @@ export const Accordion: React.FunctionComponent = ({ togglePosition = 'end', ...props }: AccordionProps) => { + if (isPlain && isNoPlainOnGlass) { + // eslint-disable-next-line no-console + console.warn( + `Accordion: When both isPlain and isNoPlainOnGlass are true, styling may conflict. It's recommended to pass only one prop according to the current theme.` + ); + } + const AccordionList: any = asDefinitionList ? 'dl' : 'div'; return ( { expect(screen.getByText('Test')).toHaveClass('pf-m-bordered'); }); -test('Renders without class pf-m-no-plain by default', () => { +test(`Renders without class ${styles.modifiers.noPlain} by default`, () => { render(Test); - expect(screen.getByText('Test')).not.toHaveClass('pf-m-no-plain'); + expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.noPlain); }); -test('Renders with class pf-m-no-plain when isNoPlainOnGlass', () => { +test(`Renders with class ${styles.modifiers.noPlain} when isNoPlainOnGlass`, () => { render(Test); - expect(screen.getByText('Test')).toHaveClass('pf-m-no-plain'); + expect(screen.getByText('Test')).toHaveClass(styles.modifiers.noPlain); }); -test('Renders without class pf-m-plain by default', () => { +test(`Renders without class ${styles.modifiers.plain} by default`, () => { render(Test); - expect(screen.getByText('Test')).not.toHaveClass('pf-m-plain'); + expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.plain); }); -test('Renders with class pf-m-plain when isPlain', () => { +test(`Renders with class ${styles.modifiers.plain} when isPlain`, () => { render(Test); - expect(screen.getByText('Test')).toHaveClass('pf-m-plain'); + expect(screen.getByText('Test')).toHaveClass(styles.modifiers.plain); +}); + +test('warns when both isPlain and isNoPlainOnGlass are true', () => { + const consoleWarning = jest.spyOn(console, 'warn').mockImplementation(); + + render( + + Test + + ); + + expect(consoleWarning).toHaveBeenCalledWith( + `Accordion: When both isPlain and isNoPlainOnGlass are true, styling may conflict. It's recommended to pass only one prop according to the current theme.` + ); + + consoleWarning.mockRestore(); +}); + +test(`applies both ${styles.modifiers.plain} and ${styles.modifiers.noPlain} when both isPlain and isNoPlainOnGlass are true`, () => { + const consoleWarning = jest.spyOn(console, 'warn').mockImplementation(); + + render( + + Test + + ); + + expect(screen.getByText('Test')).toHaveClass(styles.modifiers.plain); + expect(screen.getByText('Test')).toHaveClass(styles.modifiers.noPlain); + + consoleWarning.mockRestore(); +}); + +test('does not warn when only isNoPlainOnGlass is true', () => { + const consoleWarning = jest.spyOn(console, 'warn').mockImplementation(); + + render(Test); + + expect(consoleWarning).not.toHaveBeenCalled(); + + consoleWarning.mockRestore(); +}); + +test('does not warn when only isPlain is true', () => { + const consoleWarning = jest.spyOn(console, 'warn').mockImplementation(); + + render(Test); + + expect(consoleWarning).not.toHaveBeenCalled(); + + consoleWarning.mockRestore(); }); test('Renders without pf-m-display-lg by default', () => { From 817768d7a6f0f72a0d6d45afffc874f26d9464de Mon Sep 17 00:00:00 2001 From: Titani Date: Thu, 2 Apr 2026 10:24:15 -0400 Subject: [PATCH 11/13] Removed warning. Added test case for broken feature --- .../src/components/Accordion/Accordion.tsx | 9 +--- .../Accordion/__tests__/Accordion.test.tsx | 52 +++---------------- .../cypress/integration/accordion.spec.ts | 24 +++++++++ .../demos/Accordion/AccordionDemo.tsx | 14 +++++ 4 files changed, 45 insertions(+), 54 deletions(-) diff --git a/packages/react-core/src/components/Accordion/Accordion.tsx b/packages/react-core/src/components/Accordion/Accordion.tsx index 6919d575d66..64b9c16898c 100644 --- a/packages/react-core/src/components/Accordion/Accordion.tsx +++ b/packages/react-core/src/components/Accordion/Accordion.tsx @@ -38,20 +38,13 @@ export const Accordion: React.FunctionComponent = ({ togglePosition = 'end', ...props }: AccordionProps) => { - if (isPlain && isNoPlainOnGlass) { - // eslint-disable-next-line no-console - console.warn( - `Accordion: When both isPlain and isNoPlainOnGlass are true, styling may conflict. It's recommended to pass only one prop according to the current theme.` - ); - } - const AccordionList: any = asDefinitionList ? 'dl' : 'div'; return ( { expect(screen.getByText('Test')).toHaveClass('pf-m-bordered'); }); -test(`Renders without class ${styles.modifiers.noPlain} by default`, () => { +test(`Renders without class ${styles.modifiers.noPlainOnGlass} by default`, () => { render(Test); - expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.noPlain); + expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.noPlainOnGlass); }); -test(`Renders with class ${styles.modifiers.noPlain} when isNoPlainOnGlass`, () => { +test(`Renders with class ${styles.modifiers.noPlainOnGlass} when isNoPlainOnGlass`, () => { render(Test); - expect(screen.getByText('Test')).toHaveClass(styles.modifiers.noPlain); + expect(screen.getByText('Test')).toHaveClass(styles.modifiers.noPlainOnGlass); }); test(`Renders without class ${styles.modifiers.plain} by default`, () => { @@ -146,25 +146,7 @@ test(`Renders with class ${styles.modifiers.plain} when isPlain`, () => { expect(screen.getByText('Test')).toHaveClass(styles.modifiers.plain); }); -test('warns when both isPlain and isNoPlainOnGlass are true', () => { - const consoleWarning = jest.spyOn(console, 'warn').mockImplementation(); - - render( - - Test - - ); - - expect(consoleWarning).toHaveBeenCalledWith( - `Accordion: When both isPlain and isNoPlainOnGlass are true, styling may conflict. It's recommended to pass only one prop according to the current theme.` - ); - - consoleWarning.mockRestore(); -}); - -test(`applies both ${styles.modifiers.plain} and ${styles.modifiers.noPlain} when both isPlain and isNoPlainOnGlass are true`, () => { - const consoleWarning = jest.spyOn(console, 'warn').mockImplementation(); - +test(`applies both ${styles.modifiers.plain} and ${styles.modifiers.noPlainOnGlass} when both isPlain and isNoPlainOnGlass are true`, () => { render( Test @@ -172,29 +154,7 @@ test(`applies both ${styles.modifiers.plain} and ${styles.modifiers.noPlain} whe ); expect(screen.getByText('Test')).toHaveClass(styles.modifiers.plain); - expect(screen.getByText('Test')).toHaveClass(styles.modifiers.noPlain); - - consoleWarning.mockRestore(); -}); - -test('does not warn when only isNoPlainOnGlass is true', () => { - const consoleWarning = jest.spyOn(console, 'warn').mockImplementation(); - - render(Test); - - expect(consoleWarning).not.toHaveBeenCalled(); - - consoleWarning.mockRestore(); -}); - -test('does not warn when only isPlain is true', () => { - const consoleWarning = jest.spyOn(console, 'warn').mockImplementation(); - - render(Test); - - expect(consoleWarning).not.toHaveBeenCalled(); - - consoleWarning.mockRestore(); + expect(screen.getByText('Test')).toHaveClass(styles.modifiers.noPlainOnGlass); }); test('Renders without pf-m-display-lg by default', () => { diff --git a/packages/react-integration/cypress/integration/accordion.spec.ts b/packages/react-integration/cypress/integration/accordion.spec.ts index d644571059f..771d873f8dc 100644 --- a/packages/react-integration/cypress/integration/accordion.spec.ts +++ b/packages/react-integration/cypress/integration/accordion.spec.ts @@ -28,4 +28,28 @@ describe('Accordion Demo Test', () => { cy.get('#divAccordion-item3-content').should('have.attr', 'role', 'region'); cy.get('#definitionListAccordion-item1-content').should('not.have.attr', 'role'); }); + + it('in glass theme, does not apply glass plain transparent background when pf-m-no-plain-on-glass is present (even with pf-m-plain)', () => { + cy.visit('http://localhost:3000/accordion-demo-nav-link'); + cy.document().then((doc) => { + doc.documentElement.classList.add('pf-v6-theme-glass'); + }); + + cy.get('[data-testid="accordion-glass-plain-both"]') + .should('have.class', 'pf-m-no-plain-on-glass') + .and('have.class', 'pf-m-plain'); + + /** + * This test fails due to a css bug. + */ + cy.get('[data-testid="accordion-glass-plain-both"]').then(($el) => { + const el = $el[0]; + const win = el.ownerDocument.defaultView; + if (!win) { + throw new Error('expected window'); + } + const bg = win.getComputedStyle(el).backgroundColor; + expect(bg).not.to.match(/rgba\(0,\s*0,\s*0,\s*0\)|transparent/); + }); + }); }); diff --git a/packages/react-integration/demo-app-ts/src/components/demos/Accordion/AccordionDemo.tsx b/packages/react-integration/demo-app-ts/src/components/demos/Accordion/AccordionDemo.tsx index 483342d1ca4..e0eb9d1e4c8 100644 --- a/packages/react-integration/demo-app-ts/src/components/demos/Accordion/AccordionDemo.tsx +++ b/packages/react-integration/demo-app-ts/src/components/demos/Accordion/AccordionDemo.tsx @@ -103,6 +103,20 @@ export const AccordionDemo = () => { +
+ + + Glass theme: isPlain and isNoPlainOnGlass + +

Used by Cypress to verify classes and styles under pf-v6-theme-glass.

+ + + ); }; From 7a3c098a513a7117bc39d9447f321f8eaac11832 Mon Sep 17 00:00:00 2001 From: Titani Date: Thu, 2 Apr 2026 11:48:39 -0400 Subject: [PATCH 12/13] fix lint issue --- yarn.lock | 9 --------- 1 file changed, 9 deletions(-) diff --git a/yarn.lock b/yarn.lock index eaa65640dcc..55b094e0b08 100644 --- a/yarn.lock +++ b/yarn.lock @@ -17300,15 +17300,6 @@ __metadata: languageName: node linkType: hard -"minimatch@npm:^10.2.2": - version: 10.2.5 - resolution: "minimatch@npm:10.2.5" - dependencies: - brace-expansion: "npm:^5.0.5" - checksum: 10c0/6bb058bd6324104b9ec2f763476a35386d05079c1f5fe4fbf1f324a25237cd4534d6813ecd71f48208f4e635c1221899bef94c3c89f7df55698fe373aaae20fd - languageName: node - linkType: hard - "minimatch@npm:^3.0.2, minimatch@npm:^3.0.4, minimatch@npm:^3.1.1, minimatch@npm:^3.1.2": version: 3.1.2 resolution: "minimatch@npm:3.1.2" From c9d8d4f52bd5e41af235973cb010ca88ab11d3bd Mon Sep 17 00:00:00 2001 From: Titani Date: Thu, 2 Apr 2026 15:27:41 -0400 Subject: [PATCH 13/13] update cypress test --- .../react-integration/cypress/integration/accordion.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react-integration/cypress/integration/accordion.spec.ts b/packages/react-integration/cypress/integration/accordion.spec.ts index 771d873f8dc..6f2efc9b0ed 100644 --- a/packages/react-integration/cypress/integration/accordion.spec.ts +++ b/packages/react-integration/cypress/integration/accordion.spec.ts @@ -49,7 +49,8 @@ describe('Accordion Demo Test', () => { throw new Error('expected window'); } const bg = win.getComputedStyle(el).backgroundColor; - expect(bg).not.to.match(/rgba\(0,\s*0,\s*0,\s*0\)|transparent/); + const fullyTransparent = bg === 'transparent' || bg === 'rgba(0, 0, 0, 0)' || bg === 'rgba(0,0,0,0)'; + expect(fullyTransparent, `expected non-transparent background, got ${bg}`).to.eq(false); }); }); });