material-components / material-components-web

Modular and customizable Material Design UI components for the web
https://material.io/develop/web
MIT License
17.14k stars 2.15k forks source link

[Button] --mdc-typography-button-line-height is overridden in Button component #5812

Open e111077 opened 4 years ago

e111077 commented 4 years ago

Bug report

the typography theming system's line-height is overridden by this following line:

https://github.com/material-components/material-components-web/blob/master/packages/mdc-button/_mixins.scss#L418

Steps to reproduce

  1. Open a demo of button
  2. set --mdc-typography-button-line-height: 50px; on the button
  3. set line-height: 20px; on the button

Actual behavior

line-height is set to 20px on the label

Expected behavior

line-height should be set to 50px

Your Environment:

Software Version(s)
MDC Web canary
Browser chrome
Operating System mac os

Additional context

internal shadow parts migration

Possible solution

remove it and migrate people

e111077 commented 4 years ago

cc: @asyncLiz

asyncLiz commented 4 years ago

We originally did this to account for different font type ascenders/descenders who would get cut off with the button's line height.

Do you have a link to the clients who are setting the line height? It'd be curious to know what the use case is (or if it's a way to secretly set the height, which will come later with density for MWC).

One alternate that we could do if it's a valid use case would be to consume the same property but use inherit as the fallback.

cl/306557841 for reference.