-
Notifications
You must be signed in to change notification settings - Fork 274
UI4: Card & Row Components #4593
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
Conversation
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.
Small changes needed.
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.
Ambitious. I assume this will happen later as more stuff gets built out?
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 tried to start doing it and then found I had to make more changes upon tackling subsequent tasks and thinking of new rules for margin handling for sub components.
Figured it would be better to just handle it at the end to make sure everything works and avoid all the rebasing, it's already almost there at this point.
src/theme/variables/edgeDark.ts
Outdated
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 usually avoid putting dimensions in the theme, and instead rely on rem to scale things.
If we are committed to having the radius in the theme for some reason, then it's weird to do theme.rem(theme.cardRadius). Maybe just return the radius in dp's, not rem's, since the theme controls the scaling already.
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.
Just checked, and the other theme dimensions are already in raw dp's, such as thinLineWidth or secondaryButtonOutlineWidth.
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 a rem, I can rename the var
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 don't think it should be rem in the first place, but the rename is also an acceptable solution.
8a2633d to
f59d9c8
Compare
f59d9c8 to
d9405cd
Compare
Largely copied from Tile. - Converted to a functional component - Removes the bottom divider to let the caller handle dividers
Largely copied from Card. - Styled according to UI4 guidelines - Adds new functionality to automatically add styled dividers when multiple children are given.
d9405cd to
d95b628
Compare
Uh oh!
There was an error while loading. Please reload this page.
See branches future'd with this branch for additional visuals.
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have: