[native] Fix display of TooltipInlineEngagement
ClosedPublic
Actions

Authored by ashoat on Oct 9 2024, 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

No Lint Coverage

Unit

No Test Coverage

Event Timeline

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

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

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

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

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

tomek added inline comments.

native/chat/inline-engagement.react.js
494	Why this can be removed?

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

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

native/chat/inline-engagement.react.js
494	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.