Combine edit profile and edit home in one page with tabs #1200

thdk · 2021-05-09T17:31:27Z

It closes #949 at least partially and is the start of some changes to the profile pages to better match the design from figma.

There now is a single page with two tabs. One tab for 'about' and another one for 'home'. I've added routes so you can directly navigate to a specific tab. Also after saving the form, you'll be redirected to the related view with the updated information.

BREAKING CHANGE: the username is now REQUIRED in profile urls.

Example routes:
user/aapeli
user/aapeli/about (same as user/aapeli)
user/aapeli/home
user/edit/aapeli
user/edit/aapeli/home
user/edit/aapeli/about (same as user/edit/aapeli)

When current route is user/aapeli/home and user clicks the 'edit' button,
then user the route is changed to user/edit/aapeli/home.
Next when the user clicks 'save', the route is changed again to user/aapeli/home.

So you stay on the profile section you were editing/viewing.

Let me know if this is not what we want, I have little experience with best practices for routes.
My argument for having the user id in the url is:

you won't have issues if there is a user that uses 'edit', 'home' or 'about' as username
users can copy paste their url from their profile page and send it to other people.

Regarding the Account Settings button. There is an option to add the account settings as another tab on the edit profile page. However, @ItsiW said it actually belongs in the hamburger menu so I left it as is for now. On the top right of the edit profile page.

Frontend checklist

Formatted my code with yarn format && yarn lint --fix
There are no warnings from yarn lint
There are no console warnings when running the app
Added any new components to storybook
Added tests where relevant
=> I can probably still write (or copy) the existing test that we now have for edit hosting preferences for edit profile tab.
All tests pass
Clicked around my changes running locally and it works
Checked Desktop, Mobile and Tablet screen sizes

BREAKING CHANGE: the username is now REQUIRED in profile urls. Example routes: user/aapeli user/aapeli/about (same as user/aapeli) user/aapeli/home user/edit/aapeli user/edit/aapeli/home user/edit/aapeli/about (same as user/edit/aapeli) When current route is user/aapeli/home and user clicks the 'edit' button, then user the route is changed to user/edit/aapeli/home. Next when the user clicks 'save', the route is changed again to user/aapeli/home. So you stay on the profile section you were editing/viewing."

lucaslcode

This is much better, thanks!

app/frontend/src/routes.ts

app/frontend/src/components/Navigation/Navigation.tsx

app/frontend/src/features/profile/edit/EditHostingPreference.tsx

app/frontend/src/features/profile/edit/EditProfilePage.tsx

app/frontend/src/features/profile/hooks/useUpdateUserProfile.ts

app/frontend/src/features/profile/view/Overview.tsx

ItsiW

Looks good, waaay cleaner now. But we need username to not be required in URLs.

For your first point, we can prevent specific usernames in the backend. Good point on wanting to share profile, we can satisfy this and the requirement with:

/user/ redirects to /user/aapeli
/user/about redirects to /user/aapeli/about
/user/home redirects to /user/aapeli/home

The reason for this is that we link to profile and editing pages in a number of places so that needs to be generic.

I don't think either point applies to edit now. The only use case for sharing is people showing others how to edit their own profile, so it would be better not to have the username in that URL at all. It's sufficient to just have

/user/edit
/user/edit/about
/user/edit/home

Also, and might just be a me thing, I couldn't see the About/Home tabs immediately. Could be a question for UX because I understand we want to be consistent.

darrenvong · 2021-05-10T01:28:30Z

Agreed with what's said here already. In particular, we shouldn't have user/edit/<username> as that creates a lot of impossible URLs a normal user won't have access to (since the URL implies they can edit another user when they can't) so I think the best thing is to not have them altogether.

thdk · 2021-05-11T06:00:27Z

Wow, you are all very fast with reviewing! 👍

I've already adjusted the routing issues and will continue the other remarks as well as merging develop branch tonight.

aapeliv · 2021-05-14T13:32:44Z

Hi, just a heads up, if you change the routes, please remember to fix up also //docs/urls.md and //app/backend/src/couchers/urls.py. There's a bunch of onboarding emails going out on Sunday, so I just want to make sure they don't lead to 404s!

lucaslcode · 2021-05-14T21:51:28Z

I'm assuming this is ready for review now - remember to request so we know! :)

lucaslcode

Nice work!

There's a variable accidental rename and just a few other nits

app/frontend/src/features/profile/edit/EditProfilePage.tsx

