should not be displayed #28
Conversation
|
📊 Lookin' good 👍 |
| @@ -69,6 +69,10 @@ | |||
| } | |||
|
|
|||
| }, | |||
|
|
|||
| attached: function() { | |||
| this.style.display = 'none'; | |||
There was a problem hiding this comment.
could we add this as a :host style, instead of via JS?
There was a problem hiding this comment.
ATM <iron-iconset-svg> doesn't have a shadow root, so I didn't think I should add one. @cdata?
There was a problem hiding this comment.
word, I wonder if using created instead of attached then might be better, just to shove this as early as possible in the life cycle of the elm, to prevent crazy weird style/height flicker
There was a problem hiding this comment.
👍 I think you're right that we might end up creating an unnecessary shadowRoot. Alternatively you could also set hostAttributes to include hidden by default. This may be more expensive overall though..
There was a problem hiding this comment.
@cdata do y'all have a good way to measure "cost" yet?
There was a problem hiding this comment.
Not in a repeatable CI setting, no. We have gotten good mileage out of the Chrome timeline profiler though.
Fixes #27 (see bug for repro)
Added two tests, look at them failing before the patch:
