[native] Move componentDidUpdate logic in MultimediaMessageMultimedia to functional component
ClosedPublic
Actions

Authored by angelika on Wed, Dec 11, 12:35 PM.

Details

Reviewers

ashoat
tomek

Commits

rCOMMe06dda106414: [native] Move componentDidUpdate logic in MultimediaMessageMultimedia to…

Summary

Move the logic in componentDidUpdate to a functional component, that is: if the scroll in overlay context is now disabled and it wasn't disabled before, then call setClickable(true). Or in more human way: if a modal that was previously blocking scroll etc. disappeared then the message with the multimedia is clickable again.

Depends on D14133

Test Plan

Open a modal (a tooltip or click a message with an image)
Close a modal
Click on a message with an image
Verify the image is expanded

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

angelika created this revision.Wed, Dec 11, 12:35 PM

angelika held this revision as a draft.

angelika edited the summary of this revision. (Show Details)Wed, Dec 11, 12:35 PM

angelika added a parent revision: D14133: [native] Migrate opacity in MultimediaMessageMultimedia to Reanimated v2 API in functional component.

angelika added a child revision: D14135: [native] Move render function in MultimediaMessageMultimedia to functional component.Wed, Dec 11, 12:38 PM

Harbormaster completed remote builds in B33035: Diff 46353.Wed, Dec 11, 1:14 PM

angelika published this revision for review.Wed, Dec 11, 5:30 PM

Accepting so I don't block you, but please make sure you've considered whether there's any risks associated with this effect potentially being run on mount

native/chat/multimedia-message-multimedia.react.js
205–212 ↗	(On Diff #46353)	Will this effect run on mount? I noticed in the old class component, there is no `componentDidMount`, so technically the behavior may be different if this effect can run on mount. Even if the effect runs on mount, there might be no problem... just wondering if you've thought about this

This revision is now accepted and ready to land.Wed, Dec 11, 8:10 PM

Rebase and address feedback

angelika added inline comments.Fri, Dec 13, 6:28 AM

native/chat/multimedia-message-multimedia.react.js
205–212 ↗	(On Diff #46353)	The effect will run on mount, but `scrollWasDisabled` will be `undefined` so `props.setClickable` won't be called. All it does on the first render is setting `scrollWasDisabled`.