-
Notifications
You must be signed in to change notification settings - Fork 5
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
Fire two events when logging in #536
Comments
Hm, I am not sure if I follow regarding the original issue. Polymer flushes property effects after each cycle, but should update all property values immediately. Nonetheless, 2 events is a better solution than a random |
The issue was that we listen to the |
Ah I see. Yes let's do 2 events then.
Op vr 17 nov. 2017 om 11:59 schreef Martijn Janssen <
[email protected]>:
… The issue was that we listen to the logged-in event to save the user
data. The problem with this is that we wanted to take action on the fact
that the user was logged in, but since the eventlistener for this is closer
to the origin of the event, the code reacting to this event was executed
before the user data had bubbled up and propogated down. The user was not
fully logged in yet due to the fact how events work.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#536 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFrDb1Rhy6jujpioXyKqIPdyhMHUkFQzks5s3WccgaJpZM4QhOmq>
.
|
We can fix this by changing the event we fire from the This should be done for all |
This applies for the
lancie-login-form
but I'm placing it here since majority of the changes should be made and all behaviour should be checked in this repository.Currently we use one event to notify that the user is logged in, while this is correct, we can improve in predictability. I had to add an
async
call recently to wait for all the user's properties to propogate down in to the dom, which is pretty wierd and unwanted.I propose a change renaming the
logged-in
event tologin-data
containing the current data in the event. Immediately after, we should fire a newlogged-in
event. This ensures that all properties have propogated down the dom (since the eventhandlers trigger immediately on an event) before taking any acions that depend on the user being completely logged-in.What do you think about this @TimvdLippe?
The text was updated successfully, but these errors were encountered: