Set the label's height to static value, to prevent message height warnings.
Details
Details
Run the app on iOS and Android. Created new chat and checked if the warnings appear:
- new message,
- edited message,
- edited message with reaction,
- not edited message with reaction/sidebar.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- kuba/messageediting
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
We should source this height from the same place that we source it for message height calculations. Eg. see how timestampHeight is used. Please replace this with a variable that is also used somehow in messageItemHeight
| native/chat/inline-engagement.react.js | ||
|---|---|---|
| 246 ↗ | (On Diff #25455) | This appears to be a height constant, so it should be defined in a constant that is used in both height measurement and the definition here |
| 343 ↗ | (On Diff #25455) | We should not be returning these things from the same file as it will break Fast Refresh – see here |
| native/chat/inline-engagement.react.js | ||
|---|---|---|
| 31 ↗ | (On Diff #25455) | This should probably be defined in native/chat/chat-constants.js |
Comment Actions
High-level feedback here is that all numbers that contribute to the height of the component should be hard-coded as constants, and then the functions for determining the height of the component should simply sum those very same constants. This includes margin, padding, etc.
Comment Actions
It works similarly to inlineEngagementLeftStyle and other styles in chat-constants.js, without adding it to height measurement
Thanks for explaining, @kuba! Indeed I can see it works similarly to that... both inlineEngagementLeftStyle and your new inlineEngagementLabelStyle.topOffset are used to define styles, but are not used for height calculation.
One question before I accept – can you explain a little bit why inlineEngagementLeftStyle and inlineEngagementLabelStyle.topOffset are not used in height calculations?
Might be helpful to include visuals with lines indicating what these values handle, and how they are handled already elsewhere!
Comment Actions
It seems like the top prop is only an offset, which doesn't change the actual position (size) of the component, but only the position where the component is rendered.
I experimented a little bit, and here are the results:
Top offset: -5.
Top offset: 50
It changes the position where the label is displayed, but not where the component takes place.