PolymerElements / paper-button

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

Vertically align text within paper-button by default. #113

Closed freshp86 closed 8 years ago

freshp86 commented 8 years ago

I am trying to set a custom height on my paper-button and still have the text within the button vertically aligned, see https://jsfiddle.net/db67dmem/. Is there a way to do this?

freshp86 commented 8 years ago

Figured out a way as shown at https://jsfiddle.net/z3s8fxku/2/. Still would like to hear if there is a recommended way of doing so.

keanulee commented 8 years ago

@freshp86 There's not really a recommended way (whatever works!), though in this case I would rather set padding-top/bottom of paper-button instead of height.

freshp86 commented 8 years ago

Vertically centering text within a paper-button seems to me that it is the majority use case and maybe should be the default behavior. Anyway thanks for taking a look.

freshp86 commented 8 years ago

Could you please re-consider addressing this bug? Especially for the reason that vertically aligning the text within a button is the default behavior for the native <button> element, see https://jsfiddle.net/ogyaj487/.

Shouldn't paper-button be matching that behavior as close as possible? Having to come up with custom solutions for something so basic is not a great developer experience.

freshp86 commented 8 years ago

/cc @danbeam @tjsavage

This is related to https://bugs.chromium.org/p/chromium/issues/detail?id=595804#c10, where a custom height on the dialog buttons is requested.

danbeam commented 8 years ago

@freshp86 what if you just change height to line-height in your fiddle? https://jsfiddle.net/db67dmem/1/

keanulee commented 8 years ago

line-height would work, but it would be the height within the padding (e.g. 100px - padding-top - padding-bottom). The default padding (which in turn determines height) is calculated based on the font-size of the text inside the button, so if you set the font-size smaller then the height of the button will also shrink. I don't know how strict this requirement is, but this should give you something close to 32px (paper-button styles were after all derived from some iteration of Material Design spec). I can continue to look into better ways of doing this, but let me know if this works for you.

freshp86 commented 8 years ago

@danbeam: I tried using line-height to achieve vertical alignment. Besides the calculation being a bit tricky, it only works when you have a single line. If I make the button shorter in length to force the text to wrap it breaks. For example try adding width: 50px; at https://jsfiddle.net/db67dmem/

keanulee commented 8 years ago

@freshp86 You can also use flexbox to vertically center the text:

    paper-button {
      background-color: lightblue;
      height: 100px;
      width: 50px;
      display: inline-flex;
      align-items: center;
    }
freshp86 commented 8 years ago

@keanulee: Thanks for the inline-flex suggestion, seems to work much better than line-height. I will use that for crbug.com/595804, but the question still remains in my mind. Why is this not the default behavior of paper-button? Do you think it would be too hard to implement? Shall I even attempt coming up with a PR?

keanulee commented 8 years ago

Ping @notwaldorf @cdata - Do you think there will be an issue replacing:

display: inline-block;

with:

display: inline-flex;
align-items: center;

I don't think that'll break anything, but wanted to see if there's any glaring issue I'm missing.

freshp86 commented 8 years ago

@notwaldorf @cdata: Any updates on this? See previous comment.