fix: Filter chips lacks width for longer values #7025
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This pull request addresses issue #7018 by improving the design of filter chips in the ViewBarDetails component to handle longer filter values, especially in mobile viewports.
- Modified
StyledBarandStyledChipcontainercomponents inViewBarDetails.tsxto enhance layout and styling - Adjusted flex properties, padding, and overflow handling to prevent text from breaking
- Improved width allocation for longer filter values to ensure proper display
- Changes aim to resolve the issue of insufficient width for filter chips in mobile view
- Modifications may affect related components like SortOrFilterChip, EditableFilterChip, and EditableSortChip
1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
@harshit078 Thanks for the issue and PR :)
However, I'm not sure about these changes:
- I think the initial issue about avoiding wrapping within chip should be done in SortOrFilterChip direclty
- about the scrolling horizontal (which does not match your PR screenshot btw) I don't think it improves the UX, we want to keep piling the filters and sorts vertically
- I see several additions (z-index, position) that does not seem effective
- I see changes in margin and padding that do not match the figma: https://www.figma.com/design/xt8O9mFeLl46C5InWwoMrN/Twenty?node-id=15351-156515&node-type=frame&t=XctmAQ2oAV3UJRoV-0
Could you rework it?
Also make sure that it looks good on (desktop / mobile) x (with sort / without sort) x (with filter / without filter)
Thank you!
packages/twenty-front/src/modules/views/components/ViewBarDetails.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/views/components/ViewBarDetails.tsx
Outdated
Show resolved
Hide resolved
| min-height: 32px; | ||
| margin-left: ${({ theme }) => theme.spacing(2)}; | ||
| flex-wrap: wrap; | ||
| overflow-x: scroll; |
There was a problem hiding this comment.
I'm not sure about this scroll behavior, it's very difficult to use. I think we want they to pile vertically
@Bonapara could confirm
There was a problem hiding this comment.
@charlesBochet, In the figma designs and the video in #7018 the filter chips scroll horizontally behind the Reset and Save as new view state. If we remove this scroll then how should we approach it ?
There was a problem hiding this comment.
oh ok! I missed that, I feel this is not easy to use and need to be discussed again with Bonapara for small screens but we could merge the PR
There was a problem hiding this comment.
@charlesBochet, I agree to your point. To overcome this, can we can do a tooltip or information badge for all mobile viewports as a result when user first time applies filters, they can learn ?
packages/twenty-front/src/modules/views/components/ViewBarDetails.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/views/components/ViewBarDetails.tsx
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/views/components/ViewBarDetails.tsx
Outdated
Show resolved
Hide resolved
| display: flex; | ||
| align-items: center; | ||
| flex: 1; | ||
| overflow-x: hidden; |
There was a problem hiding this comment.
On the dev environment, filters stack on each other in column manner. But in figma they are stacking row wise and having a scroll. Is column stack behaviour we need to follow ?
Description
This PR solved the issue #7018
StyledBarandStyledChipcontainercomponents.Before
After
Screen.Recording.2024-09-15.at.1.22.06.AM.mov