Skip to content

fix(theme): wrap toggle button items to prevent clipped overflow - #4234

Open
logonoff wants to merge 1 commit into
redhat-developer:mainfrom
logonoff:RHDHBUGS-3622
Open

fix(theme): wrap toggle button items to prevent clipped overflow#4234
logonoff wants to merge 1 commit into
redhat-developer:mainfrom
logonoff:RHDHBUGS-3622

Conversation

@logonoff

Copy link
Copy Markdown
Member

Hey, I just made a Pull Request!

before:
image

after:
image

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

@logonoff
logonoff requested review from a team and ciiay as code owners August 10, 2026 15:28
@rhdh-gh-app

rhdh-gh-app Bot commented Aug 10, 2026

Copy link
Copy Markdown

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-theme workspaces/theme/plugins/theme patch v1.0.2

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

Fix theme: allow ToggleButtonGroup items to wrap to avoid overflow clipping

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Add theme override to wrap MUI ToggleButtonGroup items onto multiple rows.
• Introduce consistent row spacing to prevent clipped overflow in narrow layouts.
• Publish a patch changeset for the theme package.
Diagram

graph TD
  A["App UI"] --> B["Theme provider"] --> C["createComponents.ts"] --> D["MuiToggleButtonGroup override"] --> E["Wrapped toggle layout"]
  R["Changeset tooling"] --> S[".changeset (patch)"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Fix locally where ToggleButtonGroup is used (component-level CSS)
  • ➕ Limits behavior change to the affected screen(s) only
  • ➕ Avoids unexpected wrapping in other parts of the product
  • ➖ Requires finding and updating every usage
  • ➖ Harder to guarantee consistency across plugins/pages
2. Make wrapping opt-in via a theme variant or class selector
  • ➕ Avoids global behavior changes while still being centralized
  • ➕ Allows different layouts (wrap vs no-wrap) per context
  • ➖ More implementation complexity and API surface
  • ➖ Needs coordinated updates in consuming components
3. Address overflow via container scrolling instead of wrapping
  • ➕ Preserves single-row toggle groups visually
  • ➕ May better match some UX patterns
  • ➖ Horizontal scrolling can be less discoverable and harder to use
  • ➖ Does not reduce overall width pressure in tight layouts

Recommendation: The global theme override is a reasonable fix if overflow clipping is a cross-app issue and wrapping is acceptable UX everywhere. If only specific pages are affected or some toggle groups must remain single-row, prefer an opt-in variant/class-based approach to prevent unintended layout changes.

Files changed (2) +13 / -0

Bug fix (1) +8 / -0
createComponents.tsWrap MuiToggleButtonGroup items to prevent overflow clipping +8/-0

Wrap MuiToggleButtonGroup items to prevent overflow clipping

• Adds a 'MuiToggleButtonGroup' theme 'styleOverrides.root' configuration. Enables 'flexWrap: 'wrap'' and adds 'rowGap: 8' to improve multi-row spacing when controls overflow.

workspaces/theme/plugins/theme/src/utils/createComponents.ts

Other (1) +5 / -0
polite-eggs-care.mdAdd patch changeset for ToggleButtonGroup wrapping fix +5/-0

Add patch changeset for ToggleButtonGroup wrapping fix

• Introduces a changeset marking a patch release for '@red-hat-developer-hub/backstage-plugin-theme'. Documents the UI fix as wrapping toggle button items to prevent clipped overflow.

workspaces/theme/.changeset/polite-eggs-care.md

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Aug 10, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (1) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Global ToggleGroup layout change 🔗 Cross-repo conflict ≡ Correctness
Description
This PR makes MuiToggleButtonGroup wrap (flexWrap: 'wrap') with rowGap: 8 at the theme level;
because RHDH globally applies themes from @red-hat-developer-hub/backstage-plugin-theme, any
ToggleButtonGroup rendered in RHDH (including via dynamic plugins) may change height/layout.
Coordinate with redhat-developer/rhdh to validate no unintended toolbar/filter regressions and to
document the global behavior change in the theme package release notes.
Code

workspaces/theme/plugins/theme/src/utils/createComponents.ts[R276-281]

+    components.MuiToggleButtonGroup = {
+      styleOverrides: {
+        root: {
+          flexWrap: 'wrap',
+          rowGap: 8,
+        },
Relevance

●● Moderate

Team sometimes accepts downstream-impact checks, but cross-repo coordination/release-note requests
are inconsistently enforced.

PR-#4202
PR-#3837

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR adds a global MUI component override for MuiToggleButtonGroup. In the RHDH repo, the app
depends on @red-hat-developer-hub/backstage-plugin-theme and uses useThemes() to populate the
Backstage app themes list, meaning these component overrides apply globally in RHDH at runtime.

workspaces/theme/plugins/theme/src/utils/createComponents.ts[269-283]
External repo: redhat-developer/rhdh, packages/app/package.json [49-53]
External repo: redhat-developer/rhdh, packages/app/src/components/DynamicRoot/DynamicRoot.tsx [25-26]
External repo: redhat-developer/rhdh, packages/app/src/components/DynamicRoot/DynamicRoot.tsx [125-126]
External repo: redhat-developer/rhdh, packages/app/src/components/DynamicRoot/DynamicRoot.tsx [570-609]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The theme package now globally overrides `MuiToggleButtonGroup.root` to wrap items, which will affect *all* `ToggleButtonGroup` instances in RHDH because RHDH imports and applies themes from `@red-hat-developer-hub/backstage-plugin-theme`.

## Issue Context
RHDH wires `useThemes()` into `createApp({ themes: ... })`, so MUI component overrides in the theme package become global across the application and any loaded dynamic plugins.

## Fix Focus Areas
- workspaces/theme/plugins/theme/src/utils/createComponents.ts[269-283]
- /cross_repos/rhdh/packages/app/src/components/DynamicRoot/DynamicRoot.tsx[25-26]
- /cross_repos/rhdh/packages/app/src/components/DynamicRoot/DynamicRoot.tsx[125-126]
- /cross_repos/rhdh/packages/app/src/components/DynamicRoot/DynamicRoot.tsx[570-609]
- /cross_repos/rhdh/packages/app/package.json[49-53]

## Suggested remediation options
1) **Scope the override**: apply wrapping only to a specific variant/className you control (e.g., via `variants` keyed on props, or a theme-specific class applied by known consumers), instead of overriding every `ToggleButtonGroup`.
2) **Make it configurable**: gate the wrapping behavior behind a `ThemeConfig` option (defaulting to current behavior if you intend it globally), so RHDH can opt out for toolbars that require single-row layout.
3) **Coordination**: add a note to the changeset/release notes highlighting the global UI behavior change and ensure RHDH maintainers validate key screens before bumping the dependency.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context used
✅ Compliance rules (platform): 11 rules
✅ Cross-repo context
  Explored: repo: redhat-developer/rhdh (sha: 820e6260)
  Not relevant to this PR: redhat-developer/rhdh-chart
  Not relevant to this PR: redhat-developer/rhdh-operator
  Not relevant to this PR: redhat-developer/rhdh-local

Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.40%. Comparing base (51a6bc2) to head (ed082f8).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4234   +/-   ##
=======================================
  Coverage   58.40%   58.40%           
