fix(material-experimental/mdc-typography): update mdc components typography to match spec #21676

mmalerba · 2021-01-22T18:30:33Z

This PR contains several change that bring the MDC-based components in line with the spec w.r.t typography.

It adds a new typography config generation function that creates a config based on values from MDC itself. This results in changes to some typography levels, most noticeable the body-1 and body-2 levels. Since the config is based on MDC's values it contains rem measurements rather than px measurements that Angular Material as traditionally used. In this PR the new config is used in the dev-app only for the MDC-based components. The MDC-based config is currently not compatible with the old components, but it will be made compatible in a followup PR.
Switches all the MDC components to read 2018 typography levels if they are available in the config. If they are not available, the 2014 levels are mapped to 2018 levels so the MDC components can consume them.
Corrects several places where the wrong typography level was being used (body-1 looks very similar to subtitle-1 which lead to them being used interchangeably).
Corrects some sizing styles for the MDC-based form-field so it looks right with the new typography levels.

Demo: https://mmalerba-demo.web.app/mdc-autocomplete

CARETAKER NOTE: mmalerba will handle syncing

crisbeto · 2021-01-26T21:36:40Z

src/material-experimental/mdc-autocomplete/_autocomplete-theme.scss

+      // MDC uses the `subtitle1` level for list items, but the spec shows `body1` as the correct
+      // level.
+      .mat-mdc-option {
+        @include mdc-typography(body1, $query: $mat-typography-styles-query);


These seem to be the same between autocomplete and select. Maybe we should do it once inside the option theme instead?

I didn't see an existing mixin for option typography

There's mat-mdc-option-typography. Also now that I'm thinking about it, we likely need the same typography change for mat-optgroup since it uses the same size as the option.

Oh hmm, don't know how I missed that, I'll move it

I just took a look through the external & internal specs as well as demos, and I don't see anything that mentions menu/option groups. I guess maybe we'll just have to pick something reasonable looking for now

The groups are set up to look like options, except that they aren't clickable, so the same styles can be applied.

I think they're currently set to be subtitle1 which makes sense, after the typography levels are updated it'll wind up looking nearly identical to the options (except for the letter spacing) https://material.io/design/typography/the-type-system.html#type-scale

Funnily enough, the link you just sent has an example of an option group which is somewhat different than what we have. Anyway, my comment was mostly to raise awareness that we might want to change the option groups as well, I don't have an opinion on what they should be changed to.

I created a PR based on this one that uses a config created from MDC's typography settings: #21709 That should give an idea of how things should eventually end up looking. Though looking at it, the form-field appears to need a little more work

It looks like they're using the overline level for the option group, we may want to do that too but I'll leave it for another PR.

mmalerba · 2021-01-28T20:27:52Z

I discussed with Jeremy and we decided its best to roll the function to create a typography config from MDC's values into this PR. I've updated it and added a demo of the latest code in the PR description.

jelbourn · 2021-01-28T21:04:25Z

Does the mdc-paginator look weird?

I feel like the label and the select aren't in proportion

mmalerba · 2021-01-28T21:16:54Z

Yeah it does look a little weird. I think the paginator is just applying the dense form-field styles (which does not change the font size). I can try adding some overrides though

mmalerba · 2021-01-28T21:54:04Z

added an override for the paginator and updated the demo

jelbourn · 2021-01-28T21:20:40Z

src/material-experimental/mdc-core/option/_option-theme.scss

+    // MDC uses the `subtitle1` level for list items, but the spec shows `body1` as the correct
+    // level. Class is repeated for increased specificity.
+    .mat-mdc-option {
+      @include mdc-typography(body1, $query: $mat-typography-styles-query);


Do we have a bug filed for this?

We have an internal one filed against our gmat components - the difference is almost imperceptible with the external spec, just the letter spacing is different. I haven't opened an bug against MDC yet.

jelbourn · 2021-01-28T21:21:43Z

src/material-experimental/mdc-form-field/_form-field-subscript.scss

+    position: absolute;
+    top: 0;
+    left: 0;
+    right: 0;


Is it intentional to not include bottom here?

Yes, the idea is to behave like the non-MDC form-field where the user can add multiple error/hint or long ones that wrap, but they'll have to reserve space for it themselves. We only reserve space in the layout for one line worth of message.

We should maybe talk about this- I think we should change our behavior over to what mdc does, but I know that would be pretty breaking, so we should think about doing a follow-up that lets users switch.

jelbourn · 2021-01-28T21:32:09Z

src/material-experimental/mdc-form-field/_mdc-text-field-structure-overrides.scss

    border: none;
  }

+  // In order to ensure proper alignment of the floating label, we reset its line-height.
+  // The line-height is not important as the element is absolutely positioned and only has one line


Instead of saying "is not important", would it be accurate to say "doesn't affect the size of the element" ?

not really, it does affect the size of the element - its just that we absolutely position the element, so the size doesn't affect the rest of the layout, and its not important as far as the line spacing since labels are only one line anyways

src/material-experimental/mdc-helpers/_mdc-helpers.scss

src/material-experimental/mdc-typography/_all-typography.scss

google-cla · 2021-01-29T00:39:59Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

…refer 2018 typography mappings if the config is compatible

config from MDC

…correct typography levels

jelbourn

LGTM

typography

select

… added by angular#21676

… added by #21676 (#21772)

… added by #21676 (#21772) (cherry picked from commit fc86fb5)

… added by angular#21676 (angular#21772)

angular-automatic-lock-bot · 2021-03-05T00:14:17Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 22, 2021

mmalerba force-pushed the mdc-typog-g3 branch 3 times, most recently from 08b60ab to 46a63f6 Compare January 26, 2021 21:20

mmalerba force-pushed the mdc-typog-g3 branch from 46a63f6 to 9ea2b49 Compare January 26, 2021 21:28

mmalerba changed the title ~~fix(material-experimental/mdc-typography): fix always using mat-level~~ fix(material-experimental/mdc-typography): use correct typography levels Jan 26, 2021

mmalerba marked this pull request as ready for review January 26, 2021 21:31

mmalerba requested review from crisbeto and devversion as code owners January 26, 2021 21:31

mmalerba assigned crisbeto and andrewseguin Jan 26, 2021

mmalerba requested a review from andrewseguin January 26, 2021 21:31

crisbeto reviewed Jan 26, 2021

View reviewed changes

crisbeto approved these changes Jan 26, 2021

View reviewed changes

mmalerba changed the title ~~fix(material-experimental/mdc-typography): use correct typography levels~~ fix(material-experimental/mdc-typography): update mdc components typography to match spec Jan 28, 2021

mmalerba force-pushed the mdc-typog-g3 branch from f8ffaf2 to 57459a5 Compare January 28, 2021 20:16

mmalerba assigned jelbourn Jan 28, 2021

jelbourn reviewed Jan 28, 2021

View reviewed changes

mmalerba force-pushed the mdc-typog-g3 branch from e488100 to fa8e7a2 Compare January 29, 2021 00:38

mmalerba requested a review from a team as a code owner January 29, 2021 00:38

google-cla bot removed the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 29, 2021

google-cla bot added the cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla label Jan 29, 2021

mmalerba added 3 commits January 28, 2021 16:40

fix(material-experimental/mdc-typography): change mdc components to p…

b8cbd87

…refer 2018 typography mappings if the config is compatible

feat(material-experimental/mdc-typography): add a function to create

6e9dc70

config from MDC

fix(material-experimental/mdc-typography): update components using in…

8992161

…correct typography levels

mmalerba force-pushed the mdc-typog-g3 branch from fa8e7a2 to ba8d5d6 Compare January 29, 2021 00:41

google-cla bot added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Jan 29, 2021

jelbourn approved these changes Feb 1, 2021

View reviewed changes

mmalerba force-pushed the mdc-typog-g3 branch from ba8d5d6 to 12d9064 Compare February 1, 2021 22:59

mmalerba added the action: merge The PR is ready for merge by the caretaker label Feb 1, 2021

mmalerba added 2 commits February 1, 2021 17:01

fix(material-experimental/mdc-form-field): tweak styles to work with MDC

e4bddef

typography

fix(material-experimental/mdc-paginator): use caption typography for

63ac863

select

mmalerba force-pushed the mdc-typog-g3 branch from 12d9064 to 63ac863 Compare February 2, 2021 01:01

mmalerba merged commit d7ec42c into angular:master Feb 2, 2021

mmalerba added a commit to mmalerba/components that referenced this pull request Feb 2, 2021

fix(material-experimental/mdc-form-field): remove extraneous sass var…

04c3abf

… added by angular#21676

mmalerba mentioned this pull request Feb 2, 2021

fix(material-experimental/mdc-form-field): remove extraneous sass var… #21772

Merged

wagnermaciel pushed a commit that referenced this pull request Feb 2, 2021

fix(material-experimental/mdc-form-field): remove extraneous sass var…

fc86fb5

… added by #21676 (#21772)

wagnermaciel pushed a commit that referenced this pull request Feb 2, 2021

fix(material-experimental/mdc-form-field): remove extraneous sass var…

7b7bbfb

… added by #21676 (#21772) (cherry picked from commit fc86fb5)

wagnermaciel pushed a commit to wagnermaciel/components that referenced this pull request Feb 8, 2021

fix(material-experimental/mdc-form-field): remove extraneous sass var…

34bfa5f

… added by angular#21676 (angular#21772)

wagnermaciel pushed a commit to wagnermaciel/components that referenced this pull request Feb 8, 2021

fix(material-experimental/mdc-form-field): remove extraneous sass var…

ea7322b

… added by angular#21676 (angular#21772)

angular-automatic-lock-bot bot locked and limited conversation to collaborators Mar 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(material-experimental/mdc-typography): update mdc components typography to match spec #21676

fix(material-experimental/mdc-typography): update mdc components typography to match spec #21676

mmalerba commented Jan 22, 2021 •

edited

Loading

crisbeto Jan 26, 2021

mmalerba Jan 26, 2021

crisbeto Jan 26, 2021

mmalerba Jan 26, 2021

mmalerba Jan 26, 2021

crisbeto Jan 26, 2021

mmalerba Jan 26, 2021

crisbeto Jan 26, 2021

mmalerba Jan 26, 2021

mmalerba Jan 26, 2021

mmalerba commented Jan 28, 2021

jelbourn commented Jan 28, 2021 •

edited

Loading

mmalerba commented Jan 28, 2021

mmalerba commented Jan 28, 2021

jelbourn Jan 28, 2021

mmalerba Jan 29, 2021

jelbourn Jan 28, 2021

mmalerba Jan 29, 2021

jelbourn Feb 1, 2021

jelbourn Jan 28, 2021

mmalerba Jan 29, 2021

google-cla bot commented Jan 29, 2021

jelbourn left a comment

angular-automatic-lock-bot bot commented Mar 5, 2021

fix(material-experimental/mdc-typography): update mdc components typography to match spec #21676

fix(material-experimental/mdc-typography): update mdc components typography to match spec #21676

Conversation

mmalerba commented Jan 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmalerba commented Jan 28, 2021

jelbourn commented Jan 28, 2021 • edited Loading

mmalerba commented Jan 28, 2021

mmalerba commented Jan 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

google-cla bot commented Jan 29, 2021

jelbourn left a comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Mar 5, 2021

mmalerba commented Jan 22, 2021 •

edited

Loading

jelbourn commented Jan 28, 2021 •

edited

Loading