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

Branch

kuba/message-editing-web

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	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

4 lines

web/

chat/

edit-message-provider.js

6 lines

2 lines

Diff 26855

lib/components/modal-overlay.css

Loading...

lib/components/modal-overlay.react.js

Loading...

web/chat/edit-message-provider.js

Loading...

web/theme.css

Loading...