-
Notifications
You must be signed in to change notification settings - Fork 291
fix: header, sidebar UI and animations #5429
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?
Conversation
- Remove max-width constraints on feed container for full-width layout - Reduce default grid gap from gap-8 (32px) to gap-4 (16px) - Unify page padding to laptop:p-10 on all sides Co-authored-by: Cursor <[email protected]>
- Make header fixed instead of sticky for YouTube-like behavior - Add hidden scrollbar that shows on hover - Improve sidebar collapse animation with synchronized 300ms transitions - Adjust sidebar padding and font sizes for consistency - Make plus icon always visible in section headers Co-authored-by: Cursor <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Add aria-label to sidebar toggle button for accessibility - Fix section title opacity class for collapsed state - Update test to match renamed "Squads" section Co-authored-by: Cursor <[email protected]>
- Cast isVisible.current to boolean for aria-expanded - Remove unsupported variant prop from CreatePostButton Co-authored-by: Cursor <[email protected]>
packages/shared/src/components/sidebar/sections/CustomFeedSection.tsx
Outdated
Show resolved
Hide resolved
| grid-template-columns: 100%; | ||
| } | ||
| .container { | ||
| @screen laptopL { |
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.
Was this change intentional? It basically makes all sizes the same then? Before this would take user settings for preference into account.
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.
Yes I think it's better and makes the product simpler from a settings perspective.
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.
We can but then we have to maybe remove the settings wdyt?
rebelchris
left a comment
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.
Generic:
- There's a layout jump in sidebar if you collapse/expand it now.
- Remove density settings from appearance page (hardcode to 'eco') - Debounce sidebar state change to prevent layout jump during animation - Use proper link for 'Add to Feeds' button instead of router.push() Co-authored-by: Cursor <[email protected]>
|
@rebelchris I fixed everything you mentioned. |
Reverts the removal of max-width calculation that was causing cards to stretch too wide on large displays. Co-authored-by: Cursor <[email protected]>
Cards now use full width on screens below 2560px, and only get constrained to 20rem per card on ultra-wide/4K displays. Co-authored-by: Cursor <[email protected]>
Aligns with design system by using the highest defined breakpoint (2156px) instead of arbitrary 2560px value. Co-authored-by: Cursor <[email protected]>
rebelchris
left a comment
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.
Looks good to me now.
@claude do one sanity review to see if anything has breaking changes.
|
Claude finished @tsahimatsliah's task in 1m 49s —— View job Sanity Review for Breaking ChangesI've reviewed all 21 changed files. Here's my assessment:
|
Summary
Test plan
Preview domain
https://feed-layout-improvements.preview.app.daily.dev