Plugin filters adjustments - #39
Merged
Merged
Conversation
The filters panel was a fixed w-80 with a height-capped tag list, so a catalog with many tags scrolled a 160px box while the panel itself had room to grow. Widen it in steps and let the tag list take the remaining height, then clamp the panel to what Radix measured between the trigger and the viewport edge (--radix-popover-content-available-height, plus collisionPadding) so it can never run off screen at any width. The tag list moves from a fieldset to role=group: a rendered legend is not subtracted from the height flex assigns its fieldset, so the scroll box overflowed it. It also lays out as a grid rather than CSS columns, because a height-capped multi-column box overflows sideways into new columns instead of scrolling down. Tag labels fill their row so the tap target clears 44px on touch. The popover primitive gains the clamp for every consumer, which is the right default — HeaderQuickNav, VisibilityInfoPopover, TeamsTable and TokensTable are all narrow enough to be unaffected today. scrollbar-thin gives the tag list a transparent track so the bar does not paint over the rounded corners of the box it scrolls in. Signed-off-by: Anna Effort <anna.effort@ibm.com>
The docked sidebar took 16rem out of a 768px viewport, leaving list toolbars roughly 30rem to lay out in — enough that content had to wrap across the whole md range. Move the dock to lg so tablet widths get the sheet instead, which is the arrangement they had room for anyway. useIsMobile takes the breakpoint as a parameter so the sidebar can ask for 1024 without moving the 768 default every other caller relies on. SIDEBAR_COLLAPSE_BREAKPOINT has to stay in step with the lg: utilities that show the docked sidebar, or the sheet and the dock render at once. color-scheme tells the browser which palette to paint native UI with — scrollbars, form controls, the caret. Without it the .dark class swaps our custom properties but leaves that chrome light, which is most visible on the scrollbars the sidebar and page share. Signed-off-by: Anna Effort <anna.effort@ibm.com>
marekdano
requested changes
Aug 18, 2026
marekdano
left a comment
marekdano
left a comment
Contributor
There was a problem hiding this comment.
Findings
1. Missing vertical gap in the plugin filters tag grid (visual regression)
- Importance: High — should fix before merge
- File:
src/components/plugins/PluginToolbar.tsx:170 - Issue: The tag list was converted from a flex column with
space-y-2to a CSS grid with onlygap-x-4— no row gap. - Impact: In the plugin catalog filters popover, tag checkbox/label rows now render with 0px vertical spacing between them (grid row-gap defaults to 0), so tags visually run together. Previously
space-y-2gave 8px between rows. - Fix: Use
gap-4(both axes) or addgap-y-2alongsidegap-x-4.
2. Sidebar action buttons still key off md: (768px) after the mobile/desktop split moved to lg: (1024px)
- Importance: Medium — should fix, not blocking
- File:
src/components/ui/sidebar.tsx:408, 537, 539 - Issue:
SIDEBAR_COLLAPSE_BREAKPOINTwas raised to 1024 and most sidebar classes were updatedmd:→lg:, butSidebarGroupAction(408) andSidebarMenuAction(537, 539) still usemd:after:hidden/md:opacity-0. - Impact: Between 768–1023px viewport width, the sidebar renders as a touch Sheet (
isMobiletrue), but these two components still disable their enlarged touch hitbox and default-hide their hover-only actions at 768px — behaving as "desktop/hover" while the sidebar itself is in "mobile/touch" mode. This is exactly the risk the PR's own new comment (line 27) warns about; 3 of themd:usages were missed. - Mitigating factor: Neither component is currently rendered anywhere in the app (no call sites outside
sidebar.tsx/sidebar.test.tsx), so this is a latent bug in shared primitives, not a live regression today — hence medium rather than high.
3. Outer popover scroll container doesn't get the new scrollbar-thin treatment
- Importance: Low — nice to fix, non-blocking
- File:
src/components/ui/popover.tsx:30 - Issue: The PR adds
overflow-y-autoto the basePopoverContent, but only the inner tag list inPluginFiltersPopovergets the newscrollbar-thintransparent-track utility. - Impact: On very short viewports where a popover's total content exceeds available height, the outer panel scrolls with the OS-default scrollbar, which can paint a square edge over the panel's
rounded-mdcorners — the exact defectscrollbar-thinwas introduced to avoid, just not applied here. Edge case only.
Signed-off-by: Anna Effort <anna.effort@ibm.com>
There was a problem hiding this comment.
Pull request overview
Improves responsive plugin filtering, toolbar layouts, sidebar behavior, and native theme styling.
Changes:
- Adds responsive, scrollable plugin filters and compact toolbar rows.
- Moves the mobile sidebar breakpoint to
lg. - Constrains popovers to viewport space and updates native theme chrome.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/index.css |
Adds native color schemes and scrollbar utility. |
src/hooks/use-mobile.ts |
Supports configurable breakpoints. |
src/components/ui/sidebar.tsx |
Moves sidebar collapse behavior to lg. |
src/components/ui/popover.tsx |
Adds viewport collision and size constraints. |
src/components/server-catalog/CatalogToolbar.tsx |
Keeps search and filters on one row. |
src/components/plugins/PluginToolbar.tsx |
Adds responsive filter sizing, tag grid, and toolbar layout. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
a-effort
force-pushed
the
plugin-filters-adjustments
branch
from
August 18, 2026 07:30
57b34b5 to
aa00851
Compare
Spacing adjustments added; good suggestion! The container query thresholds never matched the panel they measure. Size queries resolve against the content box, which p-4 and the border make 2rem+2px narrower than the width utilities, so 18/24/32rem of panel leave 15.875/21.875/29.875rem to query against. @sm therefore never fired at lg and @XL never fired at all, capping the list at one column on lg and two on xl. @xs and @md sit inside those widths with room to spare. SidebarGroupAction and SidebarMenuAction kept their md: variants after the collapse breakpoint moved to lg, so between 768 and 1023px they dropped the enlarged touch hitbox and hid hover-only actions while the sidebar itself was rendering as a touch sheet. Neither has call sites outside the tests today, so this was latent rather than a live regression. PopoverContent gained overflow-y-auto without scrollbar-thin, so a panel tall enough to scroll paints an OS scrollbar over its rounded corners, which is the defect the utility was added to avoid. useIsMobile's new breakpoint parameter had no coverage; both added tests fail against a hook that ignores it. Signed-off-by: Anna Effort <anna.effort@ibm.com>
Contributor
Author
|
@marekdano all three fixed in de63635. Ty!
The same row-gap defect existed in the server catalog popover and is fixed in #40, along with a related container-query issue Copilot caught: size queries resolve against the content box, so the multi-column thresholds in both popovers were unreachable and the tag list was capped at one column on |
marekdano
approved these changes
Aug 18, 2026
marekdano
left a comment
marekdano
left a comment
Contributor
There was a problem hiding this comment.
The PR looks good!
LGTM 🚀
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Popover styling adjustments:
Resize:
Commit summary:
dbda5f0: Filters panel grows with the viewport (stepped widths, flex-height tag list) instead of scrolling a 160px box; tag list switches to role="group" + a container-query grid so it scrolls down rather than overflowing, with 44px tap targets. Adds a viewport clamp to the popover primitive and a scrollbar-thin utility.
5219d9f: Docked sidebar moves from md to lg so tablet widths get the sheet; useIsMobile takes an optional breakpoint (768 default kept). Declares color-scheme per theme so native chrome follows dark mode.
57b34b5: Retunes the popover (end-aligned, narrower steps, shorter tag list) and un-stacks the search row in both the plugin and catalog toolbars into a single flexing row. Reverses dbda5f0's alignment choice.