[web] Restyle `ThreadSettingsDeleteTab`
ClosedPublic
Actions

Authored by atul on May 12 2022, 11:09 AM.

Details

Reviewers

• jacek
tomek
• benschac

Commits

rCOMM591e5403ef5d: [web] Restyle `ThreadSettingsDeleteTab`

Summary

There aren't any designs for this in Figma, so tried to get close to what we have in ThreadSettingsGeneralTab. There's definitely room to improve the design here, this was strictly a first pass.

Depends on D4018

Test Plan

Before (start of stack):

Screen Shot 2022-05-12 at 1.04.18 PM.png (792×716 px, 69 KB)

After: (after this diff):

Screen Shot 2022-05-12 at 2.07.54 PM.png (920×716 px, 154 KB)

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.May 12 2022, 11:09 AM

Herald added subscribers: • abosh, • adrian, ashoat. · View Herald TranscriptMay 12 2022, 11:09 AM

atul edited the test plan for this revision. (Show Details)May 12 2022, 11:11 AM

atul edited the summary of this revision. (Show Details)

atul added a parent revision: D4018: [web] Use `Input` component in `ThreadSettingsDeleteTab`.

Harbormaster completed remote builds in B9044: Diff 12587.May 12 2022, 11:15 AM

atul requested review of this revision.May 12 2022, 11:15 AM

• benschac accepted this revision.May 12 2022, 11:57 AM

• benschac added inline comments.

web/modals/threads/thread-settings-delete-tab.css
23 ↗	(On Diff #12587)	nit: can we use `var(--bold);`?
web/modals/threads/thread-settings-delete-tab.react.js
91 ↗	(On Diff #12587)	niiiiiiiice

This revision is now accepted and ready to land.May 12 2022, 11:57 AM

atul added 1 blocking reviewer(s): tomek.May 12 2022, 12:08 PM

This revision now requires review to proceed.May 12 2022, 12:08 PM

atul added inline comments.May 12 2022, 12:08 PM

web/modals/threads/thread-settings-delete-tab.css
23 ↗	(On Diff #12587)	true, will fix!

Looks ok! Regarding the designs, we have this: https://www.figma.com/file/a1nkbWgbgjRlrOY9LVurTz/Comm-%2F-Desktop-app?node-id=1060%3A61958. Do you think it is relevant? It looks like on the designs we don't require a password, and show a modal. I guess your design works better, as it verifies the identity.

This revision is now accepted and ready to land.May 13 2022, 1:51 AM

atul mentioned this in D4016: [web] Make `ThreadSettingsDeleteTab` "Delete" button full width.May 15 2022, 4:55 AM

Include changes from D4016 (literally just making the "Delete" button full-width) and use var(--bold) for font-weight as @benschac suggested.

Do you think it is relevant? It looks like on the designs we don't require a password, and show a modal.

I did take a look at this on Figma when restyling this "tab" of the modal.

Based on the filler copy (eg "If you delete a channel this and this will happen. This can happen too."), I figured that this screen wasn't "finalized" and didn't think it was intentional that the "confirm password" prompt was left out.

Can remove the "Confirm password" functionality if we decide to at a later point, but this diff was just about styling.

Harbormaster completed remote builds in B9114: Diff 12678.May 15 2022, 5:05 AM

rebase before landing

Harbormaster completed remote builds in B9115: Diff 12679.May 15 2022, 6:23 AM

Closed by commit rCOMM591e5403ef5d: [web] Restyle `ThreadSettingsDeleteTab` (authored by atul). · Explain WhyMay 15 2022, 6:49 AM

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMM591e5403ef5d: [web] Restyle `ThreadSettingsDeleteTab`.