Skip to content
This repository has been archived by the owner on Dec 19, 2024. It is now read-only.

Commit

Permalink
Use flex to center text both horizontally and vertically (#115)
Browse files Browse the repository at this point in the history
  • Loading branch information
keanulee committed May 23, 2016
1 parent 5a2a837 commit dddb834
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions paper-button.html
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,15 @@
<template strip-whitespace>
<style include="paper-material">
:host {
display: inline-block;
@apply(--layout-inline);
@apply(--layout-center-center);
position: relative;
box-sizing: border-box;
min-width: 5.14em;
margin: 0 0.29em;
background: transparent;
-webkit-tap-highlight-color: rgba(0, 0, 0, 0);
-webkit-tap-highlight-color: transparent;
text-align: center;
font: inherit;
text-transform: uppercase;
outline-width: 0;
Expand Down

6 comments on commit dddb834

@jannakha
Copy link

@jannakha jannakha commented on dddb834 May 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that broke so many things! do you even test?
bring back text-align: center;

@keanulee
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jannakha Can you give us a reproduction? From what I see --layout-center-center gives the same result: http://jsbin.com/qedecequpa/edit?html,output

@jannakha
Copy link

@jannakha jannakha commented on dddb834 May 25, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NO it doesn't, especially when you have a bit more complex layout inside the button with icons.
You've added inline-flex as well! here's a before and after screenshots:

before:
screen shot 2016-05-25 at 10 00 38 am

after:

screen shot 2016-05-25 at 10 06 56 am

There are more important bugs (some of which my team has reported) in Polymer than layout of paper button. We even did some pull requests - and ZERO response. It's a bit frustrating :-(

@keanulee
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jannakha The reason why we switched to inline-flex was because we wanted to, by default, vertically center button contents (#113). We definitely did not intend for this to be a breaking change, and we didn't foresee this case. Going forward, to accomplish the vertical layout you've shown, I would suggest applying flex-direction: column (or @apply(--layout-vertical); @apply(--layout-inline);); see http://jsbin.com/guteqeteka/edit?html,output . Please feel free to file/bump any outstanding issues/PRs so that the element maintainer can respond to them.

@jannakha
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have already fixed this issue with @apply(--layout-vertical); in my project. Just something unexpected.

@valdrinkoshi
Copy link
Member

@valdrinkoshi valdrinkoshi commented on dddb834 Dec 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change broke use cases like the following one:

<style>
  .container { display: flex; flex-direction: column; }
  paper-button { text-align: left; }
</style>
<div class="container">
  <paper-button>hello</paper-button>
</div>

Previously, the button text will be aligned to the left, after this change, the whole button will be centered inside the container.
To fix this, the user is now required to reset display: inline-block on the paper-button

<style>
  .container { display: flex; flex-direction: column; }
  paper-button { display: inline-block; text-align: left; }
</style>
<div class="container">
  <paper-button>hello</paper-button>
</div>

Please sign in to comment.