-
Notifications
You must be signed in to change notification settings - Fork 157
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
fix inconsistency in listing components #3984
Conversation
…o 3898-inconsistency-in-list-components
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.
Code LGTM. Please update the PR description to read like a release note 👍
border-radius: 15px; | ||
white-space: nowrap; | ||
background-color: ${(props) => props.theme.colors.neutralGray}; | ||
`; | ||
|
||
const LabelFlex = styled(Flex)` | ||
padding: ${(props) => props.theme.spacing.xxs} 0; | ||
overflow-x: scroll; |
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.
Does the component still scroll on overflow after removing this?
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.
I use wrap with Flex so whenever it's overflowing it'll be displayed in multiple lines
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.
Yep - I see now that you added wrap
to the Labels - thanks!
Closes #3898
What changed?
#3898 (comment)
Why was this change made?
Throughout the different functionalities of the Demo we have different ways of organizing the information in lists.
From my point of view, the options with two columns are clearer. I would leave it as the standard component but reduce the spacing between columns.
redirect3-2023-09-03_14.08.21.mp4