Skip to content

fix(theme): CSS-only PF page inset without PageMainContainer - #4938

Merged
karthikjeeyar merged 5 commits into
redhat-developer:mainfrom
ciiay:fix/theme-RHIDP-14506-css-only-page-inset
Sep 30, 2026
Merged

karthikjeeyar merged 5 commits into
redhat-developer:mainfrom
ciiay:fix/theme-RHIDP-14506-css-only-page-inset

Conversation

@ciiay

@ciiay ciiay commented Sep 23, 2026

Copy link
Copy Markdown
Member

Description

Follow-up to #4891 for RHIDP-14506. Removes PageMainContainer and the custom app/layout override in favor of a CSS-only PatternFly page inset on BackstageSidebarPage: the shell is the sole scrollport, with sticky convex corner masks so BUI Header + Container siblings (and classic <main>) share one rounded well. This restores compatibility with the default upstream Backstage layout / theme upload path while keeping the PF inset look.

Fixed

  • RHIDP-14506 — Address Backstage UI theme feedback from UX (Shiran)

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)

Made with Cursor

Replace the PageMainContainer layout wrapper with SidebarPage scroll
and sticky convex corner masks so BUI siblings share one PatternFly-style
well while staying compatible with the default Backstage app layout.

Fixes: https://redhat.atlassian.net/browse/RHIDP-14506
Signed-off-by: Yi Cai <yicai@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@rhdh-gh-app

rhdh-gh-app Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name Package Path Changeset Bump Current Version
app-legacy workspaces/theme/packages/app-legacy none v0.0.0
app workspaces/theme/packages/app none v0.0.0
@red-hat-developer-hub/backstage-plugin-theme workspaces/theme/plugins/theme minor v1.3.0

@codecov

codecov Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.57%. Comparing base (ddbeedb) to head (616f7c0).
⚠️ Report is 98 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4938   +/-   ##
=======================================
  Coverage   63.57%   63.57%           
=======================================
  Files        2702     2701    -1     
  Lines      106498   106504    +6     
  Branches    30021    30034   +13     
=======================================
+ Hits        67710    67714    +4     
- Misses      36945    36947    +2     
  Partials     1843     1843           
Flag Coverage Δ *Carryforward flag
adoption-insights 84.77% <ø> (ø) Carriedforward from d73622d
ai-integrations 87.10% <ø> (ø) Carriedforward from d73622d
app-defaults 67.23% <ø> (ø) Carriedforward from d73622d
augment 46.67% <ø> (ø) Carriedforward from d73622d
boost 92.92% <ø> (ø) Carriedforward from d73622d
bulk-import 73.12% <ø> (ø) Carriedforward from d73622d
cost-management 13.56% <ø> (ø) Carriedforward from d73622d
dcm 73.47% <ø> (ø) Carriedforward from d73622d
e2e-adoption-insights 60.00% <ø> (ø) Carriedforward from d73622d
e2e-extensions 62.31% <ø> (ø) Carriedforward from d73622d
e2e-global-header 52.40% <ø> (ø) Carriedforward from d73622d
e2e-homepage 61.11% <ø> (ø) Carriedforward from d73622d
e2e-intelligent-assistant 45.57% <ø> (ø) Carriedforward from d73622d
e2e-orchestrator 49.49% <ø> (ø) Carriedforward from d73622d
e2e-orchestrator-plugin 49.48% <ø> (ø) Carriedforward from d73622d
e2e-quickstart 55.21% <ø> (ø) Carriedforward from d73622d
e2e-scorecard 49.77% <ø> (ø) Carriedforward from d73622d
e2e-theme 16.43% <ø> (+0.07%) ⬆️ Carriedforward from d73622d
extensions 58.30% <ø> (ø) Carriedforward from d73622d
global-floating-action-button 71.18% <ø> (ø) Carriedforward from d73622d
global-header 69.10% <ø> (ø) Carriedforward from d73622d
homepage 55.05% <ø> (ø) Carriedforward from d73622d
install-dynamic-plugins 73.52% <ø> (ø) Carriedforward from d73622d
intelligent-assistant 78.72% <ø> (ø) Carriedforward from d73622d
konflux 91.98% <ø> (ø) Carriedforward from d73622d
lightspeed 69.02% <ø> (ø) Carriedforward from d73622d
mcp-integrations 84.46% <ø> (ø) Carriedforward from d73622d
orchestrator 77.69% <ø> (ø) Carriedforward from d73622d
quickstart 63.74% <ø> (ø) Carriedforward from d73622d
sandbox 79.56% <ø> (ø) Carriedforward from d73622d
scorecard 88.99% <ø> (ø) Carriedforward from d73622d
theme 87.44% <71.42%> (-0.26%) ⬇️
translations 5.12% <ø> (ø) Carriedforward from d73622d
x2a 78.44% <ø> (ø) Carriedforward from d73622d

*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 ddbeedb...616f7c0. 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.

@ciiay

ciiay commented Sep 23, 2026 •

Copy link
Copy Markdown
Member Author

