googlearchive / paper-dialog

A dialog à la Material Design
19 stars 16 forks source link

Fix up demo and css #1

Closed ebidel closed 10 years ago

ebidel commented 10 years ago

Reviewer: @morethanreal

morethanreal commented 10 years ago

I just made a change in paper-button to use the "label" property instead of textContent to label the button. Do you mind updating that too before I merge this?

ebidel commented 10 years ago

Sure. Out of curiosity, what's the reason not to support textContent? It's intuitively how <button> is used.

Could it be label as default content?

<content>{{label}}</content>
morethanreal commented 10 years ago

I refactored button so the FAB and icon button can extend from it and you can also easily make a button with an icon and some text. That caused a styling issue, because the icon and text needs different margins and I can't directly style the text node:

<!-- If I style outer, both the icon and text gets the same margin -->
<div id="outer">
    <core-icon></core-icon>
    <!-- If I style inner, there's some extra space if the content is empty -->
    <div id="inner"><content></content></div>
</div>

Maybe there's a different way to solve this problem, like using <template if>, but I also thought it doesn't make sense to put anything other than text inside this button since it already contains a lot of styling. I made textContent forward to label and print out a warning, also.

ebidel commented 10 years ago

+1 on the warning. It's too bad you can't use #inner:empty with <content>.

Anyways, updated the demo page to use label.

morethanreal commented 10 years ago

Perfect, thanks!