=======================================
  Files        2438     2438           
  Lines       96960    96961    +1     
  Branches    26937    26937           
=======================================
+ Hits        56628    56629    +1     
  Misses      40135    40135           
  Partials      197      197           
Flag Coverage Δ *Carryforward flag
adoption-insights 84.55% <ø> (ø) Carriedforward from 51a6bc2
ai-integrations 69.76% <ø> (ø) Carriedforward from 51a6bc2
app-defaults 69.79% <ø> (ø) Carriedforward from 51a6bc2
augment 46.67% <ø> (ø) Carriedforward from 51a6bc2
boost 77.63% <ø> (ø) Carriedforward from 51a6bc2
bulk-import 72.79% <ø> (ø) Carriedforward from 51a6bc2
cost-management 13.55% <ø> (ø) Carriedforward from 51a6bc2
dcm 67.21% <ø> (ø) Carriedforward from 51a6bc2
extensions 56.59% <ø> (ø) Carriedforward from 51a6bc2
global-floating-action-button 71.18% <ø> (ø) Carriedforward from 51a6bc2
global-header 66.50% <ø> (ø) Carriedforward from 51a6bc2
homepage 47.50% <ø> (ø) Carriedforward from 51a6bc2
install-dynamic-plugins 59.95% <ø> (ø) Carriedforward from 51a6bc2
intelligent-assistant 75.24% <ø> (ø) Carriedforward from 51a6bc2
konflux 91.98% <ø> (ø) Carriedforward from 51a6bc2
lightspeed 69.02% <ø> (ø) Carriedforward from 51a6bc2
mcp-integrations 83.40% <ø> (ø) Carriedforward from 51a6bc2
orchestrator 66.91% <ø> (ø) Carriedforward from 51a6bc2
quickstart 63.74% <ø> (ø) Carriedforward from 51a6bc2
sandbox 79.56% <ø> (ø) Carriedforward from 51a6bc2
scorecard 86.23% <ø> (ø) Carriedforward from 51a6bc2
theme 88.80% <100.00%> (+0.02%) ⬆️
translations 5.12% <ø> (ø) Carriedforward from 51a6bc2
x2a 79.20% <ø> (ø) Carriedforward from 51a6bc2

*This pull request uses carry forward flags. Click here to find out more.


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51a6bc2...ed082f8. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the theme package’s MUI component overrides to prevent MuiToggleButtonGroup content from being clipped by allowing toggle buttons to wrap onto multiple rows.

Changes:

  • Add MuiToggleButtonGroup styleOverrides.root with flexWrap: 'wrap' and a vertical gap between wrapped rows.
  • Add a changeset to publish a patch release of @red-hat-developer-hub/backstage-plugin-theme.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
workspaces/theme/plugins/theme/src/utils/createComponents.ts Adds a MUI MuiToggleButtonGroup style override to allow wrapping and prevent overflow clipping.
workspaces/theme/.changeset/polite-eggs-care.md Declares a patch changeset for the theme package describing the toggle button wrapping fix.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread workspaces/theme/plugins/theme/src/utils/createComponents.ts Outdated
Comment on lines +276 to +280
components.MuiToggleButtonGroup = {
styleOverrides: {
root: {
flexWrap: 'wrap',
rowGap: 8,
Signed-off-by: logonoff <git@logonoff.co>
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants