angular / components

Component infrastructure and Material Design components for Angular
https://material.angular.io
MIT License
24.33k stars 6.73k forks source link

bug(Typography): Default font settings on `.mat-typography` is `body-2` instead of `body-1` #27112

Open avidan-chen opened 1 year ago

avidan-chen commented 1 year ago

Is this a regression?

The previous version in which this bug was not present was

14

Description

When a theme is configured according to the docs, and typography is set up by adding this line: @include mat.typography-hierarchy(mat.define-typography-config()) https://v15.material.angular.io/guide/typography#using-typography-styles-in-your-application

Applying the .mat-typography class on the body element defines body-2 as the default font for the application: https://github.com/angular/components/blob/16.0.1/src/material/core/typography/_typography.scss#L273

body-1 has a font-size of 16px. body-2 has a font-size of 14px;

The docs say: body-1 | Base body text.
body-2 | Secondary body text.
https://github.com/angular/components/blob/16.0.1/guides/typography.md?plain=1#L54

If body-1 is considered base body text, I expect it to apply when using the mat-typography css class, and not body-2.

Reproduction

  1. Follow the docs to set up typography styles in a brand new Angular + Angular Material app: https://v15.material.angular.io/guide/typography#using-typography-styles-in-your-application

  2. Add mat-typography to the body tag.

  3. Inspect font-size on the body tag.

Expected Behavior

I expect body-1 to be the default font settings which apply when using mat-typography (since the docs claim this is base body text).

Actual Behavior

body-2 is the default font settings when using @include mat.typography-hierarchy(mat.define-typography-config()) instead of body-1.

Environment

michaelfaith commented 1 year ago

Later in the docs, though, this table shows body-2 as the standard body text, which is accurate. So, is it just the table higher up that needs updating to be consistent? This is also consistent with the default body in the pre-mdc typography styles

image

avidan-chen commented 1 year ago

@michaelfaith in pre-mdc typography styles, the base body text, the default font was body-1: https://github.com/angular/components/blob/14.2.7/src/material/core/typography/_typography.scss#L268

