Paths

Table of Contentst

Differential D7930

[web] Changed edit modal overlay opacity
ClosedPublic
Actions

Authored by • kuba on May 23 2023, 1:06 AM.

Tags

None

Referenced Files

	F3351905: D7930.diff
	Sat, Nov 23, 3:59 AM

	Unknown Object (File)
	Thu, Nov 7, 10:56 AM

	Unknown Object (File)
	Thu, Nov 7, 10:56 AM

	Unknown Object (File)
	Tue, Nov 5, 9:05 AM

	Unknown Object (File)
	Tue, Nov 5, 9:05 AM

	Unknown Object (File)
	Wed, Oct 30, 12:23 PM

	Unknown Object (File)
	Wed, Oct 30, 12:23 PM

	Unknown Object (File)
	Wed, Oct 30, 12:23 PM

Subscribers

Details

Reviewers

michal
inka

Commits

rCOMMb19e1f9e521e: [web] Changed edit modal overlay opacity

Summary

Suggested in: https://linear.app/comm/issue/ENG-3539/add-overlay-to-prevent-the-user-from-changing-thread-when-in-edit-mode

Test Plan

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

Diff Detail

Repository

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• kuba created this revision.May 23 2023, 1:06 AM

Herald added subscribers: • ted, atul, tomek, ashoat. · View Herald TranscriptMay 23 2023, 1:06 AM

• kuba added a parent revision: D7850: [web] Handle new messages when in edit mode.May 23 2023, 1:06 AM

Harbormaster completed remote builds in B19627: Diff 26835.May 23 2023, 1:24 AM

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

Rebase

Harbormaster completed remote builds in B19647: Diff 26855.May 23 2023, 3:29 AM

• kuba added a child revision: D7940: [lib] Make messageID in useEditMessage mandatory.May 23 2023, 4:27 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

lib/components/modal-overlay.react.js
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

Harbormaster completed remote builds in B19674: Diff 26882.May 23 2023, 7:00 AM

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

Harbormaster completed remote builds in B19681: Diff 26889.May 23 2023, 7:50 AM

inka accepted this revision.May 23 2023, 7:53 AM

This revision is now accepted and ready to land.May 23 2023, 7:53 AM

ashoat added inline comments.May 23 2023, 11:36 AM

lib/components/modal-overlay.react.js
59 ↗	(On Diff #26889)	Shorthand

Shorthand

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

Harbormaster completed remote builds in B19715: Diff 26941.May 24 2023, 2:51 AM

Closed by commit rCOMMb19e1f9e521e: [web] Changed edit modal overlay opacity (authored by kosmydel <kukubaczek@gmail.com>). · Explain WhyMay 24 2023, 4:01 AM

This revision was automatically updated to reflect the committed changes.

kosmydel <kukubaczek@gmail.com> added a commit: rCOMMb19e1f9e521e: [web] Changed edit modal overlay opacity.

Revision Contents
Changeset List

Path

Size

lib/

components/

modal-overlay.css

2 lines

modal-overlay.react.js

11 lines

web/

chat/

edit-message-provider.js

6 lines

2 lines

Diff 26882

lib/components/modal-overlay.css

Loading...

lib/components/modal-overlay.react.js

Loading...

web/chat/edit-message-provider.js

Loading...

web/theme.css

Loading...