Skip to content

Commit be77e83

Browse files
committed
add warning
1 parent c51aa5a commit be77e83

File tree

2 files changed

+68
-10
lines changed

2 files changed

+68
-10
lines changed

packages/react-core/src/components/Accordion/Accordion.tsx

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ export interface AccordionProps extends React.HTMLProps<HTMLDListElement> {
1515
asDefinitionList?: boolean;
1616
/** Flag to indicate the accordion had a border */
1717
isBordered?: boolean;
18-
/** Flag to prevent the accordion from automatically applying plain styling when glass theme is enabled. */
18+
/** @beta Flag to prevent the accordion from automatically applying plain styling when glass theme is enabled. */
1919
isNoPlainOnGlass?: boolean;
20-
/** Flag to add plain styling to the accordion. */
20+
/** @beta Flag to add plain styling to the accordion. */
2121
isPlain?: boolean;
2222
/** Display size variant. */
2323
displaySize?: 'default' | 'lg';
@@ -38,6 +38,13 @@ export const Accordion: React.FunctionComponent<AccordionProps> = ({
3838
togglePosition = 'end',
3939
...props
4040
}: AccordionProps) => {
41+
if (isPlain && isNoPlainOnGlass) {
42+
// eslint-disable-next-line no-console
43+
console.warn(
44+
`Accordion: When both isPlain and isNoPlainOnGlass are true, styling may conflict. It's recommended to pass only one prop according to the current theme.`
45+
);
46+
}
47+
4148
const AccordionList: any = asDefinitionList ? 'dl' : 'div';
4249
return (
4350
<AccordionList

packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx

Lines changed: 59 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,28 +122,79 @@ test('Renders with pf-m-bordered when isBordered=true', () => {
122122
expect(screen.getByText('Test')).toHaveClass('pf-m-bordered');
123123
});
124124

125-
test('Renders without class pf-m-no-plain by default', () => {
125+
test(`Renders without class ${styles.modifiers.noPlain} by default`, () => {
126126
render(<Accordion>Test</Accordion>);
127127

128-
expect(screen.getByText('Test')).not.toHaveClass('pf-m-no-plain');
128+
expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.noPlain);
129129
});
130130

131-
test('Renders with class pf-m-no-plain when isNoPlainOnGlass', () => {
131+
test(`Renders with class ${styles.modifiers.noPlain} when isNoPlainOnGlass`, () => {
132132
render(<Accordion isNoPlainOnGlass>Test</Accordion>);
133133

134-
expect(screen.getByText('Test')).toHaveClass('pf-m-no-plain');
134+
expect(screen.getByText('Test')).toHaveClass(styles.modifiers.noPlain);
135135
});
136136

137-
test('Renders without class pf-m-plain by default', () => {
137+
test(`Renders without class ${styles.modifiers.plain} by default`, () => {
138138
render(<Accordion>Test</Accordion>);
139139

140-
expect(screen.getByText('Test')).not.toHaveClass('pf-m-plain');
140+
expect(screen.getByText('Test')).not.toHaveClass(styles.modifiers.plain);
141141
});
142142

143-
test('Renders with class pf-m-plain when isPlain', () => {
143+
test(`Renders with class ${styles.modifiers.plain} when isPlain`, () => {
144144
render(<Accordion isPlain>Test</Accordion>);
145145

146-
expect(screen.getByText('Test')).toHaveClass('pf-m-plain');
146+
expect(screen.getByText('Test')).toHaveClass(styles.modifiers.plain);
147+
});
148+
149+
test('warns when both isPlain and isNoPlainOnGlass are true', () => {
150+
const consoleWarning = jest.spyOn(console, 'warn').mockImplementation();
151+
152+
render(
153+
<Accordion isPlain isNoPlainOnGlass>
154+
Test
155+
</Accordion>
156+
);
157+
158+
expect(consoleWarning).toHaveBeenCalledWith(
159+
`Accordion: When both isPlain and isNoPlainOnGlass are true, styling may conflict. It's recommended to pass only one prop according to the current theme.`
160+
);
161+
162+
consoleWarning.mockRestore();
163+
});
164+
165+
test(`applies both ${styles.modifiers.plain} and ${styles.modifiers.noPlain} when both isPlain and isNoPlainOnGlass are true`, () => {
166+
const consoleWarning = jest.spyOn(console, 'warn').mockImplementation();
167+
168+
render(
169+
<Accordion isPlain isNoPlainOnGlass>
170+
Test
171+
</Accordion>
172+
);
173+
174+
expect(screen.getByText('Test')).toHaveClass(styles.modifiers.plain);
175+
expect(screen.getByText('Test')).toHaveClass(styles.modifiers.noPlain);
176+
177+
consoleWarning.mockRestore();
178+
});
179+
180+
test('does not warn when only isNoPlainOnGlass is true', () => {
181+
const consoleWarning = jest.spyOn(console, 'warn').mockImplementation();
182+
183+
render(<Accordion isNoPlainOnGlass>Test</Accordion>);
184+
185+
expect(consoleWarning).not.toHaveBeenCalled();
186+
187+
consoleWarning.mockRestore();
188+
});
189+
190+
test('does not warn when only isPlain is true', () => {
191+
const consoleWarning = jest.spyOn(console, 'warn').mockImplementation();
192+
193+
render(<Accordion isPlain>Test</Accordion>);
194+
195+
expect(consoleWarning).not.toHaveBeenCalled();
196+
197+
consoleWarning.mockRestore();
147198
});
148199

149200
test('Renders without pf-m-display-lg by default', () => {

0 commit comments

Comments
 (0)