-
Notifications
You must be signed in to change notification settings - Fork 163
Bookmarks modal canvas display #8650
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Bookmarks modal canvas display #8650
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 <[email protected]>
|
Cursor Agent can help with this pull request. Just |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 <[email protected]>
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.