[native] Fix display of TooltipInlineEngagement
ClosedPublic
Actions

Authored by ashoat on Wed, Oct 9, 8:25 PM.

Details

Reviewers

tomek

Commits

rCOMMc29733db3f07: [native] Fix display of TooltipInlineEngagement

Summary

This diff does two things:

We need to pass positioning from TooltipInlineEngagement to InlineEngagement in order to make sure that the reactions and sidebar button appear in the same order. The point of TooltipInlineEngagement is to appear directly over InlineEngagement to make the animation look good; if the order of buttons is different, the animation is ruined.
After passing positioning to InlineEngagement, I was able to clean up some of the styles used in TooltipInlineEngagement.

Depends on D13676

Test Plan

On my local iOS simulator, I tested 6 cases: multimedia, text, and robotext, each sent from the viewer and from somebody else. I made sure each test case had a sidebar and a reaction. In all cases, I made sure that the inline engagement appears exactly in the place it should.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ashoat created this revision.Wed, Oct 9, 8:25 PM

ashoat added a child revision: D13678: [lib][native][web] Introduce chatMessageItemHasEngagement.

Harbormaster completed remote builds in B32161: Diff 45036.Wed, Oct 9, 9:09 PM

ashoat requested review of this revision.Wed, Oct 9, 9:09 PM

tomek accepted this revision.Fri, Oct 11, 5:56 AM

tomek added inline comments.

native/chat/inline-engagement.react.js
494 ↗	(On Diff #45036)	Why this can be removed?

This revision is now accepted and ready to land.Fri, Oct 11, 5:56 AM

ashoat added inline comments.Sun, Oct 13, 6:01 PM

native/chat/inline-engagement.react.js
494 ↗	(On Diff #45036)	To be honest, I'm not sure. I initially thought that passing `positioning` to `InlineEngagement` resulted in these no longer being necessary, but after reading the code, that theory no longer make sense to me. I now suspect that a regression was introduced here recently, and these changes fixed the regression. What I know is that I tested a wide variety of cases, and the `InlineEngagement` appeared in the incorrect place without these changes, and in the correct place with these changes.