Skip to content

fix(catalog): enable independent scrolling for filter panel#2475

Open
manaswinidas wants to merge 7 commits intokubeflow:mainfrom
manaswinidas:fix/filter-panel-independent-scroll
Open

fix(catalog): enable independent scrolling for filter panel#2475
manaswinidas wants to merge 7 commits intokubeflow:mainfrom
manaswinidas:fix/filter-panel-independent-scroll

Conversation

@manaswinidas
Copy link
Copy Markdown
Contributor

@manaswinidas manaswinidas commented Mar 25, 2026

Description

Enables independent scrolling for the filter panel in Model Catalog, preventing users from losing their filtering context when scrolling through model results.

Previously, the filter panel and content area scrolled together, making it difficult for users to maintain context of their filtering selections while browsing models. The filter panel now uses sticky positioning with its own scroll container.

Changes:

  • Added sticky positioning to filter panel (SidebarPanel)
  • Set max-height: 100vh with overflow-y: auto
  • Filter panel now scrolls independently from content area

How Has This Been Tested?

Manual verification:

  1. Navigate to Model Catalog
  2. Reduce browser window height to trigger scrolling
  3. Scroll through model results
  4. Verify filter panel remains visible and scrolls independently
  5. With many filters applied, verify filter panel scrolls separately
  6. Test at various viewport sizes

Merge criteria:

  • All the commits have been signed-off (To pass the DCO check)
  • The commits have meaningful messages
  • Automated tests are provided as part of the PR for major new functionalities; testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work.
  • Code changes follow the kubeflow contribution guidelines.
  • For first time contributors: Please reach out to the Reviewers to ensure all tests are being run, ensuring the label ok-to-test has been added to the PR.

If you have UI changes

  • The developer has added tests or explained why testing cannot be added.
  • Included any necessary screenshots or gifs if it was a UI change.
  • Verify that UI/UX changes conform the UX guidelines for Kubeflow.

🤖 Generated with Claude Code

Makes the filter panel in Model Catalog scroll independently from the
content area, preventing users from losing their filtering context when
scrolling through models.

The filter panel now uses sticky positioning with its own scroll
container, matching the original design intent where filters remain
accessible while browsing model results.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Manaswini Das <dasmanaswini10@gmail.com>
@manaswinidas
Copy link
Copy Markdown
Contributor Author

/retest

Copy link
Copy Markdown
Contributor Author

@manaswinidas manaswinidas left a comment

Choose a reason for hiding this comment

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

/lgtm

But needs a second opinion - here's a screen recording that will help the upcoming reviewers decide

Screen.Recording.2026-03-25.at.8.28.55.PM.mov

@google-oss-prow
Copy link
Copy Markdown
Contributor

@manaswinidas: you cannot LGTM your own PR.

Details

In response to this:

/lgtm

But needs a second opinion - here's a screen recording that will help the upcoming reviewers decide

Screen.Recording.2026-03-25.at.8.28.55.PM.mov

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@google-oss-prow
Copy link
Copy Markdown
Contributor

@Philip-Carneiro: changing LGTM is restricted to collaborators

Details

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Address review feedback to use PatternFly's native sticky variant
instead of custom CSS positioning, following PF best practices.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Manaswini Das <dasmanaswini10@gmail.com>
@manaswinidas
Copy link
Copy Markdown
Contributor Author

Updated to use PatternFly's variant="sticky" prop instead of custom positioning. Thanks for the feedback!

>
<Sidebar hasBorder hasGutter>
<SidebarPanel>
<SidebarPanel variant="sticky" style={{ maxHeight: '100vh', overflowY: 'auto' }}>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

note that scrollbar consumes some width from the actual sidebar, so a little of the border of the things inside are cut out as the image bellow:

Image

@google-oss-prow
Copy link
Copy Markdown
Contributor

@Philip-Carneiro: changing LGTM is restricted to collaborators

Details

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@manaswinidas
Copy link
Copy Markdown
Contributor Author

Thanks for pointing this out! The scrollbar does consume some width, but I think the current width behavior is acceptable. The sidebar layout adjusts naturally and the content remains accessible. If this becomes a UX concern during testing, we can revisit with additional width adjustments, but for now the default behavior should work well.

Add scrollbar-gutter: stable to reserve space for the scrollbar,
preventing layout shift and content cutoff when the scrollbar appears.
This maintains constant sidebar width regardless of scroll state.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Manaswini Das <dasmanaswini10@gmail.com>
@manaswinidas
Copy link
Copy Markdown
Contributor Author

You're absolutely right about the width issue! I've added scrollbar-gutter: 'stable' which reserves space for the scrollbar even when it's not visible. This prevents the layout shift and content cutoff you observed - the sidebar width now stays constant regardless of whether the scrollbar is showing or not. This is the best of both worlds: independent scrolling without layout shift. 👍

