Page MenuHomePhabricator
Differential D10317

[web] cleanup account delete modal
ClosedPublic
Actions

Authored by ginsu on Dec 12 2023, 12:49 PM.
Tags
None
Referenced Files
F3507094: D10317.diff
Fri, Dec 20, 7:19 PM
Unknown Object (File)
Thu, Dec 19, 7:13 PM
Unknown Object (File)
Sun, Dec 15, 4:33 PM
Unknown Object (File)
Nov 8 2024, 8:31 AM
Unknown Object (File)
Nov 8 2024, 8:20 AM
Unknown Object (File)
Nov 8 2024, 7:14 AM
Unknown Object (File)
Nov 8 2024, 6:29 AM
Unknown Object (File)
Oct 31 2024, 9:17 AM
View All 61 Files
Subscribers
ashoat
tomek

Details

Reviewers
atul
rohan
kamil
Commits
rCOMM41bff2ee310d: [web] cleanup account delete modal
Summary

This diff cleans up the one off styles in the account delete modal and replaces it with styles of the redesigned modal.

Linear task: https://linear.app/comm/issue/ENG-5943/extendmodify-the-modal-props-api-to-follow-new-modal-designs

Depends on D10316

Test Plan

Please see the screenshots below

Screenshot 2023-12-12 at 3.50.49 PM.png (1×3 px, 729 KB)

error case:

Screenshot 2023-12-12 at 3.51.50 PM.png (1×3 px, 740 KB)

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu created this revision.Dec 12 2023, 12:49 PM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptDec 12 2023, 12:49 PM
ginsu edited the summary of this revision. (Show Details)Dec 12 2023, 12:52 PM
ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: atul, rohan, kamil.
ginsu added a parent revision: D10316: [web] cleanup message search modal.
ginsu attached a referenced file: F931007: Screenshot 2023-12-12 at 3.51.50 PM.png. (Show Details)
ginsu attached a referenced file: F931006: Screenshot 2023-12-12 at 3.50.49 PM.png. (Show Details)
ginsu updated this revision to Diff 34542.Dec 12 2023, 12:55 PM

update

ginsu added a child revision: D10318: [web] update account delete modal to use camelcase.Dec 12 2023, 12:59 PM
Harbormaster completed remote builds in B25000: Diff 34541.Dec 12 2023, 1:10 PM
Harbormaster completed remote builds in B25001: Diff 34542.Dec 12 2023, 1:19 PM
ginsu requested review of this revision.Dec 12 2023, 1:19 PM
rohan accepted this revision.Dec 12 2023, 1:26 PM

Looks good - just one comment inline.

Also just a personal opinion on the design of the modal (I know you didn't introduce the change in this diff so no changes are needed here), but I think the little ! circle icon would look better inline with the text, either in its own column side-by-side to the text, or just next to the first line of text. Not a big deal though, just wanted to call it out

web/settings/account-delete-modal.css
22

Could be good to be consistent with the file (maybe either make this error_placeholder or make everything else camel case)

This revision is now accepted and ready to land.Dec 12 2023, 1:26 PM
ginsu added a comment.Dec 12 2023, 1:35 PM

Also just a personal opinion on the design of the modal (I know you didn't introduce the change in this diff so no changes are needed here), but I think the little ! circle icon would look better inline with the text, either in its own column side-by-side to the text, or just next to the first line of text. Not a big deal though, just wanted to call it out

I think side by side would look better too, I can make that change if we are all aligned on it.

cc @ashoat @atul

web/settings/account-delete-modal.css
22

Sorry should have called this out, but the very next diff does switch the casing for each style to be camel case

ashoat added a comment.Dec 13 2023, 5:47 PM

Let's try side-by-side

ginsu updated this revision to Diff 37146.Feb 15 2024, 12:33 AM

update icon and text to be side by side + rebase before landing

This revision was landed with ongoing or failed builds.Feb 15 2024, 12:44 AM
Closed by commit rCOMM41bff2ee310d: [web] cleanup account delete modal (authored by ginsu). · Explain Why
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMM41bff2ee310d: [web] cleanup account delete modal.
Harbormaster completed remote builds in B26870: Diff 37146.Feb 15 2024, 12:51 AM

Revision Contents
Changeset List

PathSize
web/
settings/
18 lines
48 lines

Diff 34542

View Options

web/settings/account-delete-modal.css

Loading...
View Options

web/settings/account-delete-modal.react.js

Loading...