app-layout icon indicating copy to clipboard operation
app-layout copied to clipboard

app-drawer align="start" not working as expceted on RTL

Open freshp86 opened this issue 9 years ago • 3 comments

There seems to be a race condition at the https://github.com/PolymerElements/app-layout/blob/master/app-drawer/app-drawer.html#L324 method. At the time it executes on startup window.getComputedStyle(this).direction always returns 'ltr' and the drawer is wrongly positioned on the left. If I trigger a _resetPosition manually from the developer tools, then the drawer is properly positioned on the right.

freshp86 avatar Jun 20 '16 17:06 freshp86

_resetPosition/_isRTL is run after the drawer is attached (observers: [ '_resetPosition(align, isAttached)' ]), so if dir isn't set by then, you'll need to reset the position yourself. Also note that we changed the default of align to 'left' because there' a performance hit when calling getComputedStyle, so I would suggest setting align to left/right instead of start/end.

http://jsbin.com/zedose/edit?html,output

keanulee avatar Jun 20 '16 17:06 keanulee

Thanks for the suggestion I switched my code to manually set left/right as needed. Feel free to close this bug.

I think though that documentation should mention how isRTL is determined and the fact that users should ensure that 'dir' is set before this element is attached. Also by "need to reset the position yourself" do you mean calling resetLayout()? I did not try, but looking at the code it does not seem to trigger _resetPosition(), and 'position' property is readOnly, so the only way to reposition is to set the align property to some value.

freshp86 avatar Jun 20 '16 18:06 freshp86

I agree, it's not intuitive that you need to set direction before attached, but there's no efficient way of "listening" for when direction changes after attached. I did mean reseting the align property, but setting it back to the same value (e.g. start) won't cause _resetPosition to be called. Maybe _resetPosition should be be made public.

keanulee avatar Jun 20 '16 18:06 keanulee