[User Settings] Add new Profile view

[Maftalion]

Details

• Added new personal details update form to profile page
• Created new checkbox component

Fixed Issues

Fixes https://github.com//issues/1711

Tests

• Go to settings/profile, modify details
• Click "save" and refresh app to confirm changes took effect

Tested On

• Web
• Mobile Web
• Desktop
• iOS
• Android

Video

Screen.Recording.2021-03-13.at.1.44.57.PM.mov

Web

Mobile Web

Desktop

iOS

Android

[Maftalion]

How are pages with keyboards normally handled?

[Maftalion]

@shawnborton updated

[nickmurray47]

Some initial comments, I think the first way of approaching the self-select option @Maftalion is good.

This might be slightly outside the scope of this particular PR @shawnborton but it seems awkward to me that after clicking Save in the profile section that nothing happens to indicate the details were actually saved. I feel like we should give the user a small pop-up like User details updated! and it can just be added as a callback from the updatePersonalDetails method.

[shawnborton]

Visually this looks good to me, will leave the code review for @nickmurray47

[Maftalion]

@nickmurray47 updated

This might be slightly outside the scope of this particular PR @shawnborton but it seems awkward to me that after clicking Save in the profile section that nothing happens to indicate the details were actually saved. I feel like we should give the user a small pop-up like User details updated! and it can just be added as a callback from the updatePersonalDetails method.

I brought this up in the proposal as well, sounds like it will be covered with follow-up work #1711 (comment)

[nickmurray47]

I brought this up in the proposal as well, sounds like it will be covered with follow-up work #1711 (comment)

Oh nice! Missed this comment, thanks for the clarification

[nickmurray47]

Works great @Maftalion!

[nickmurray47]

Think there was a weird GH bug yesterday, going to re-run the checks @Maftalion so we can get unblocked on this

[NikkiWines]

This is looking really great. Left som minor comments and style suggestions

[NikkiWines]

Other than a couple minor color alterations, this is looking great 🎉 Will re-review once those are updated and conflicts are resolved.

[shawnborton]

@NikkiWines @michelle-thompson @Maftalion I wanted to get your thoughts on some things quickly:

Do we think the Save button should be fixed to the bottom of the profile view?
I can see an argument where this matches other patterns we've established where the primary button on a view like this is fixed to the bottom. So if that's the case, we would want to make all of the Profile content scrollable but keep the button fixed, like this:

Can we conditionally show the save button?
I'm not sure if this was discussed elsewhere, but it doesn't make sense to show a Save button when nothing has changed in this view. If we always want to show a Save button, I would suggest that we make it appear disabled until a value changes, or not show it at all until a value changes.

[michelle-thompson]

Yeah, I like the idea of disabling the button until changes are detected. Since we'll also eventually add in-line buttons with the same behavior, this would make things more consistent too!

[NikkiWines]

Do we think the Save button should be fixed to the bottom of the profile view?

I do think we should fix it to the bottom and make the profile scrollable. This is helpful for users with smaller screens too.

Can we conditionally show the save button?

I don't really think we should do this. For the password page, it makes sense to have the Save button conditionally enabled because we disable the view if there's an invalid password based on discrete conditions (fields are empty, they don't match, they're not complex enough, etc.) For this page though, you can have empty fields and we don't really have any conditions that need to be matched (for now), so there's no reason you couldn't save your profile at any time.

Overall, I think it adds complexity for no real reason.

[NikkiWines]

Since this issue is holding up a couple of other ones, maybe we could think about a conditionally enabled safe button as a v2? I'd like to get this one out asap if possible.

[Maftalion]

Both variants seem fine, I think the main thing is being consistent across the app.
Here are my opinions though:

Do we think the Save button should be fixed to the bottom of the profile view?
Personally I like having the CTA always visible so the user knows what actions they can make without needing to look for it. The only drawback is on smaller devices if the form is cut-off perfectly then the user might not know there are other fields they can scroll down to.

Can we conditionally show the save button?
I tend to favor disabled buttons vs hidden buttons so a user knows that they can complete an action to "unlock" the button vs not knowing it even exists

PS. I've been replicating the button behavior on the change password screen (it's been merged recently) so if we change it here, we should probably update that screen as well.

[shawnborton]

Cool, so sounds like fix the button to the bottom, and use a disabled button instead of show/hide. I like it!

[nickmurray47]

LGTM rn, and it sounds like it'd be best for those additional UI updates to be in v2, right @NikkiWines?

[NikkiWines]

I guess it depends on how quickly @Maftalion can make those changes. @Maftalion, is that something you think you can handle today? If not maybe we can make a follow-up issue to implement this.

[Maftalion]

I can prob get to it later today, just a couple of questions.

1. By fixed button, am I just reverting it back to how it was before or something different?
2. For disabling the button, should it enable when the form is dirty or when the new values !== onyx values

[shawnborton]

If you go to create a new group in Expensify.cash, you will see the behavior we want for the fixed button.

[NikkiWines]

Two minor comments -- looks great overall! 🚀

[NikkiWines]

Looks good!

[shawnborton]

Looks good, and the scrolling works well.

One thing I noticed while testing this morning was that if I change the value in the pronoun menu, the Save button does not become enabled:

[nickmurray47]

LGTM and tests well

[NikkiWines]

Added one minor comment, also you have conflicts.

[NikkiWines]

Nice! 🎉 Going to merge since Nick approved previously