PolymerElements / paper-button

A button à la Material Design
https://www.webcomponents.org/element/PolymerElements/paper-button
138 stars 64 forks source link

apply --paper-font-common-base #89

Closed valdrinkoshi closed 8 years ago

valdrinkoshi commented 8 years ago

Fixes #86 by setting paper-font-common-base to the paper-button

valdrinkoshi commented 8 years ago

Actually, since both paper-button and paper-card include the styles from paper-material, it makes more sense to add this styling to paper-material. Closing this

notwaldorf commented 8 years ago

Hmm I don't think it makes sense. Paper-material isn't rendering any text, it's just a lifted piece of paper. paper-button on the other hand, is.

valdrinkoshi commented 8 years ago

I think paper-material should also bring the official paper font. I see paper-material as the paper div :)

notwaldorf commented 8 years ago

But paper-material isn't displaying any text. It's just a lifted piece of paper. I think it's weird to have a typography import given that it doesn't have any typography.

/cc @cdata for input

cdata commented 8 years ago

paper-material also has the unfortunate distinction of being a performance anti-pattern.

I agree that paper-material is kind of a Material div element. However a div does not impose a font family, and as a primitive it feels strange to thing that a pretty drop shadow forces a pretty font at the same time. This is just my personal opinion.

valdrinkoshi commented 8 years ago

My expectation would be that whenever I use a paper-* element, all textual content that is displayed should have the right font. e.g.

<paper-material> This is the Paper div </paper-material>

I expect all texts to be in Roboto out-of-the-box.

Without typography, paper-material looks like this:

screen shot 2016-01-12 at 12 18 45 pm

WDYT?

notwaldorf commented 8 years ago

I think that is true only to the extend that the element is meant to convey text. paper-material is only a shadow, and that's already material (it's got a well defined elevation according to the spec). I would not expect an element that is not using any typography to have any opinions about typography (paper-spinner, for example, should also not have this import) :(

valdrinkoshi commented 8 years ago

uhm, so should we expect the user to set the style in this case?

<style>
  * {
    font-family: Arial;
  }
</style>
<paper-material> The font needs to be set by the user </paper-material>

Also, for cases like paper-card, should Roboto be applied like this?

<style>
  * {
    font-family: Arial;
  }
</style>
<paper-card heading="Heading"> <== Roboto
   <div> Hello </div> <== Arial
   <div class="card-content"> Content </div> <== Roboto
</paper-card>
notwaldorf commented 8 years ago

IMO, with paper-card you know you're displaying text (that's what card does. it promises you a header and to format your text according to https://www.google.com/design/spec/components/cards.html), so it makes sense for paper-card to internally force the font to Roboto.

For paper-material, yes, I would agree with expecting the user to style it. You don't know what the user will put in there (maybe it's an image, maybe it's their own content), and the element doesn't require/promise anything in its contract, so it's on the user to manage it.

valdrinkoshi commented 8 years ago

sounds good, then reopening this PR & will prepare one for paper-card too.

notwaldorf commented 8 years ago

Sorry, hadn't seen these had gotten updated. I don't think we should be doing the import here (talked offline)

valdrinkoshi commented 8 years ago

@notwaldorf I've updated the PR to only apply the mixin :+1:

notwaldorf commented 8 years ago

LGTM 🚢