Billing UI feature update #72
Conversation
- data-table table class - data-list definition list class - update tabs with improved mobile layout - add select to input patterns - predefined input length classes - emoji class for better alignment
| @@ -61,3 +61,66 @@ table.card { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| // Billing UI data-table | |||
There was a problem hiding this comment.
Can these styles not be used anywhee? Labelling them for a specific page seems at odds with the objective of reusable styles.
There was a problem hiding this comment.
These are used throughout the billing pages (The entire app) so in that sense they are reused style.
There was a problem hiding this comment.
But are they reusable outside of the billing app? If so this comment is falsely restrictive.
| @@ -109,6 +109,7 @@ del, .neg { | |||
| .center { text-align: center; } | |||
| .right { text-align: right; } | |||
| .monospace { font-family: $mono-stack; } | |||
| .emoji { vertical-align: middle; } | |||
There was a problem hiding this comment.
One of the takeaways from k82 was that we shouldn't use emojies. But I guess there's no value ina dding that note here, since devs wouldn't see it while consuming the styles anyway. 🤔
There was a problem hiding this comment.
¯\_(ツ)_/¯ they are used in billing for currency – I agree in principle but this at least would ensure alignment w/ text.
| max-width: 100%; | ||
| overflow-x: auto; | ||
| overflow-y: hidden; | ||
| @media #{$screen-width-medium} { overflow: visible; } |
There was a problem hiding this comment.
Running the Peak site locally, I can't see any behavioural difference here..
-edit-
Okay, when I run WB billing I can see the overlfow working. Peak demo site will need an update on the tabs page to actually show this functionality.
There was a problem hiding this comment.
yep, I have a WIP PDS branch that covers this change and will show the data-table and data-list.
scss/base/tables.scss
Outdated
| overflow-x: auto; | ||
| th, td { | ||
| background-clip: padding-box; | ||
| border: 1px solid $neutral-700; |
There was a problem hiding this comment.
This is quite a chunky line. 500 would be better.
There was a problem hiding this comment.
This is just based on whe was currently implemented in cliff's Billing UI.
scss/base/tables.scss
Outdated
| dl.data-list { | ||
| margin-bottom: 1rem; | ||
| > div { | ||
| border: 1px solid $neutral-700; |
There was a problem hiding this comment.
500 is also better here. Even 300 works great here, but 300 might be too light for the other one.
pairs with: https://github.com/WealthBar/wealthbar/pull/1925 and WealthBar/peak-design-system#76
Summary
Submitter Checklist:
Reviewer Checklist: