[web] lift primary button logic from AddMembersModalContent into AddMembersModal
ClosedPublic
Actions

Authored by ginsu on Feb 5 2024, 6:43 AM.

Details

Reviewers

atul
inka

Commits

rCOMM0c107f1fb030: [web] lift primary button logic from AddMembersModalContent into AddMembersModal

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

To follow the conventions of the new Modal api introduced earlier in this project, we need to lift the logic for the primary button to the same level where we render SearchModal so we can pass the button in as the primaryButton prop

Linear task: https://linear.app/comm/issue/ENG-5976/add-members-modal-experience

Depends on D10943

Test Plan

Please see demo video below (please note there are some weird UI stuff that still needs to be polished up but a subsequent diff will fix that)

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ginsu created this revision.Feb 5 2024, 6:43 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptFeb 5 2024, 6:43 AM

ginsu edited the summary of this revision. (Show Details)Feb 5 2024, 6:47 AM

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

ginsu added reviewers: atul, inka.

ginsu added a parent revision: D10943: [web] replace AddMembersListContent experience with AddUsersList.

ginsu attached a referenced file: F1116443: Screen Recording 2024-02-05 at 9.44.52 AM.mov. (Show Details)

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 5 2024, 6:57 AM

Harbormaster failed remote builds in B26496: Diff 36620!

fix eslint issue

Harbormaster completed remote builds in B26724: Diff 36950.Feb 9 2024, 11:21 AM

ginsu requested review of this revision.Feb 9 2024, 11:21 AM

atul accepted this revision.Feb 9 2024, 12:16 PM

atul added inline comments.

web/modals/threads/members/add-members-modal.react.js
35–36 ↗	(On Diff #36950)	Do we still need nested `div`s here now that there is nothing in `addMembersContent` div except for `addMembersListContainer`? Feel free to consolidate or leave as is depending on your preference.