feat(Accordion): Added isPlain and isNoPlainOnGlass prop to Accordion

[tlabaj]

What: Closes #12276

• Added isPlain prop to add pf-m-plain class when set
• Added isNoPlainOnGlass prop that adds the pf-m-no-plain class to prevent the accordion from automatically applying plain styling when glass theme is enabled

Summary by CodeRabbit

• New Features
  • Accordion adds two optional styling flags: a "plain" mode and a flag to adjust plain styling when the glass theme is active.
• Tests
  • Expanded tests to cover the new styling flags and verify conditional class behavior and inherited prop handling.

[patternfly-build]

Preview: https://pf-react-pr-12288.surge.sh

A11y report: https://pf-react-pr-12288-a11y.surge.sh

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds two optional boolean props to the Accordion component—isPlain and isNoPlainOnGlass—and applies corresponding CSS modifier classes when set. Tests were updated/added to assert the presence or absence of these modifier classes.

Changes

Cohort / File(s)	Summary
Accordion Component packages/react-core/src/components/Accordion/Accordion.tsx	Added isPlain?: boolean and isNoPlainOnGlass?: boolean to AccordionProps, defaulted both to false, and conditionally apply styles.modifiers.plain and styles.modifiers.noPlain to the root element's className.
Accordion Tests packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx	Updated test imports and fragment usage, adjusted some assertions to coerce values to strings, renamed one test description, and added tests verifying styles.modifiers.plain and styles.modifiers.noPlain are applied only when their respective props are set.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

• nicolethoen

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: adding two new props (isPlain and isNoPlainOnGlass) to the Accordion component, which matches the primary changes in the pull request.
Linked Issues check	✅ Passed	The PR successfully implements both required modifiers (isPlain and isNoPlainOnGlass) as specified in issue #12276, with proper class application logic and comprehensive test coverage.
Out of Scope Changes check	✅ Passed	All changes are directly related to the linked issue requirements: the two new props, their default values, class application logic, and comprehensive test cases for the new modifiers are all within scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/react-core/src/components/Accordion/Accordion.tsx (1)

34-45: ⚠️ Potential issue | 🟠 Major

isPlain default broadens behavior beyond explicit opt-out.

With Line 34 defaulting isPlain to false, Line 45 applies noPlain under glass theme even when consumers omit isPlain. This conflicts with the “apply when isPlain={false}” behavior in the PR objective.

Proposed fix

 export const Accordion: React.FunctionComponent<AccordionProps> = ({
@@
-  isPlain = false,
+  isPlain,
@@
-        !isPlain && hasGlassTheme() && styles.modifiers.noPlain,
+        isPlain === false && hasGlassTheme() && styles.modifiers.noPlain,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/react-core/src/components/Accordion/Accordion.tsx` around lines 34 -
45, The component currently defaults isPlain = false and uses !isPlain in the
className, which causes noPlain to be applied when the prop is omitted; update
the logic so noPlain is only applied when isPlain is explicitly false. Change
the className conditional from !isPlain && hasGlassTheme() to isPlain === false
&& hasGlassTheme() (or alternatively remove the default and set isPlain to
undefined) so styles.modifiers.noPlain is only added when the consumer passed
isPlain={false}; reference: prop isPlain, Accordion component,
styles.modifiers.noPlain, and hasGlassTheme().

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx`:
- Around line 144-150: The test that adds the 'pf-v6-theme-glass' root class
(test named "Renders with class ${styles.modifiers.noPlain} when isPlain=false
and glass theme is applied") should guard cleanup with a try/finally: add the
class before rendering and put the render/assert inside a try block, then always
remove the class in the finally block to prevent leakage into other tests;
update the test that references Accordion and styles.modifiers.noPlain
accordingly.

---

Outside diff comments:
In `@packages/react-core/src/components/Accordion/Accordion.tsx`:
- Around line 34-45: The component currently defaults isPlain = false and uses
!isPlain in the className, which causes noPlain to be applied when the prop is
omitted; update the logic so noPlain is only applied when isPlain is explicitly
false. Change the className conditional from !isPlain && hasGlassTheme() to
isPlain === false && hasGlassTheme() (or alternatively remove the default and
set isPlain to undefined) so styles.modifiers.noPlain is only added when the
consumer passed isPlain={false}; reference: prop isPlain, Accordion component,
styles.modifiers.noPlain, and hasGlassTheme().

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b2854331-7f1e-4197-909a-6702d0712ea8

📥 Commits

Reviewing files that changed from the base of the PR and between 641c888 and ecbb0be.

📒 Files selected for processing (3)

• packages/react-core/src/components/Accordion/Accordion.tsx
• packages/react-core/src/components/Accordion/__tests__/Accordion.test.tsx
• packages/react-core/src/helpers/util.ts

[thatblindgeye]

Looks good! Just wondering, are the other changes to the test file necessary to get tests to pass?

[thatblindgeye]

Actually we may want to add the beta flag here similar to what Katie did in her Table PR