Conversation
- Add flex-wrap when col=false to allow filter chips to wrap properly - Remove unnecessary wrapper div around TimeRangeReadOnly - Add items-center for proper vertical alignment - Aligns with FilterChipsReadOnly layout behavior Fixes APP-683 Co-authored-by: eric.okuma <eric.okuma@rilldata.com>
|
Cursor Agent can help with this pull request. Just |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on February 25
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| comparisonTimeRange={comparisonRange | ||
| ? { expression: comparisonRange } | ||
| : undefined} | ||
| /> |
There was a problem hiding this comment.
Wrapper removal breaks column layout for time chips
Medium Severity
Removing the wrapper div around TimeRangeReadOnly changes layout behavior when col=true. The TimeRangeReadOnly component renders two sibling Chip elements (time range and comparison). Previously, the flex gap-x-2 wrapper kept these chips side-by-side regardless of parent layout. Now they become direct children of the flex-col container and stack vertically when there's a comparison range. This affects DefaultFilterDisplay.svelte which uses the default col=true and passes comparisonRange.
…e-by-side The wrapper ensures time range and comparison chips stay side-by-side regardless of whether the parent is in column (col=true) or row mode. This preserves correct layout for DefaultFilterDisplay which uses col=true. Co-authored-by: eric.okuma <eric.okuma@rilldata.com>
| class:flex-col={col} | ||
| class="flex gap-y-2 gap-x-2 w-full flex-none" | ||
| class:flex-wrap={!col} | ||
| class="flex gap-y-2 gap-x-2 w-full items-center" |
There was a problem hiding this comment.
These changes are not needed. Just class:flex-wrap={!col} was enough.
Lets not change other stuff since this component is used in canvas component as well.
Only adds class:flex-wrap={!col} to allow filter chips to wrap properly
when displayed horizontally (col=false) in the bookmarks modal.
Fixes APP-683
Co-authored-by: eric.okuma <eric.okuma@rilldata.com>
AdityaHegde
deleted the
cursor/APP-683-bookmarks-modal-canvas-display-fd10
branch
* fix: fix broken bookmarks modal layout in Canvas
- Add flex-wrap when col=false to allow filter chips to wrap properly
- Remove unnecessary wrapper div around TimeRangeReadOnly
- Add items-center for proper vertical alignment
- Aligns with FilterChipsReadOnly layout behavior
Fixes APP-683
Co-authored-by: eric.okuma <eric.okuma@rilldata.com>
* fix: restore wrapper div for TimeRangeReadOnly to keep time chips side-by-side
The wrapper ensures time range and comparison chips stay side-by-side
regardless of whether the parent is in column (col=true) or row mode.
This preserves correct layout for DefaultFilterDisplay which uses col=true.
Co-authored-by: eric.okuma <eric.okuma@rilldata.com>
* fix: add flex-wrap for horizontal layout in Canvas bookmarks modal
Only adds class:flex-wrap={!col} to allow filter chips to wrap properly
when displayed horizontally (col=false) in the bookmarks modal.
Fixes APP-683
Co-authored-by: eric.okuma <eric.okuma@rilldata.com>
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
3 participants
Fixes APP-683
The Bookmarks Modal in Canvas appeared broken due to
CanvasFilterChipsReadOnly.sveltenot properly wrapping filter chips when displayed horizontally (col={false}). This was caused by a missingflex-wrapclass and an unnecessary wrapperdivaroundTimeRangeReadOnly.This PR adds
flex-wrapfor horizontal layouts,items-centerfor alignment, removesflex-none, and removes the redundantTimeRangeReadOnlywrapper, aligning its behavior withFilterChipsReadOnly.svelte.Checklist:
Linear Issue: APP-683
Note
Improves layout of read-only filter chips in Canvas.
class:flex-wrap={!col}and switches container classes toflex ... items-center(removesflex-none) to enable wrapping/alignment in horizontal modeTimeRangeReadOnly, inlining its conditional blockWritten by Cursor Bugbot for commit 9e9b5ed. This will update automatically on new commits. Configure here.