Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
freshp86 opened this issue Jun 20, 2016 · 3 comments
Open

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

freshp86 opened this issue Jun 20, 2016 · 3 comments

Comments

@freshp86
Copy link

freshp86 commented Jun 20, 2016

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 freshp86 changed the title align="start" not working as expceted on RTL app-drawer align="start" not working as expceted on RTL Jun 20, 2016
@keanulee
Copy link
Contributor

_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

@freshp86
Copy link
Author

freshp86 commented Jun 20, 2016

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.

@keanulee
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants