This modal is displayed after the community has been created and allows users to optionally add members.
Details
Looks as expected:
Works as expected:
Diff Detail
- Repository
- rCOMM Comm
- Branch
- master
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
web/sidebar/community-creation/community-creation-members-modal.css | ||
---|---|---|
8–36 | This is copy/pasted, will pull out the keyserver label into separate component in subsequent diff | |
web/sidebar/community-creation/community-creation-members-modal.react.js | ||
26–32 | This is copy/pasted, will pull out the keyserver label into separate component in subsequent diff |
Generally looks good, but confused about some of the changes to existing modals...
Can you explain what the impact is there, ideally with some screenshots?
web/modals/threads/members/members-modal.css | ||
---|---|---|
2 | What impact does this change have on existing modals? I'm a little confused why it's in this diff | |
web/modals/threads/members/members-modal.react.js | ||
134 | What impact does this change have on existing modals? I'm a little confused why it's in this diff | |
web/sidebar/community-creation/community-creation-members-modal.css | ||
8–36 | Would've preferred to have seen that diff first since it would've avoided the copy-paste | |
web/sidebar/community-creation/community-creation-members-modal.react.js | ||
13 | For flexibility |
Basically there are three "modal sizes": small, large, and fit-content. Using small and large makes sense because we're setting the dimensions of the "container"/parent and having the elements arranged within via flexbox. fit-content is weird because it "flips things." The size of the container/parent (modal) is dependent on the contents.
On the surface that seems fine, but it means eg that because AddMembersModalContent has a hard-coded width, it isn't "flexible" and can't be used in modals of different size. For example if we try to consume AddMembersModalContent within CommunityCreationMembersModal, we get something like this:
By removing the hard-coded width, we get something like:
which is what we want.
The problem then is in MembersModal where AddMembersModalContent is also consumed. Because we had fit-content there, and we aren't hard-coding a width, the modal looks very skinny:
so I "rounded up" to large (instead of moving the hardcoded width somewhere else), and got:
If we really want to preserve the previous width we could possibly introduce a medium size modal, but I think the sizing here is reasonable. This approach ("named" modal sizes) is imo way easier to work with/adjust than hard-coding widths in CSS.
web/sidebar/community-creation/community-creation-members-modal.react.js | ||
---|---|---|
13 | There's some cascading flow stuff that this change introduces that comes down to how modal.react.js types the onClose prop: It might make sense to just have a followup diff that changes this broadly? |
web/sidebar/community-creation/community-creation-members-modal.css | ||
---|---|---|
8–36 | Agree in hindsight that this should've been handled in a previous diff |