Adds padding-right to filter content to ensure borders and content
are not clipped by the scrollbar. This maintains proper spacing
between filter components and the scrollbar edge.

Removed scrollbar-gutter approach in favor of content padding,
which provides better visual consistency.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Manaswini Das <dasmanaswini10@gmail.com>
@manaswinidas
Copy link
Copy Markdown
Contributor Author

You're right - the previous approaches still had the scrollbar taking width. I've switched to a simpler solution: added paddingRight to the filter content itself. This ensures the filter components have proper spacing from the scrollbar edge, preventing any border cutoff, while keeping the implementation clean and simple. The scrollbar naturally takes its space, but content stays properly spaced. 👍

Copy link
Copy Markdown
Contributor

@mturley mturley left a comment

Choose a reason for hiding this comment

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

@manaswinidas I'm still seeing the edge of the sidebar get cut off. Interestingly, most of the contents have the correct width but it's just the perf view toggle box that is too wide. Here's a screenshot, and then another scrolled to the right:

Image Image

The 280px minWidth was causing the card to overflow the sidebar width
when the scrollbar was present. Removing the constraint allows the card
to naturally fit within the sidebar and respect container width limits.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Manaswini Das <dasmanaswini10@gmail.com>
@manaswinidas
Copy link
Copy Markdown
Contributor Author

Good catch! The issue was the ModelPerformanceViewToggleCard had a hardcoded minWidth: '280px' which was causing it to overflow when the scrollbar was present. I've removed this constraint so the card naturally fits within the sidebar width. The other filter components didn't have this issue because they didn't have a fixed minimum width. Should be resolved now! 👍

@manaswinidas
Copy link
Copy Markdown
Contributor Author

Now it looks like this(human message):
Screenshot 2026-03-26 at 5 16 01 PM

@Philip-Carneiro
Copy link
Copy Markdown
Contributor

/lgtm
/approve

@google-oss-prow
Copy link
Copy Markdown
Contributor

@Philip-Carneiro: changing LGTM is restricted to collaborators

Details

In response to this:

/lgtm
/approve

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@google-oss-prow
Copy link
Copy Markdown
Contributor

@Philip-Carneiro: changing LGTM is restricted to collaborators

Details

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@manaswinidas
Copy link
Copy Markdown
Contributor Author

/approve

@google-oss-prow
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: manaswinidas, Philip-Carneiro

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@manaswinidas
Copy link
Copy Markdown
Contributor Author

manaswinidas commented Mar 30, 2026

@mturley can you LGTM if this looks right ?

@mturley
Copy link
Copy Markdown
Contributor

mturley commented Mar 30, 2026

@manaswinidas I'm so sorry for the nitpick, but can we slightly increase the width of the sidebar so the model performance view toggle text still doesn't wrap? It would look closer to the design

Set SidebarPanel width to 300px to ensure the "Model performance view"
toggle text doesn't wrap, keeping the layout closer to the design while
maintaining a reasonable sidebar width.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Manaswini Das <dasmanaswini10@gmail.com>
@manaswinidas
Copy link
Copy Markdown
Contributor Author

No worries at all! I've increased the sidebar width to 300px using PatternFly's width prop. This gives the "Model performance view" toggle text enough room to display without wrapping while keeping the sidebar at a reasonable size. 👍

@google-oss-prow google-oss-prow bot added size/S and removed size/XS labels Mar 31, 2026
PatternFly's width prop only accepts percentage values (width_25, width_33, etc.),
not pixel values. Using inline style to set a fixed 300px width instead.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Manaswini Das <dasmanaswini10@gmail.com>
@manaswinidas
Copy link
Copy Markdown
Contributor Author

Fixed TypeScript error - PatternFly's width prop only accepts percentage values (width_25, width_33, etc.), so I've used inline style to set the fixed 300px width instead. Tests should pass now! ✅

@google-oss-prow google-oss-prow bot added size/XS and removed size/S labels Mar 31, 2026
Copy link
Copy Markdown
Contributor

@mturley mturley left a comment

Choose a reason for hiding this comment

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

@manaswinidas sorry to continue the nitpicking - it seems like the 300px width doesn't have any effect, and even if I edit the style in my browser to be wider it's not changing the width. The toggle card still is narrow enough that its text wraps and doesn't look like it used to. Testing in mock mode PF theme:

Screenshot from your branch:
Screenshot 2026-04-03 at 4 41 23 PM

Screenshot from main, wider with the icon and text all inline with the toggle:
Screenshot 2026-04-03 at 4 41 51 PM

This might require some tinkering by hand, AI isn't great at CSS stuff. Happy to take a closer look if you want a hand troubleshooting.

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