From 19b6dae3fdc0af631db416989df436d73b81e9a1 Mon Sep 17 00:00:00 2001 From: Titani Date: Mon, 23 Mar 2026 17:33:47 -0400 Subject: [PATCH 01/10] 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 91c45a40567b1f803e4486f4a51c7ef6b83e3317 Mon Sep 17 00:00:00 2001 From: Titani Date: Tue, 24 Mar 2026 08:53:02 -0400 Subject: [PATCH 02/10] 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 ecbb0be6e0b8d6c8452757a3d06c4de8e4b7435b Mon Sep 17 00:00:00 2001 From: Titani Date: Tue, 24 Mar 2026 09:03:39 -0400 Subject: [PATCH 03/10] 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 fc5cf2fdfc52d4da4ac5887a6bb96d19383c0854 Mon Sep 17 00:00:00 2001 From: Titani Date: Tue, 24 Mar 2026 10:59:18 -0400 Subject: [PATCH 04/10] 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 a0e636c5b065e1feb0af9e56f6866beaf4e5db91 Mon Sep 17 00:00:00 2001 From: Titani Date: Tue, 24 Mar 2026 11:09:27 -0400 Subject: [PATCH 05/10] 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 e6fdc9a44947de1a366bc6687ca805456218d73a Mon Sep 17 00:00:00 2001 From: Titani Date: Wed, 25 Mar 2026 09:26:04 -0400 Subject: [PATCH 06/10] 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 f052aac9e275bb213d119c472a6d0d473c57e61f Mon Sep 17 00:00:00 2001 From: Titani Date: Wed, 25 Mar 2026 09:36:09 -0400 Subject: [PATCH 07/10] 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 f6c9eddfd094893be96215a74b80d2b675d4ff0d Mon Sep 17 00:00:00 2001 From: Titani Date: Wed, 25 Mar 2026 14:28:24 -0400 Subject: [PATCH 08/10] 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 c51aa5abd9c2aab2ed41ed66901688914c9f8c1e Mon Sep 17 00:00:00 2001 From: Titani Date: Wed, 25 Mar 2026 14:39:19 -0400 Subject: [PATCH 09/10] 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 be77e8318b792ef8b876623dcf2d9853294e077a Mon Sep 17 00:00:00 2001 From: Titani Date: Thu, 26 Mar 2026 11:58:02 -0400 Subject: [PATCH 10/10] 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', () => {