Details

Reviewers

atul
ginsu
ashoat

Commits

rCOMM69e6bfdd8d3b: [web] Relocate the icon in ThreadSettingsDeleteTab to be in-line with the…

Summary

Moved the i icon to be in-line with the warning text when
attempting to delete a chat. This was done by moving the SWMansionIcon
element to be inside the paragraph element.

Addresses: https://linear.app/comm/issue/ENG-1921/relocate-the-icon-in-threadsettingsdeletetab-to-be-in-line-with-the

Test Plan

Confirmed visually that this change works as expected, and will
attach before/after photos so reviewers can see.

Before:

After:

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rohan created this revision.Sep 30 2022, 1:03 PM

Herald added subscribers: • abosh, tomek, ashoat. · View Herald TranscriptSep 30 2022, 1:03 PM

rohan retitled this revision from [web] Relocate the icon in ThreadSettingsDeleteTab to be in-line with the warning text to [web] Relocate the icon in ThreadSettingsDeleteTab to be in-line with thewarning text.Sep 30 2022, 1:04 PM

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

rohan edited the test plan for this revision. (Show Details)

rohan attached a referenced file: F187505: After.png. (Show Details)

rohan attached a referenced file: F187504: Before.png. (Show Details)

Harbormaster completed remote builds in B12567: Diff 17253.Sep 30 2022, 1:13 PM

rohan requested review of this revision.Sep 30 2022, 1:13 PM

Let's get it aligned better, we don't want the text to wrap with the icon

This revision now requires changes to proceed.Sep 30 2022, 1:51 PM

Align the icon and text side by side without putting the icon inside the
paragraph content. If it's better to push the content and icon a little to the left still, that can be done

rohan attached a referenced file: F187650: Revision.png. (Show Details)Sep 30 2022, 4:34 PM

Harbormaster completed remote builds in B12570: Diff 17258.Sep 30 2022, 4:44 PM

Center icon, increasing padding

This revision now requires changes to proceed.Oct 1 2022, 3:55 AM

Spoke with Ashoat offline and improved the centering & padding

Screen Shot 2022-10-01 at 3.38.11 PM.png (1×732 px, 675 KB)

Screen Shot 2022-10-01 at 3.35.52 PM.png (1×778 px, 253 KB)

rohan attached a referenced file: F188560: Screen Shot 2022-10-01 at 3.35.52 PM.png. (Show Details)Oct 2 2022, 6:22 AM

rohan attached a referenced file: F188561: Screen Shot 2022-10-01 at 3.38.11 PM.png. (Show Details)

rohan retitled this revision from [web] Relocate the icon in ThreadSettingsDeleteTab to be in-line with thewarning text to [web] Relocate the icon in ThreadSettingsDeleteTab to be in-line with the warning text.

Harbormaster completed remote builds in B12574: Diff 17276.Oct 2 2022, 6:31 AM

Looks great!! Will also love @atul's take

I would personally align the i icon back a bit so that it is flush with the text below

In D5273#155747, @ginsu wrote:

I would personally align the i icon back a bit so that it is flush with the text below

Agree with this, think they should be left-aligned

atul requested changes to this revision.Oct 3 2022, 12:44 PM

This revision now requires changes to proceed.Oct 3 2022, 12:44 PM

In D5273#155747, @ginsu wrote:

I would personally align the i icon back a bit so that it is flush with the text below

Thanks for the suggestion @ginsu! Do you think just the icon should be shifted to the left with the text below, or the entire container (ie the icon and the warning text) should be shifted left?

Tagging @atul as well since you have the same suggestion

I would shift the entire container (icon with the warning text) to the left

ginsu requested changes to this revision.Oct 3 2022, 12:55 PM