[web] introduce light mode to confirm leave thread modal
ClosedPublic
Actions

Authored by ginsu on Jan 8 2024, 1:16 AM.

Details

Reviewers

atul
inka

Commits

rCOMMd3f8d846c067: [web] introduce light mode to confirm leave thread modal

Summary

This diff introduces the new color design system/light mode to the confirm leave thread modal.

For additional context about this new color design system and the new naming convention here is the notion doc:
https://www.notion.so/commapp/New-Color-System-Proposal-ef80b4e4b9ec42949095056161223a42

Linear task: https://linear.app/comm/issue/ENG-5037/confirm-leave-thread-modal

Test Plan

Please see screenshots below

dark:

Screenshot 2024-01-08 at 4.18.39 AM.png (2×3 px, 879 KB)

light:

Screenshot 2024-01-08 at 4.19.06 AM.png (2×3 px, 873 KB)

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Jan 8 2024, 1:16 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJan 8 2024, 1:16 AM

will make sure ci passes before landing

ginsu added a child revision: D10571: [web] introduce light mode to sidebar promote modal.Jan 8 2024, 1:24 AM

Harbormaster failed remote builds in B25579: Diff 35346!Jan 8 2024, 1:28 AM

Several things look weird about this modal:

The "Are you sure" line isn't aligned, neither with the icon nor with the header text
The spacing between the "Are you sure" line and the one above seems too light given the spacing elsewhere
It doesn't have a separation between the button area and the text area. I thought Ted's most recent designs for modals introduced this, but I'm not sure if it was meant to apply universally or not

Not sure this diff is meant to address any of this. Feel free to land without addressing, but in that case please link a relevant Linear issue that explicitly mentions all of the above before landing

Changes described in summary look good.

This revision is now accepted and ready to land.Jan 8 2024, 12:35 PM

In D10570#305644, @ashoat wrote:

Several things look weird about this modal:

The "Are you sure" line isn't aligned, neither with the icon nor with the header text

The spacing between the "Are you sure" line and the one above seems too light given the spacing elsewhere

It doesn't have a separation between the button area and the text area. I thought Ted's most recent designs for modals introduced this, but I'm not sure if it was meant to apply universally or not

Not sure this diff is meant to address any of this. Feel free to land without addressing, but in that case please link a relevant Linear issue that explicitly mentions all of the above before landing

When I rebased this diff to get the updates of the new redesigned modals, it seemed like all your pieces of feedback were addressed. To make sure this is true tho, I created this linear task to continue the conversation:
https://linear.app/comm/issue/ENG-6885/clean-up-confirm-leave-thread-modal