Page MenuHomePhabricator

[web] Changed edit modal overlay opacity

Authored by kuba on Tue, May 23, 1:06 AM.
Referenced Files
F570695: D7930.id26958.diff
Sat, Jun 3, 3:39 PM
F570488: D7930.id26835.diff
Sat, Jun 3, 6:50 AM
F570212: D7930.id26882.diff
Fri, Jun 2, 8:32 PM
Unknown Object (File)
Fri, Jun 2, 2:34 PM
Unknown Object (File)
Fri, Jun 2, 12:57 PM
Unknown Object (File)
Fri, Jun 2, 9:21 AM
Unknown Object (File)
Thu, Jun 1, 8:39 PM
Unknown Object (File)
Wed, May 31, 4:58 PM


Test Plan

Checked if new opacity is applied in edit mode. Checked if it is not applied in other modals.

Diff Detail

rCOMM Comm
Lint Not Applicable
Tests Not Applicable

Event Timeline

kuba requested review of this revision.Tue, May 23, 1:24 AM

Looks like @ted suggested using two different opacities for modals here. My first instinct would be to just have a single opacity, but he's on vacation so we can just proceed with this approach for now

59 ↗(On Diff #26855)

This should be memoized. By passing in this new object every time, you've actually made all of the memoization above completely useless (memoization of onBackgroundMouseDown, onBackgroundMouseUp, and onKeyDown is now useless)

Memoized backgroundColor

Can you remove the color from modal-overlay.css and set the default in ModalOverlay? I feel this will be more readable.

Moved default background color

This revision is now accepted and ready to land.Tue, May 23, 7:53 AM
59 ↗(On Diff #26889)


kuba marked an inline comment as done.Wed, May 24, 2:33 AM