Skip to content
This repository has been archived by the owner on Jul 1, 2024. It is now read-only.

Add email loading state until user info resolved #1098

Merged
merged 1 commit into from
Jan 12, 2015

Conversation

deckar01
Copy link
Contributor

Expose a function in session that returns a promise that resolves to the user private info.
Add a loading state and an error state to the email setting.

Fixes #1059.

@deckar01 deckar01 force-pushed the email-loading-state branch from de83699 to 4f00c6f Compare January 12, 2015 18:08
@bartekn
Copy link
Contributor

bartekn commented Jan 12, 2015

Pulled & tested 👍

@thejollyrogers
Copy link
Contributor

Here's an idea:
instead of creating new promise waitingForUserInfo and creating a new method (we already have a method getUser() in session), make User.load return a promise and store it in the session cache.

session.js line 88 do:

cache.set("userPrivateInfo", UserPrivateInfo.load...)

then just call getUser() inside email settings, and it will return either the unresolved promise or the user object.

@deckar01
Copy link
Contributor Author

@thejollyrogers I agree that is how it should have been designed in the first place, but I don't want to change getUser(), because I would have to change all 108 usages which would significantly increase the surface area of this PR.

deckar01 pushed a commit that referenced this pull request Jan 12, 2015
Add email loading state until user info resolved
@deckar01 deckar01 merged commit 7d67a9b into master Jan 12, 2015
@jedmccaleb jedmccaleb removed the pr label Jan 12, 2015
@deckar01 deckar01 added the pr label Jan 12, 2015
@deckar01 deckar01 deleted the email-loading-state branch January 12, 2015 21:02
@thejollyrogers
Copy link
Contributor

if we're taking the time to fix it now, shouldn't we take the time to do it right rather than moving forward with redundancy, different implementations, and confusion?

@deckar01
Copy link
Contributor Author

@thejollyrogers This PR is to fix #1059, not refactor the way the entire app waits for user data. Opened issue #1129 to unify the code paths.

@thejollyrogers
Copy link
Contributor

Okay, thank you

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

Successfully merging this pull request may close these issues.

Email settings hidden after refreshing a page
4 participants