Page MenuHomePhabricator
Differential D8137

[web] Create a modal that allows admins to change a member's role (modal, description, avatar)
ClosedPublic
Actions

Authored by rohan on Jun 7 2023, 5:03 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 5, 9:42 PM
Unknown Object (File)
Fri, Apr 5, 8:43 AM
Unknown Object (File)
Fri, Apr 5, 8:43 AM
Unknown Object (File)
Fri, Apr 5, 8:43 AM
Unknown Object (File)
Fri, Apr 5, 8:43 AM
Unknown Object (File)
Fri, Apr 5, 8:43 AM
Unknown Object (File)
Fri, Apr 5, 8:43 AM
Unknown Object (File)
Fri, Apr 5, 8:43 AM
View All 42 Files
Subscribers
ashoat
• ted
tomek

Details

Reviewers
atul
ginsu
Commits
rCOMMee54409da806: [web] Create a modal that allows admins to change a member's role (modal…
Summary

This is 1/2 of the modal that allows admins to modify member roles. This will just cover the actual modal, the description, and displaying the user avatar and info.

https://linear.app/comm/issue/ENG-4038/create-a-modal-that-allows-admins-to-change-a-members-role

Depends on D8136

Test Plan

Please see below a screenshot of what the modal looks like for now

Screenshot 2023-06-07 at 8.20.01 AM.png (448×1 px, 54 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rohan created this revision.Jun 7 2023, 5:03 AM
Herald added subscribers: • ted, tomek, ashoat. · View Herald TranscriptJun 7 2023, 5:03 AM
rohan added a child revision: D8138: [web] Create a modal that allows admins to change a member's role (dropdown menu, actionable buttons).Jun 7 2023, 5:04 AM
Harbormaster completed remote builds in B20079: Diff 27514.Jun 7 2023, 5:19 AM
rohan requested review of this revision.Jun 7 2023, 5:19 AM
rohan edited the test plan for this revision. (Show Details)Jun 7 2023, 5:20 AM
rohan attached a referenced file: F575228: Screenshot 2023-06-07 at 8.20.01 AM.png. (Show Details)
ginsu requested changes to this revision.Jun 7 2023, 3:20 PM

Overall looks good, let's just address the onClose behavior

web/modals/threads/members/change-member-role-modal.react.js
21 ↗(On Diff #27514)

Nit I would just write modalName inline:

name="Change Role"

21 ↗(On Diff #27514)

Assuming this gets handled in a subsequent diff, but to make sure this diff can land/stand on its own we should call popModal in here now

This revision now requires changes to proceed.Jun 7 2023, 3:20 PM
rohan updated this revision to Diff 27544.Jun 8 2023, 6:09 AM

Address feedback

Harbormaster completed remote builds in B20101: Diff 27544.Jun 8 2023, 6:26 AM
ginsu accepted this revision.Jun 8 2023, 9:22 AM
This revision is now accepted and ready to land.Jun 8 2023, 9:22 AM
rohan updated this revision to Diff 27579.Jun 9 2023, 11:13 AM

Rebase on D8136

Harbormaster completed remote builds in B20129: Diff 27579.Jun 9 2023, 11:31 AM
rohan added a comment.EditedJun 13 2023, 4:47 AM

cc @atul @ted

Based on our discussion for using size="profile" in the screen on native, do we also want to do that on web? I personally think the profile-sized avatar looks too big on web, but open to a second option

Screenshot 2023-06-13 at 7.46.50 AM.png (998×992 px, 97 KB)
Screenshot 2023-06-13 at 7.47.23 AM.png (856×1 px, 78 KB)

rohan attached a referenced file: F585235: Screenshot 2023-06-13 at 7.46.50 AM.png. (Show Details)Jun 13 2023, 4:47 AM
rohan attached a referenced file: F585238: Screenshot 2023-06-13 at 7.47.23 AM.png. (Show Details)
• ted added a comment.Jun 13 2023, 8:17 AM
In D8137#242017, @rohan wrote:

cc @atul @ted

Based on our discussion for using size="profile" in the screen on native, do we also want to do that on web? I personally think the profile-sized avatar looks too big on web, but open to a second option

Screenshot 2023-06-13 at 7.46.50 AM.png (998×992 px, 97 KB)
Screenshot 2023-06-13 at 7.47.23 AM.png (856×1 px, 78 KB)

Hey @rohan, thanks for the screenshots! It does look a bit big on the web modal. In the designs, I do have a size that fits nicer. Is it possible to match the design size?

Screenshot 2023-06-13 at 10.56.08 AM.png (1×1 px, 211 KB)

rohan added a comment.Jun 13 2023, 8:21 AM
In D8137#242030, @ted wrote:

Hey @rohan, thanks for the screenshots! It does look a bit big on the web modal. In the designs, I do have a size that fits nicer. Is it possible to match the design size?

We could probably match it, though it'd mean introducing a new 'size' to the avatars, @atul and @ginsu I believe worked on that so they can probably help make a call here

• ted added a comment.Jun 13 2023, 8:51 AM
In D8137#242031, @rohan wrote:
In D8137#242030, @ted wrote:

Hey @rohan, thanks for the screenshots! It does look a bit big on the web modal. In the designs, I do have a size that fits nicer. Is it possible to match the design size?

We could probably match it, though it'd mean introducing a new 'size' to the avatars, @atul and @ginsu I believe worked on that so they can probably help make a call here

Actually, I think the 'large' size can work fine for this

rohan updated this revision to Diff 27703.Jun 13 2023, 1:13 PM

Rebase

Harbormaster completed remote builds in B20225: Diff 27703.Jun 13 2023, 1:32 PM
rohan updated this revision to Diff 27938.Jun 21 2023, 8:04 AM

Rebase

Harbormaster completed remote builds in B20384: Diff 27938.Jun 21 2023, 8:23 AM
Closed by commit rCOMMee54409da806: [web] Create a modal that allows admins to change a member's role (modal… (authored by rohan). · Explain WhyJun 21 2023, 8:41 AM
This revision was automatically updated to reflect the committed changes.
rohan added a commit: rCOMMee54409da806: [web] Create a modal that allows admins to change a member's role (modal….

Revision Contents
Changeset List

PathSize
web/
modals/
threads/
members/
29 lines
37 lines
9 lines
2 lines

Diff 27957

View Options

web/modals/threads/members/change-member-role-modal.css

Loading...
View Options

web/modals/threads/members/change-member-role-modal.react.js

Loading...
View Options

web/modals/threads/members/member.react.js

Loading...
View Options

web/theme.css

Loading...