Skip to content

Commit

Permalink
[native] fix forcing the incorrect height for multimedia messages
Browse files Browse the repository at this point in the history
Summary:
This diff is part 3/3 to fix forcing the incorrect height for messages with inline engagement in prod.

This [[ https://linear.app/comm/issue/ENG-4606/fix-forcing-incorrect-height-for-messages-with-inline-engagement-in | linear ]] comment explains this issue and what I did to address this

> Since item.contentHeight is now the height of the inner text message AND the height of the inline engagement we are now forcing the wrong height for the inner text message component. An idea I want to explore to address this is to force the height higher up in the component tree (somewhere like ComposedMessage) where both the "inner message" and the inline engagement is rendered so that when we force the height withitem.contentHeight, it is correct

Depends on D8798

Test Plan:
Please see demo video where I force and unforce the correct height of the multimedia messages and it did not change visually (outside of the green background) and I did not get any error logs talking about incorrect height measurements, I also purposely force an incorrect height (which resulted in some logs about incorrect height measurements.

Also please note for the video, I set a green background so that it is easier to visually see what is happening, but these are only for the demo videos

With the correct height measurement:
{F686605}

With the incorrect height measurement:
{F686622}

Here is also a demo video to show that there was no regression by removing the `expand` style (I pressed and was able to view the the full screen image for multimedia messages with both a single image and multiple images):
{F689819}

Also here is another demo video to view the the full screen image for multimedia messages with both a single image and multiple images W/O an inline engagement
{F689917}

Reviewers: atul, kamil, tomek, ashoat

Reviewed By: ashoat

Subscribers: ashoat, tomek

Differential Revision: https://phab.comm.dev/D8799
  • Loading branch information
ginsueddy committed Aug 15, 2023
1 parent 14a2d48 commit e44f2c5
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 8 deletions.
5 changes: 5 additions & 0 deletions native/chat/composed-message.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,11 @@ class ComposedMessage extends React.PureComponent<Props> {
// want to measure it in Message to see if it's correct
if (item.messageShapeType === 'text') {
viewStyle.push({ height: item.contentHeight });
} else if (item.messageShapeType === 'multimedia') {
const height = item.inlineEngagementHeight
? item.contentHeight + item.inlineEngagementHeight
: item.contentHeight;
viewStyle.push({ height });
}
}

Expand Down
10 changes: 2 additions & 8 deletions native/chat/multimedia-message.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type {
} from '@react-navigation/native';
import { useNavigation, useRoute } from '@react-navigation/native';
import * as React from 'react';
import { StyleSheet, View } from 'react-native';
import { View } from 'react-native';

import { messageKey } from 'lib/shared/message-utils.js';
import {
Expand Down Expand Up @@ -217,7 +217,7 @@ class MultimediaMessage extends React.PureComponent<Props, State> {
shouldDisplayPinIndicator={shouldDisplayPinIndicator}
{...viewProps}
>
<View style={styles.expand} onLayout={this.onLayout} ref={this.viewRef}>
<View onLayout={this.onLayout} ref={this.viewRef}>
<InnerMultimediaMessage
item={item}
verticalBounds={verticalBounds}
Expand All @@ -232,12 +232,6 @@ class MultimediaMessage extends React.PureComponent<Props, State> {
}
}

const styles = StyleSheet.create({
expand: {
flex: 1,
},
});

const ConnectedMultimediaMessage: React.ComponentType<BaseProps> =
React.memo<BaseProps>(function ConnectedMultimediaMessage(props: BaseProps) {
const navigation = useNavigation();
Expand Down

0 comments on commit e44f2c5

Please sign in to comment.