Details

Reviewers

atul
inka

Commits

rCOMMd05bba67d105: [web] introduce previouslySelectedUsers ui to add users list

Summary

PLEASE NOTE THAT THIS DIFF AND SUBSEQUENT DIFFS IN THIS STACK WILL NOT BE LANDED UNTIL MORE OF THE REDESIGN IS READY SINCE THIS WILL CAUSE REGRESSIONS IN PROD

Now that we have the previously selected users stored in the previouslySelectedUsers map, we should display them in the add users list.

Linear task: https://linear.app/comm/issue/ENG-5960/selected-users-section

Depends on D10820

Test Plan

Please see the demo video below

The user list is now in alphabetical order with ENS names:

Screenshot 2024-01-31 at 2.30.48 PM.png (2×3 px, 777 KB)

Screenshot 2024-01-31 at 2.30.58 PM.png (2×3 px, 781 KB)

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ginsu created this revision.Jan 25 2024, 12:24 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJan 25 2024, 12:24 PM

ginsu edited the summary of this revision. (Show Details)Jan 25 2024, 12:34 PM

ginsu edited the test plan for this revision. (Show Details)

ginsu added reviewers: atul, inka.

ginsu added a parent revision: D10820: [web] introduce previouslySelectedUsers map to add users list.

ginsu edited the summary of this revision. (Show Details)Jan 25 2024, 12:38 PM

Harbormaster completed remote builds in B26165: Diff 36142.Jan 25 2024, 12:44 PM

ginsu requested review of this revision.Jan 25 2024, 12:44 PM

ginsu added a child revision: D10823: [web] update toggleUser to use previouslySelectedUsers map if previously selected user is toggled.Jan 25 2024, 1:30 PM

atul mentioned this in D10820: [web] introduce previouslySelectedUsers map to add users list.Jan 29 2024, 7:34 PM

Question inline about order of alphabetizing usernames and resolving ENS names

web/settings/relationship/add-users-list.react.js
151 ↗	(On Diff #36142)	Don't we want to sort AFTER we've resolved ENS names? Couldn't the sort order change? (eg if someone is aaaaaaaaaaa.eth they should be first in order)
230–237 ↗	(On Diff #36142)	Wouldn't hurt to memoize this

This revision now requires changes to proceed.Jan 29 2024, 7:40 PM

ginsu mentioned this in D10897: [web] introduce useSortedENSResolvedUsers.Jan 31 2024, 11:28 AM

address comments

ginsu edited the test plan for this revision. (Show Details)Jan 31 2024, 12:30 PM

ginsu attached a referenced file: F1103012: Screenshot 2024-01-31 at 2.30.58 PM.png. (Show Details)

ginsu attached a referenced file: F1103013: Screenshot 2024-01-31 at 2.30.48 PM.png. (Show Details)

ginsu mentioned this in rCOMM8edec5cb6dad: [web] introduce useSortedENSResolvedUsers.Jan 31 2024, 12:36 PM

Harbormaster completed remote builds in B26372: Diff 36461.Jan 31 2024, 12:45 PM

Thanks for addressing feedback, could we use if statement instead of invariant where I annotated?

web/settings/relationship/add-users-list.react.js
123–125 ↗	(On Diff #36461)	Could we use `if` statement instead of `invariant`?

This revision is now accepted and ready to land.Jan 31 2024, 12:53 PM

rebase + address comments

Thanks for addressing feedback, could we use if statement instead of invariant where I annotated?

will handle this in D10823

Harbormaster completed remote builds in B26393: Diff 36494.Jan 31 2024, 3:13 PM

update vip users with previouslySelectedUsers

fix typo

ginsu retitled this revision from [web] introduce vip users ui to add users list to [web] introduce previouslySelectedUsers ui to add users list.Jan 31 2024, 3:34 PM

ginsu edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B26394: Diff 36495.Jan 31 2024, 3:43 PM

Harbormaster completed remote builds in B26395: Diff 36496.Jan 31 2024, 3:51 PM

rebase before landing

This revision was landed with ongoing or failed builds.Feb 15 2024, 1:19 AM

Closed by commit rCOMMd05bba67d105: [web] introduce previouslySelectedUsers ui to add users list (authored by ginsu). · Explain Why

This revision was automatically updated to reflect the committed changes.

ginsu added a commit: rCOMMd05bba67d105: [web] introduce previouslySelectedUsers ui to add users list.

Harbormaster completed remote builds in B26889: Diff 37180.Feb 15 2024, 1:42 AM