[web] Add navigation path for account settings
ClosedPublic
Actions

Authored by tomek on Mar 4 2022, 7:47 AM.

Details

Reviewers

Commits

Summary

The idea here is to introduce a new path settings under which there will be located all the settings (things that are displayed when a user clicks a cog icon in https://www.figma.com/file/a1nkbWgbgjRlrOY9LVurTz/Comm-%2F-Desktop-app?node-id=1272%3A88750).

Test Plan

The behavior should be the same after this diff. Tested in combination with the next one.

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

lib/utils/url-utils.js
12 ↗	(On Diff #10077)	When a new settings path is introduced, this will become `'account' \| 'other-path' \| ...`
22 ↗	(On Diff #10077)	We could as well capture the subpath as a group, but being explicit here is safer and makes this more readable.

tomek requested review of this revision.Mar 4 2022, 7:52 AM

• benschac added inline comments.

lib/utils/url-utils.js
53 ↗	(On Diff #10077)	This looks correct. Kind of wild we didn't use the `react-router` functionality to route pages for us but are doing it manually.

This revision is now accepted and ready to land.Mar 7 2022, 6:28 AM

ashoat added inline comments.

lib/utils/url-utils.js
6–12 ↗	(On Diff #10077)	What happens if we set this as `$ReadOnly` by prefixing individual properties with `+`? For any properties that lead to errors when you add `+`, it's probably not worth it. But if there are any properties where you can add `+` without errors, we should probably do it

lib/utils/url-utils.js
6–12 ↗	(On Diff #10077)	It works ok for all the props - going to update the diff with this change
53 ↗	(On Diff #10077)	Agree. Created a task for it https://linear.app/comm/issue/ENG-853/use-routing-library