Details
- Reviewers
bartek kamil inka ashoat - Commits
- rCOMM6fee9ffa8b7c: [web] Create manage links modal
Click manage links button and check if the modal is correct, both when a public link is present and when a community doesn't have a link.
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Accepting to unblock, but have a couple comments. The title stuff could potentially be handled in a follow-up if that's your preference, as I suspect it might require design changes
web/invite-links/manage-invite-links-modal.react.js | ||
---|---|---|
43 ↗ | (On Diff #28343) | It looks like we're using spaces for padding here |
64 ↗ | (On Diff #28343) | In ENG-4304, I mention in a couple places that I'd prefer to avoid putting user-generated names in screen / modal titles. Is there an alternate design we could take where we show the community name inside the content rather than the title? Separately, it appears that the equivalent on native (ManagePublicLinkScreen) uses a different title ("Public Link"). Let me know if I'm looking at the wrong component, but if it's right – should we aim for consistency? |
web/invite-links/manage-invite-links-modal.react.js | ||
---|---|---|
43 ↗ | (On Diff #28343) | That's correct. In the design https://www.figma.com/file/a1nkbWgbgjRlrOY9LVurTz/Comm-%2F-Desktop-app?node-id=11341%3A283370&mode=dev it looks like the intention is for this link to look like a part of a text block. To achieve that we should have a gap that is exactly as long as a width of a space char - and the easiest way to have that is by rendering the space. I can set e.g. a fixed margin instead, but it won't be guaranteed that its width will always be equal to a single space width. What risks can you see in the current approach? |
64 ↗ | (On Diff #28343) | In the future, this screen will also contain custom links https://www.figma.com/file/a1nkbWgbgjRlrOY9LVurTz/Comm-%2F-Desktop-app?node-id=11341%3A277530&mode=dev. Also, Public link is already a name of a section within this modal. We can consider using e.g. Manage invite links as a modal title, which makes a lot of sense because the button which opens the modal has this label. As for native, there's no 1:1 matching within these. The screen that uses Public Link name on native corresponds to a modal this is open after clicking Edit public link / Enable buttons here. |
web/invite-links/manage-invite-links-modal.react.js | ||
---|---|---|
43 ↗ | (On Diff #28343) | On native, we're displaying this button in a new line |
Update modal name
web/invite-links/manage-invite-links-modal.react.js | ||
---|---|---|
64 ↗ | (On Diff #28343) |
web/invite-links/manage-invite-links-modal.react.js | ||
---|---|---|
64 ↗ | (On Diff #28343) | Heads-up, this attachment is not showing up |