Closed sbutkovi closed 2 years ago
Simple fix. Should have a null validation state if email is undefined.
@luxaritas I was changing the password fields to account for null state as well, and I realized we don't have any password requirements. Should we at least enforce a length requirement, like 8 characters or more?
I don't have any strong opinions about that. In theory we should have some minimal requirements, but we might want to wait on that and stay consistent with the website for now.
Also: The fix here is probably to wait for the current user profile to load before showing the page
Sure, we can punt on password requirements for now. Personally, I feel the real fix is that these user fields ought to be loaded on login, not on settings request. It's three properties (email, and two notification preferences).
I just don't like our current reliance on pages requesting data on mount, because it usually leads to loading spinners and an slower-feeling app. I still maintain that handling null states is a fix worth adding, because empty password fields with a valid "Passwords match" is strange. But I will address it in the action as well.
I'd be a bit nervous about just doing data loading like that on login, as it could mean we have stale data. The way we handle it on a website is that we have some of the page (ie the nav) available, and then have the preloader in the content area (at some point, we had discussed switching that to a skeleton loader but didn't have the time to look into it). At the very least I guess, we'd want to have the input fields disabled if we're not at a valid point to have the content, though we'd still need some indicator that it's still loading.
Agreed with the passwords matched thing looking odd.
Isn't there always the risk of stale data? Someone opens the app on their phone, navigates to settings, then changes settings within the site on their computer. Both cases are admittedly niche, devil's advocate sort of situations. I can match the other pages for now (e.g., loading spinner), but something I'd like to address in next is how to approach async data loading to minimize it's impact on the user and improve perceived app responsivity (responsive-ness?)
Sure, we can't have true consistency, but it at least leaves less room for error (especially considering the website, which is an SPA, and some users may leave the tab open for days/weeks...). Certainly in general something we should discuss.
Or more specifically, we could load on initial login, display the initial data, but fetch again on page mount. Sort of an optimistic update situation. But yes, food for future thought. I'll fix it more simply right now.
When I first open the settings page, I get the following error message regarding the email address:
But after a few seconds, the current email fills itself in and the error message is removed (email address censored):