Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Paper-button does not follow MD typographical styles #129

Open
diogocamposwd opened this issue Jul 19, 2016 · 5 comments
Open

Paper-button does not follow MD typographical styles #129

diogocamposwd opened this issue Jul 19, 2016 · 5 comments

Comments

@diogocamposwd
Copy link

https://material.google.com/components/buttons.html#buttons-style

@diogocamposwd
Copy link
Author

(sigh) Just saw #95. Sorry.

Anyway:

  • Paper-button is missing a dependency (paper-styles/typography.html) to satisfy the use of "--paper-font-common-base".
  • Not applying the proper "--paper-font-button" mixin makes paper-button to miss:
    • white-space: nowrap;
    • overflow: hidden;
    • text-overflow: ellipsis;
    • font-size: 14px;
    • letter-spacing: 0.018em;
    • line-height: 24px;
    • text-transform: uppercase;*
  • Changing the font-width on focus makes the paper-button itself to expand, which causes a weird visual/layout behavior.
  • The focus style in the MD spec (I suppose) is fine. If correct:
    • The library developers should provide defaults that work (the spec defaults, in this case).
    • The app developers should take care of not choosing color combinations that hurt a11y.
  • If changing 'background-color' really really will never work (spec problem?), I think a (dashed?) border simulating a good old outline can be better than a 'font-weight' change.

And, since I'm here: why paper-button is setting paddings, margins and width in 'em' units instead of CSS pixels?

Sorry for bad English. Hugs.

@notwaldorf
Copy link
Contributor

Just a quick note: as I mentioned on the PR, not importing typography.html is intentional -- this way you can use the Roboto font if you want (by explicitly including typography.html in your app), but by default, the button would inherit whatever font you were using.

@diogocamposwd
Copy link
Author

Interesting.

BTW, it can also be done with:

:root {
    --paper-font-common-base: {
        font-family: 'Comic Sans', sans-serif;
        -webkit-font-smoothing: antialiased;
    };
}

@notwaldorf
Copy link
Contributor

Sure, but by then you've already downloaded the Roboto font (if you make paper-button always use it), only to un-use it. That's extra problematic if your app is offline, becausetypography.html tries to download it from google fonts. That's a whole network request and response you don't need

@RhysyG
Copy link

RhysyG commented Oct 14, 2016

Hopefully this is somewhat related. In the paper-button demo the height of the button is 35.59 including the touchable target, however in MD spec it says the height should be 48px including touchable target? Also, when using the button from Polymer starter kit which sets the body line-height to 1.5, it ends up being 40.59. Am I interpreting this wrong or are there some inconsistency here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants