Page MenuHomePhabricator

[web] Add navigation path for account settings
ClosedPublic

Authored by tomek on Mar 4 2022, 7:47 AM.
Tags
None
Referenced Files
F3488627: D3335.diff
Wed, Dec 18, 10:53 AM
Unknown Object (File)
Fri, Dec 6, 4:22 AM
Unknown Object (File)
Nov 12 2024, 12:44 PM
Unknown Object (File)
Nov 12 2024, 9:28 AM
Unknown Object (File)
Nov 12 2024, 9:22 AM
Unknown Object (File)
Nov 8 2024, 8:07 PM
Unknown Object (File)
Nov 3 2024, 1:21 PM
Unknown Object (File)
Oct 27 2024, 8:44 AM

Details

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.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

lib/utils/url-utils.js
12

When a new settings path is introduced, this will become 'account' | 'other-path' | ...

22

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

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

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

It works ok for all the props - going to update the diff with this change

53