search

PolymerElements / paper-button

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

Ensure hidden attribute works. #123

Closed coreyfarrell closed 8 years ago

coreyfarrell commented 8 years ago

Fixes #122.

keanulee commented 8 years ago

See https://github.com/PolymerElements/paper-button/issues/119#issuecomment-227209514. In particular, [hidden] styling (which is debatable whether it should be used for styling by anything other than the browser) should be implemented at the element-user level.

coreyfarrell commented 7 years ago

As noted in MDN, the hidden attribute does not hide an element if another display property is set on that element (https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/hidden).

I don't follow the logic that the default style of a polymer element should break the hidden attribute. I'm not setting the display attribute as an element user so I should not need to reset it everywhere for hidden to work. hidden is defined as a global attribute so the default style should respect it. A similar commit has already been accepted for paper-input and paper-checkbox.

Another fix would be to move @apply(--layout-inline); to a separate selector :host(:not([hidden])). That's similar to how I was initially going to fix paper-input but @notwaldorf pointed out that positive selectors are easier to read than :not selectors..

I've updated my commit, sorry for the cut/paste error with disabled instead of hidden. I'm guessing it doesn't appear here because the PR is currently closed, but it's visible from #122.

keanulee commented 7 years ago

Technically, with :host([hidden]) { display: none !important } you would be breaking a user who does paper-button { display: block; } ... <paper-button hidden> (since display is set, the button should appear, but !important has precendence), but I suppose that's a less popular use case. Feel free to open a new PR for the fixed commit.