Page MenuHomePhabricator

[web] Basic `CommunityCreationModal` layout/styling
ClosedPublic

Authored by atul on May 9 2023, 7:36 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 27, 11:53 PM
Unknown Object (File)
Mar 3 2024, 8:45 AM
Unknown Object (File)
Mar 2 2024, 7:57 PM
Unknown Object (File)
Mar 2 2024, 7:57 PM
Unknown Object (File)
Mar 2 2024, 7:56 PM
Unknown Object (File)
Feb 11 2024, 3:07 PM
Unknown Object (File)
Feb 10 2024, 4:59 PM
Unknown Object (File)
Feb 10 2024, 4:58 PM
Subscribers

Details

Summary

Introduce some of the layout/styling for the CommunityCreationModal.

This diff is mostly about placing the necessary elements, subsequent diffs will handle enabling functionality.

Test Plan

Looks as expected (w/ avatar placeholder):

Screen Shot 2023-05-09 at 4.22.34 PM.png (1×712 px, 133 KB)

Looks as expected (w/o avatar placeholder):

Screen Shot 2023-05-09 at 4.25.13 PM.png (1×712 px, 76 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul added inline comments.
web/sidebar/community-creation/community-creation-modal.react.js
52 ↗(On Diff #26310)

This is a placeholder just for layout purposes. Once we have avatar editing on desktop we can swap this out for some sort of EditAvatar component.

65 ↗(On Diff #26310)

This is also placeholder. We'll need to display some sort of dropdown that lets the user select keyservers for which they're an admin of?

atul requested review of this revision.May 9 2023, 7:55 AM
web/sidebar/community-creation/community-creation-modal.react.js
65 ↗(On Diff #26310)

Right now we can either exclude this part of the modal, or we can try to approximate it.

We don't really have the concept of "keyservers" in the Redux ThreadStore, but we can use useKeyserverAdmin(...) and the community root admins as an approximation?

We could possibly.

  1. Determine all the community roots
  2. Determine the first admin for all of those community roots
  3. Display the admin as the keyserver if the user is the admin?

IMO that's probably a stretch and the logic will be irrelevant once we have multi-keyserver support. I think we should just drop the ancestry section of this modal until we have multi-keyserver support.

Open to hearing other thoughts/opinions.

web/sidebar/community-creation/community-creation-modal.react.js
65 ↗(On Diff #26310)

I feel like the best thing would be to hardcode "ashoat" as the keyserver name since that is the only keyserver we have right now. When we do have multi-keyserver support we can then come back and update this accordingly

ashoat requested changes to this revision.May 9 2023, 10:16 AM
ashoat added inline comments.
web/sidebar/community-creation/community-creation-modal.react.js
18–19 ↗(On Diff #26310)

This is definitely better than that the copy we have in the "Create Subchannel" modal on web today.

That said, I'd like to revise this for three reasons:

  1. Would be good to match the language we have in the "Create Subchannel" modal, which doesn't prefix with "Make it so"
  2. Would be good to try to use language we can also use in the "Create Subchannel" modal. Granted in this case it's applied to the community root versus an arbitrary channel, so there will probably need to be some differences
  3. As part of the Roles project, it will be possible for the admin to assign privileges to post in announcement channels to other roles

How about this?

Subchannel creation modal

Only admins and other admin-appointed roles can send messages in an announcement channel.

Community creation modal

This option sets the community's root channel to an announcement channel. Only admins and other admin-appointed roles can send messages in an announcement channel.

65 ↗(On Diff #26310)

I think we should just drop the ancestry section of this modal until we have multi-keyserver support.

This could work

I feel like the best thing would be to hardcode "ashoat" as the keyserver name since that is the only keyserver we have right now. When we do have multi-keyserver support we can then come back and update this accordingly

I'd probably prefer this

It's a tradeoff between confusing users who might think it's weird to see my username there, versus misleading users who might be surprised to learn that their community is hosted on my keyserver. I think I lean towards Ginsu's suggestion, but I can see arguments for both sides

68 ↗(On Diff #26310)

What does form do here? Searched the codebase and I can see we use it in some other places in web. I know we're not actually using the classic HTTP form functionality (sending on submit), and I vaguely recall <form> elements having some other function (outside of sending on submit), but I don't recall what that functionality is

This revision now requires changes to proceed.May 9 2023, 10:16 AM
web/sidebar/community-creation/community-creation-modal.react.js
18–19 ↗(On Diff #26310)

Oh yeah I definitely agree. I just cut/pasted the copy from the Figma designs.

I'll update with the changes.

65 ↗(On Diff #26310)

Makes sense, will hardcode "@ashoat" for now

68 ↗(On Diff #26310)

I mostly went with it because that's what we're doing in ThreadSettingsModal.

My loose memory was that we preferred form instead of div because it was "more semantic" and there might be some benefit with accessibility?

My instinct is that we could just swap out form for div, but I'm not fully sure... I'll do a bit more research: https://linear.app/comm/issue/ENG-3873/research-form-vs-div-on-web

partially address feedback (copy)

atul planned changes to this revision.May 10 2023, 3:58 AM

placeholder keyserver name

Updated to display "@ ashoat" as default keyserver. Also made the modal "large" so the copy would fit a bit better:

Screen Shot 2023-05-10 at 3.44.29 PM.png (565×526 px, 39 KB)

Here's how it looked "small":

Screen Shot 2023-05-10 at 3.48.49 PM.png (646×376 px, 36 KB)

ashoat added inline comments.
web/sidebar/community-creation/community-creation-modal.react.js
18 ↗(On Diff #26346)

Can we use &apos;?

This revision is now accepted and ready to land.May 11 2023, 8:42 AM

rebase before addressing feedback

This revision was landed with ongoing or failed builds.May 15 2023, 9:06 AM
This revision was automatically updated to reflect the committed changes.