normalize UI component font styles #9694
Conversation
Builds ready [80bdbd5]
Page Load Metrics (368 ± 39 ms)
|
| @@ -228,15 +227,6 @@ input[type="submit"][disabled] { | |||
| opacity: 0.5; | |||
| } | |||
|
|
|||
| button.primary { | |||
There was a problem hiding this comment.
this is unused in the codebase and is thus removed.
| @@ -26,13 +26,6 @@ | |||
| align-items: center; | |||
| } | |||
|
|
|||
| &__message { | |||
There was a problem hiding this comment.
This was unused and thus removed
| font-weight: 500; | ||
| line-height: 2rem; |
There was a problem hiding this comment.
| margin-right: 1.5rem; | ||
| } | ||
|
|
||
| &__subtitle { | ||
| @include H6; |
There was a problem hiding this comment.
this results in a small change to font size. @Gudahtt also reviewed this change here:
There was a problem hiding this comment.
These changes all make sense and I'm very happy that we can properly structure font sizes -- yay!
One thing that does concern me is the number of "headings" we have; H9 feels like a sight for sore eyes. I'd love if we could get down to less overall sizes and have a more uniform look and feel.
|
@darkwing -- although they are labeled as H7-H9, they are not used as headings (typically 🙃). I have also asked the design team to consider alternative, descriptive labels for these font settings, such as "subtitle," "label," etc... My experience comes from material design typography settings, which have more overall variations in font size but with a distinct purpose. Cc @MetaMask/design. In the previous PR that this was split from, @rachelcope also pointed out many areas where we are using the entirely wrong font settings. Still, this PR's goal was to improve visibility of which typography styles are in use. I hope that helps for additional context, and thanks for the review! |
|
adding @Gudahtt as a required reviewer on all of these as he had already done an extensive review on the massive PR. |
Builds ready [ff078d4]
Page Load Metrics (411 ± 59 ms)
|
|
Though I did notice that this seems to require a rebase! The first commit is a duplicate of the one from the branch this was pointed at. I bet the other four need to be rebased as well. |
brad-decker
force-pushed
the
normalize-ui-component-fonts
branch
from
ff078d4 to
635056b
Compare
|
@Gudahtt good catch, rebased. |
Builds ready [635056b]
Page Load Metrics (371 ± 60 ms)
|
Depends on #9693
Breaking apart #9074 so that it allows for easier review in parts. #9074 was reviewed a few times and changes were made but the latest rebase puts it into a state where the full review is once again needed. This part of the effort focuses on UI components. This PR has the broadest implications given that it touches the UI library which is used throughout the site.