app/frontend/src/features/profile/hooks/useUpdateHostingPreferences.ts

app/frontend/src/features/profile/hooks/useUpdateUserProfile.ts

app/frontend/src/routes.ts

app/frontend/src/features/profile/view/ProfilePage.tsx

thdk · 2021-05-15T13:20:26Z

@aapeliv @darrenvong @lucaslcode thank you all for reviewing.

I'm still busy with this PR. I had some trouble on finding the best routes with optional usernames.

/User/home (for route user/username?/tab?)

Still don't know how the frontend can now for such route whether 'home' is a user or a tab on the profile page.

Except for hardcoding something like:

const user,tab = useParams()

If user === home || user === about
tab = user
user = undefined
end if

This long weekend I was visiting my family in law in Czechia so I pushed my changes already in case I would have some spare time here :)

This week Ill have some more time again. I'll work on your feedback and add tests.

thdk · 2021-05-17T07:42:43Z

Hi, just a heads up, if you change the routes, please remember to fix up also //docs/urls.md and //app/backend/src/couchers/urls.py. There's a bunch of onboarding emails going out on Sunday, so I just want to make sure they don't lead to 404s!

Ah didn't know about these, so good that you mentioned it. I've added the new routes to both these files. But not sure if they should support the 'tab' parameter as well. Let me know if changes are required.

The typescript overloads were a little overkill here indeed. Having two seperate functions greatly simplifies it.

darrenvong · 2021-05-17T21:48:46Z

Visiting other profiles is completely broken - the URL says "aapeli" but it's still showing the test user's profile and it seems to the same for all profiles

app/frontend/src/components/TabBar/TabBar.tsx

aapeliv · 2021-05-23T20:13:07Z

@thdk: how're you doing on this one?

thdk · 2021-05-23T20:51:57Z

I have no computer with me this weekend as I'm on a family trip. I'll finish it beginning of this week. Op zo 23 mei 2021 22:13 schreef Aapeli Vuorinen ***@***.***>:

…

@thdk <https://github.com/thdk>: how're you doing on this one? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1200 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABABVDV4YXOBI5ZWH5F76RLTPFOWBANCNFSM44PL6MWQ> .

darrenvong

The TabBar thing is minor really and I was happy to leave it if there wasn't anything else... but since your merge here is introducing a bug by replacing the user invalidation on save (by accident I hope!), thought I'll point it out as you might as well fix both together

app/frontend/src/features/profile/hooks/useUpdateUserProfile.ts

app/frontend/src/components/TabBar/TabBar.tsx

darrenvong

Sorry I didn't review this as quickly, there's also a merge conflict with develop now but hopefully this should be it once fixed 🤞

app/frontend/src/features/constants.ts

