Details

Reviewers

tomek
atul
• benschac

Commits

rCOMM1a646434baa1: [web] Redesign `Modal` to match Figma

Summary

Redesigned modal to match existing designs. Refactored and simplified CSS classes and removed redundant container in component.
Removed fixedHeight property, as it was used in one place and had no effect on existing implementation.
Existing modal sizes (small, large) should work as before. Introduced fit-content size option, to allow adjust modal size by its content.

Screenshot_Google Chrome_2022-04-08_150355.png (895×1 px, 114 KB)

Test Plan

Opened currently existing modals. It's size should remain unchanges, but fonts and design should now match Figma.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• jacek created this revision.Apr 8 2022, 5:54 AM

Herald added subscribers: • abosh, • benschac, • karol and 4 others. · View Herald TranscriptApr 8 2022, 5:54 AM

Harbormaster completed remote builds in B8021: Diff 11226.Apr 8 2022, 5:59 AM

• jacek requested review of this revision.Apr 8 2022, 5:59 AM

• jacek added a child revision: D3671: [web] Adjust `MembersModal` size to match designs.Apr 8 2022, 6:00 AM

• jacek added parent revisions: D3667: [web] Convert `Modal` into functional component, D3668: [web] Remove `Modal`s usused style.

• jacek edited the summary of this revision. (Show Details)Apr 8 2022, 6:04 AM

• jacek added reviewers: tomek, atul, • benschac.Apr 8 2022, 6:07 AM

The modal in the provided screenshot looks way better!

web/modals/modal.css
8	I think we could add colors like this (w/ alpha channel) to the `theme.css` file... but I think it's fine either way
25	Just a thought (and outside the scope of this diff), but I wonder if naming these `modalContainerNarrow` and `modalContainerWide` would make it clear that the difference is just along the X-axis?
27	Is specifying `min-height: 0` necessary? I'm assuming you found that you needed to specify this to get things to look right, just making sure it wasn't included extraneously?