However, the definition for body-1 was the same as for body-2 (14px) for some reason (which seems counter intuitive to me, as I'd expect body-1 to be bigger than body-2): https://github.com/angular/components/blob/14.2.7/src/material/core/typography/_typography.scss#L60

Also, in the Material specs it is clearly stated body-1 should be 16px: https://m2.material.io/design/typography/the-type-system.html#type-scale Screenshot 2023-05-31 at 9 18 31

With the new Material spec (v3), there are 3 font sizes for body, largest being 16px as well: https://m3.material.io/styles/typography/type-scale-tokens Screenshot 2023-05-31 at 9 20 44

Therefore I think the correct behavior would be that body-1 is set to 16px and not 14px.

michaelfaith commented 1 year ago

Therefore I think the correct behavior would be that body-1 is set to 16px and not 14px.

It sounds like you agree with me then, since body-1 is already set to 16px and body-2 is 14px? Your original post says that's already the case, which has been my experience too. It's just that body-2 in MDC-based Angular Material is default, which means the size is consistent with pre-MDC. If they change body-1 (16px) to be default, then that would increase everyone's default font size to 16 and likely break a lot of people. Keeping body-2 (14) as default not only makes the size consistent with pre-MDC styles, but it seems to be consistent with M3 styles, since you probably want the default to be "medium". It seems like the easiest, least impactful, and most correct thing to do would be to just change the table you referred to to match the table lower down in the page and leave body-2 (14px) as default. When they roll out M3-based styling those tables will need to be overhauled again anyway.

avidan-chen commented 1 year ago

@michaelfaith main issue here is that the font-size that's being applied to .mat-typography by default is 14px and not 16px (i.e., body-2 and not body-1).

It's just that body-2 in MDC-based Angular Material is default

which is incorrect according to the specs, as I've demonstrated in my previous comment.

If they change body-1 (16px) to be default, then that would increase everyone's default font size to 16 and likely break a lot of people

So? Breaking changes are a fact. You document it, you bump major version. Developers read the docs and understand there are breaking changes when migrating to major versions.

Keeping body-2 (14) as default not only makes the size consistent with pre-MDC styles, but it seems to be consistent with M3 styles, since you probably want the default to be "medium".

Incorrect IMO. See the links and screenshots from my previous comment.

It seems like the easiest, least impactful, and most correct thing to do would be to just change the table you referred to to match the table lower down in the page and leave body-2 (14px) as default.

Strongly disagree. I get that this change should be in the next major version since it's a breaking change, but I think if Angular Material claims it follows the Material spec., it should do so.

michaelfaith commented 1 year ago

It's just that body-2 in MDC-based Angular Material is default

which is incorrect according to the specs, as I've demonstrated in my previous comment.

How is that incorrect? The spec doesn't say which body should be default text size. It just says body-1 is 16 px, which it is, and body-2 is 14px, which it is. So it does match the spec.

Keeping body-2 (14) as default not only makes the size consistent with pre-MDC styles, but it seems to be consistent with M3 styles, since you probably want the default to be "medium".

Incorrect IMO. See the links and screenshots from my previous comment.

How is that incorrect? 14px equates to medium body in m3, which is what body-2 currently is (m2), which is default. How is that wrong? I think you're just getting tripped up by the fact that it's currently named 2. At the point where m3 rolls out, it'll likely be named body-medium, which I don't think would be problematic, would it? It would still be 14px though

So, it seems like this whole issue should just result in an adjustment in this table: https://github.com/angular/components/blob/6e51008dc1ef0715b065381c968c08560313fba5/guides/typography.md?plain=1#L44-L57

to match this table: https://github.com/angular/components/blob/6e51008dc1ef0715b065381c968c08560313fba5/guides/typography.md?plain=1#L182-L196

Notice the "Level Name" column maps directly to the M2 spec...

avidan-chen commented 1 year ago

How is that incorrect? The spec doesn't say which body should be default text size.

My pre-assumption that default body text should be body-1 was because in pre-MDC, body-1 was the default (although it was set to 14px and not 16px as it should have been).

In M2 spec, it says:

The default for modern web browsers is 16px

Taking these two into consideration, it makes more sense to me that the default behavior out of the box would be to set the font-size to 16px, but I agree, it's not clearly defined.

Aligning the docs would definitely improve the situation though.

michaelfaith commented 1 year ago

If nobody's taken it up by the weekend, I can take a stab at aligning the docs.

michaelfaith commented 1 year ago

@avidan-chen I had time to put a PR in for this, if you want to take a look and see what you think

wshager commented 1 year ago

We had the same problem, so I'd like to keep this issue active. I think this is the most confusing part:

Name Description
body-1 Base body text.
body-2 Secondary body text.

Also note that form controls have body-1 styles, while paragraphs and such have body-2 styles. This clearly differs from Angular < 15, where both had body-1 styles. So confusing and inconsistent.

marek-aguita commented 11 months ago

I think the problem is in material/core/typography/_typography.scss, where body-2 typography is applied in .mat-typography class: image

In my opinion correct would be to just change body-2 to body-1 in this scss code snippet as body-1 should be the default within .mat-typography.

HakanSvenssonNexer commented 7 months ago

I also ran into this issue and think is a bug.
@marek-aguita do you know if your suggested solution will be implemented?

One "proof" of that this does not work as expected is this https://material.angular.io/guide/typography

It says that it shall be styled as body-1 but it gets font 14px (and not 16 as body-1 should be) image

Another example: mat-card-content gives 16px and so that

also should get 16px seems logically correct (body-1)

JoshuaWi commented 6 months ago

Hi all, is there any movement on this issue? We are busy migrating from v14 - v15 and are now running into this issue where all our things that were previously set to 16px/1rem are now being changed to 14px/0.875rem because the main mat-typography that we are applying on the body tag now defaults to using mat-body-2 instead of mat-body-1 in v14 v14: image v15: image (4)

*edit

We attempted to work around the issue by swapping the old body-1 typography config for body-2 and vice versa. However, body-1 adds additional bottom margin since it's also used for h4 tags and is a heading in v15. This is causing us headaches because now where we are trying to use body-1 in place of the old body-2 we are now required to also negate the bottom margins being added.

v14 had body-1 as it's own class and as such was not adding such bottom margins: https://v14.material.angular.io/guide/typography#using-typography-styles-in-your-application v15 groups body-1 with h4: https://v15.material.angular.io/guide/typography#using-typography-styles-in-your-application

I have not seen this level of detail included any any migration guides and it fundamentally changes the look and feel of our existing application

notbenj commented 3 months ago

Hello,

I think there definitely is a breaking change here, but only for M2 projects.

To me this change (mat-body defaults to mat-body-2 instead of mat-body-1 as before) is correct for M3 projects, because in M3 specs:

But in M2 specs it's different, as explained by @avidan-chen above:

With this change, M2 projects are now broken, their mat-body class now displays 14px text instead of 16px.

I think the following fix would solve the issue:

Any news would be greatly appreciated, thank you.