More merge conflicts need to be fixed :(

lucaslcode

There shouldn't have been prettier issues - I just updated prettier. Try running yarn && yarn format

thdk · 2021-05-27T19:06:43Z

There shouldn't have been prettier issues - I just updated prettier. Try running yarn && yarn format

Auch, now I have even more changes.. I'll commit them so you can see if that's normal.

user id is not required anymore

thdk added 3 commits May 9, 2021 19:13

feat: create edit profile page with 'about' and 'home' tabs

7b989e1

test: fix edit hosting preference test

632a577

thdk requested a review from a team May 9, 2021 18:27

lucaslcode requested changes May 9, 2021

View reviewed changes

ItsiW previously requested changes May 10, 2021

View reviewed changes

thdk added 3 commits May 11, 2021 07:52

filter undefined values from tabbar

9c41e61

adjust user profile routes

984614c

style: remove wrapping react fragment

c923d09

thdk added 3 commits May 12, 2021 17:05

review feedback

401ed3a

Merge branch 'develop' into frontend/feature/edit-profile-1

4afec4a

add username to profile url on save

d762957

lucaslcode requested changes May 14, 2021

View reviewed changes

lucaslcode mentioned this pull request May 14, 2021

After editing "home" landing page is "about" instead of "my home" #1203

Closed

darrenvong reviewed May 15, 2021

View reviewed changes

app/frontend/src/routes.ts Outdated Show resolved Hide resolved

app/frontend/src/routes.ts Outdated Show resolved Hide resolved

darrenvong reviewed May 15, 2021

View reviewed changes

app/frontend/src/routes.ts Outdated Show resolved Hide resolved

darrenvong reviewed May 15, 2021

View reviewed changes

app/frontend/src/features/profile/view/ProfilePage.tsx Outdated Show resolved Hide resolved

update urls.py and urls.md

4e98ac3

root and others added 4 commits May 17, 2021 21:33

create seperate function routeToEditUser

68cd00a

The typescript overloads were a little overkill here indeed. Having two seperate functions greatly simplifies it.

throw error if user is undefined after save

2ceadd3

fixup! create seperate function routeToEditUser

c75a5cb

WIP: user route issues

0c005ec

Merge branch 'develop' into frontend/feature/edit-profile-1

49d316c

review feedback

610f637

thdk requested a review from darrenvong May 21, 2021 05:49

thdk mentioned this pull request May 21, 2021

Separate profile route from user routes #1272

Closed

darrenvong requested changes May 22, 2021

View reviewed changes

app/frontend/src/components/TabBar/TabBar.tsx Outdated Show resolved Hide resolved

app/frontend/src/components/TabBar/TabBar.tsx Outdated Show resolved Hide resolved

thdk added 3 commits May 24, 2021 19:20

tabbar review feedback

62d1346

Merge branch 'develop' into frontend/feature/edit-profile-1

de8193a

merge develop branch fixes

63e57f7

thdk requested a review from darrenvong May 24, 2021 17:56

darrenvong requested changes May 25, 2021

View reviewed changes

app/frontend/src/features/profile/hooks/useUpdateUserProfile.ts Show resolved Hide resolved

app/frontend/src/components/TabBar/TabBar.tsx Outdated Show resolved Hide resolved

fix query invalidation bug

3617422

thdk requested a review from darrenvong May 25, 2021 11:27

darrenvong requested changes May 26, 2021

View reviewed changes

app/frontend/src/features/constants.ts Outdated Show resolved Hide resolved

thdk added 3 commits May 27, 2021 08:00

remove unused constants

7576022

Merge branch 'develop' into frontend/feature/edit-profile-1

314d550

style: formatting

770ca91

thdk requested a review from darrenvong May 27, 2021 07:13

darrenvong previously approved these changes May 27, 2021

View reviewed changes

thdk added 2 commits May 27, 2021 20:42

Merge branch 'develop' into frontend/feature/edit-profile-1

1952668

style: fix prettier issues of develop branch

2900567

lucaslcode reviewed May 27, 2021

View reviewed changes

style: yarn && yarn format

603833d

lucaslcode approved these changes May 27, 2021

View reviewed changes

thdk merged commit 8080da7 into develop May 27, 2021

thdk deleted the frontend/feature/edit-profile-1 branch May 27, 2021 19:29

thdk mentioned this pull request May 28, 2021

Frontend/feature/profile route #1310

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Combine edit profile and edit home in one page with tabs #1200

Combine edit profile and edit home in one page with tabs #1200

thdk commented May 9, 2021 •

edited

lucaslcode left a comment

ItsiW left a comment •

edited

darrenvong commented May 10, 2021

thdk commented May 11, 2021

aapeliv commented May 14, 2021

lucaslcode commented May 14, 2021

lucaslcode left a comment

thdk commented May 15, 2021

thdk commented May 17, 2021

darrenvong commented May 17, 2021

aapeliv commented May 23, 2021

thdk commented May 23, 2021 via email

darrenvong left a comment

darrenvong left a comment

lucaslcode left a comment

thdk commented May 27, 2021

Combine edit profile and edit home in one page with tabs #1200

Combine edit profile and edit home in one page with tabs #1200

Conversation

thdk commented May 9, 2021 • edited

lucaslcode left a comment

Choose a reason for hiding this comment

ItsiW left a comment • edited

Choose a reason for hiding this comment

darrenvong commented May 10, 2021

thdk commented May 11, 2021

aapeliv commented May 14, 2021

lucaslcode commented May 14, 2021

lucaslcode left a comment

Choose a reason for hiding this comment

thdk commented May 15, 2021

thdk commented May 17, 2021

darrenvong commented May 17, 2021

aapeliv commented May 23, 2021

thdk commented May 23, 2021 via email

darrenvong left a comment

Choose a reason for hiding this comment

darrenvong left a comment

Choose a reason for hiding this comment

lucaslcode left a comment

Choose a reason for hiding this comment

thdk commented May 27, 2021

thdk commented May 9, 2021 •

edited

ItsiW left a comment •

edited