Paths

Table of Contentst

Differential D10317

[web] cleanup account delete modal
ClosedPublic
Actions

Authored by ginsu on Dec 12 2023, 12:49 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Fri, Dec 20, 10:14 PM

	Unknown Object (File)
	Fri, Dec 20, 9:41 PM

	Unknown Object (File)
	Fri, Dec 20, 8:47 PM

	Unknown Object (File)
	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

View All 64 Files

Subscribers

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

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)

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

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 ↗	(On Diff #34542)	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

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 ↗	(On Diff #34542)	Sorry should have called this out, but the very next diff does switch the casing for each style to be camel case

Let's try side-by-side

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

Path

Size

web/

settings/

account-delete-modal.css

23 lines

account-delete-modal.react.js

47 lines

Diff 37146

web/settings/account-delete-modal.css

Loading...

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

Loading...