Hi @logonoff @christoph-jerolimov , I raised this PR as a follow-up. This solution should align up well with BUI theme structure. The original ticket got closed but I haven't updated the overlays pr yet. If this looks good we can include this in our 2.1 release.

@logonoff logonoff left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the scrollbar isn't being clipped by the page inset, and some elements are appearing outside of the main container:

Image

…lscreen

Use margin + clip-path on BackstageSidebarPage so the scrollbar follows the
rounded well, and position .fullscreen so its absolute control stays on the graph.

Co-authored-by: Cursor <cursoragent@cursor.com>

@logonoff logonoff left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the scrollbar and scroll container are independently clipped

Image

bg colour in mobile is using the wrong token in certain pages

Image

Keep page-inset layout desktop-only, but paint the content well on all
viewports so mobile no longer falls through to body --bui-bg-app.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ciiay

ciiay commented Sep 24, 2026

Copy link
Copy Markdown
Member Author

the scrollbar and scroll container are independently clipped

Image bg colour in mobile is using the wrong token in certain pages Image

Hi @logonoff , how did you test it? I tried to reproduce it with yarn start followed by yarn install and I'm not able to see the same as you showed above. For me the scrollbar looks inside of the main content only. Can you re-verify it on your side?

The mobile background color is fixed. Good catch 🤝

@logonoff

Copy link
Copy Markdown
Member

Hi @logonoff , how did you test it? I tried to reproduce it with yarn start followed by yarn install and I'm not able to see the same as you showed above. For me the scrollbar looks inside of the main content only. Can you re-verify it on your side?

The mobile background color is fixed. Good catch 🤝

Firefox 156 on macOS 27, this setting turned on

image

@ciiay

ciiay commented Sep 24, 2026

Copy link
Copy Markdown
Member Author

Hi @logonoff , how did you test it? I tried to reproduce it with yarn start followed by yarn install and I'm not able to see the same as you showed above. For me the scrollbar looks inside of the main content only. Can you re-verify it on your side?
The mobile background color is fixed. Good catch 🤝

Firefox 156 on macOS 27, this setting turned on

image

Thanks for the info. It looks like an edge case to me. On my Firefox it looks good on default settings. I wasn't able to find the "Always show scroll bar" setting on my Firefox either. Do you think we can approve this solution for removing the extra main content container?

cc. @christoph-jerolimov

image image

@logonoff

Copy link
Copy Markdown
Member

Thanks for the info. It looks like an edge case to me. On my Firefox it looks good on default settings. I wasn't able to find the "Always show scroll bar" setting on my Firefox either. Do you think we can approve this solution for removing the extra main content container?

Always show scrollbars is a macOS setting. I can see this on Chrome too

image image

@logonoff

logonoff commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

trackpad scrolling moves the page inset corners along with it

Screen.Recording.2026-09-24.at.2.27.29.PM.mov

…asks

Disable rubber-band overscroll on the SidebarPage well, and raise sticky
corner mask z-index above Backstage Header so MUI test pages keep top-left rounding.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ciiay

ciiay commented Sep 25, 2026

Copy link
Copy Markdown
Member Author

Thanks for the follow-up and for clarifying the macOS “Always show scrollbars” setting — I can reproduce that in Chrome as well.

On that scrollbar strip: this PR keeps a single CSS-only scrollport on BackstageSidebarPage (clip-path + margin inset) so we can drop PageMainContainer and stay aligned with the BUI layout (shared well for PluginHeader + Container siblings). With overlay scrollbars (the default), the bar stays inside the rounded well. With always-on / classic scrollbars, the track takes layout width on the same element as the clip, so you can get that extra strip on the right. The robust fix would be the nested outer-clip / inner-scroll wrapper again, which is what we’re trying to remove. I’d treat always-on scrollbars as an edge case relative to the default overlay behavior and accept that tradeoff for this approach.

On the trackpad / overscroll issue (inset corners moving / a second rounded block appearing above the content): that’s addressed in the latest commits — overscroll-behavior: none on the scrollport, sticky corner masks kept for the left edge (including above Backstage Header on MUI pages), and the mobile mainSection background fix from earlier.

Could you re-verify on your side with the latest branch tip, @logonoff? Especially:

  1. Trackpad overscroll at the top (no ghost well / corners shouldn’t drift)
  2. MUI v4 / v5 Tests top-left rounding
  3. Default scrollbar behavior (overlay)

Happy to discuss whether always-on scrollbars should block dropping PageMainContainer for 2.1.

cc @christoph-jerolimov @karthikjeeyar

Match hover affordance for sortable headers and document that arrow
keys (not Tab) move between headers inside the React Aria table.

Signed-off-by: Yi Cai <yicai@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@sonarqubecloud

Copy link
Copy Markdown

@karthikjeeyar karthikjeeyar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/approve
/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Sep 30, 2026
@karthikjeeyar
karthikjeeyar merged commit a6ada16 into redhat-developer:main Sep 30, 2026
17 checks passed
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.

3 participants