paper-dialog-scrollable-1.1.2 moved from flex to fit layout breaking some pages

[ikelos]

trafficstars

I've only been using polymer for a little while, but I had a component that offered a fixed height list down one side of a larger scrollable pane. It was convenient to put the list in its own scrollable container (since it can sometimes be significantly longer than the page it's attached to), and up until version 1.1.2, it appeared that paper-dialog-scrollable did not require a paper-dialog (see issue #15).

Having upgraded to 1.1.2 (and 1.1.3 and 1.1.4) all the pages using that component broke, because the paper-scrollable-dialog has a height of 0 and was not inside anything implementing the paper-dialog behaviour. Examining the code, this now seems to force a fit layout, rather than the previous flex layout, and I have yet to find a way to gracefully override that from outside the component. One workaround would require housing it within a relative, fixed-size component, but this will then sometimes be too large (when the list is empty) or too small (when the list reaches the scroll limit).

As a standalone component it was very useful to be able to have an area that could grow to a maximum limit and then start scrolling with all the paper niceties. It seems as though this could be averted by not including the "else if" part of the condition in _ensureTarget. This would still allow a user to manually apply the fit layout when required, but also allow them to change the layout as they required.

If such a change isn't desired or feasible, then the documentation for using this outside of a dialog should really be updated to clarify that it now must be used within a relative-position, fixed-size element (much like the flex-layout documentation points out).

[valdrinkoshi]

Hi @ikelos, paper-dialog-scrollable sets the fit class to #scrollable, not to the host, which is needed to have the #scrollable take the full width/height of its parent (the host, paper-dialog-scrollable). When this element is used inside a paper-dialog, we don't need to set this class, as it will be the sizing target, hence it will be used to calculate the max-width/max-height. We use fit instead of flex because the latter causes rendering problems in IE11 (see this issue)

If you want to use this outside a paper-dialog, you can still do it, just make sure you give a size to paper-dialog-scrollable, or make it flex (as it's done in the demo page).

Hope it helps, closing this for now. Will reopen if you think the problem persists :)

[ikelos]

Yep, that's perfect thanks! I was misusing flex (not in a vertical layout, so the expansion wasn't happening as I expected). It is still a slightly unexpected change to existing components, but given that the workaround is reasonably trivial I'm not too concerned. Thanks again for the quick response! 5:)

[valdrinkoshi]

No problem, happy to help! Thinking more of this, probably we could better handle the case when paper-dialog-scrollable is not contained in a paper-dialog, but the name already suggests its usage and the plan is to move this element into paper-dialog. I'll reopen this one anyways, and assign a lower priority ;)

[valdrinkoshi]

@ikelos if you feel like it, feel free to propose a fix for this, here's our contributing guidelines.

[ikelos]

Given there was reason behind the layout change and it was simply me misusing some of the other components that caused me problems, I'm not sure I'm best placed to contribute back a reasonable fix! 5:)

Given its utility, it seems like a shame to hide it away in the paper-dialog component though. It appears like it might be well suited to cards as well as dialogs (although I'm not sure if fits in with the design philosophy of paper-card)...