Fix 75496: Android-Chat-using header markdown, typing texts from the third line not fit on composer box#77607
Conversation
|
@eVoloshchak Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Videos look good to me 👍 |
| onKeyPress={handleKeyPress} | ||
| textAlignVertical="top" | ||
| style={[styles.textInputCompose, isComposerFullSize ? styles.textInputFullCompose : styles.textInputCollapseCompose]} | ||
| style={Platform.OS === 'android' ? StyleSheet.flatten([...composerStyle, styles.androidComposerFix]) : composerStyle} |
There was a problem hiding this comment.
@annaweber830, according to https://github.com/Expensify/App/blob/main/contributingGuides/philosophies/CROSS-PLATFORM.md#platform-specific-file-extensions
Platform specific code MUST be placed in dedicated files and folders
But I don't think it makes sense to create a separate *.android.ts file for this case, let's move Platform.OS check directly to src/styles/index file (textInputCollapseCompose) so it's consistent with the rest of the app. This will also allow us to get rid of androidComposerFix name
Screen.Recording.2025-12-17.at.20.48.13.mov@annaweber830, this is resolved in normal composer, but text is still cut off when editing the message |
I fixed this issue. I updated my PR. |
|
@annaweber830, nice! And since we use different styles for full-size composer ( Screen.Recording.2025-12-19.at.14.36.01.mov |
Hi @eVoloshchak I updated my PR. |
| alignSelf: 'stretch', | ||
| flex: 1, | ||
| maxHeight: '100%', | ||
| verticalAlign: 'top', |
There was a problem hiding this comment.
Can this be moved to textInputCompose? Since we use this style in both components
src/styles/utils/index.ts
Outdated
| } | ||
|
|
||
| const composerLineHeight = styles.textInputCompose.lineHeight ?? 0; | ||
| const composerLineHeight = variables.lineHeightXLarge; |
There was a problem hiding this comment.
Why is this change needed? It's already lineHeight: variables.lineHeightXLarge in textInputCompose
There was a problem hiding this comment.
On Android, styles.textInputCompose.lineHeight is undefined, so the height of the composer is 0 in the original code.
Therefore, we need to explicitly set the composer height, and I have updated the code accordingly.
src/styles/utils/index.ts
Outdated
| } | ||
|
|
||
| const composerLineHeight = styles.textInputCompose.lineHeight ?? 0; | ||
| const composerLineHeight = Platform.OS === 'android' ? variables.lineHeightXLarge : (styles.textInputCompose.lineHeight ?? 0); |
There was a problem hiding this comment.
But isn't this the same? styles.textInputCompose.lineHeight is already defined as variables.lineHeightXLarge
Line 2095 in 6913f8b
There was a problem hiding this comment.
On Android, styles.textInputCompose.lineHeight is undefined, so the height of the composer is 0 in the original code.
Therefore, we need to explicitly set the composer height, and I have updated the code accordingly.
There was a problem hiding this comment.
Ah, I see. In that case, could we just do const composerLineHeight = variables.lineHeightXLarge ?? 0
?
|
bump for review @eVoloshchak |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-01-07.at.12.01.16.movAndroid: mWeb ChromeScreen.Recording.2026-01-07.at.12.02.47.moviOS: HybridAppScreen.Recording.2026-01-07.at.12.05.49.moviOS: mWeb SafariScreen.Recording.2026-01-07.at.12.08.14.movMacOS: Chrome / SafariScreen.Recording.2026-01-06.at.16.32.40.mov |
eVoloshchak
left a comment
There was a problem hiding this comment.
Looking good!
@annaweber830, could you please change the latest test step to
- Verify the text fits into the composer box without being cut off
And add tests from #12669 here, according to the comment in styles.ts
Be extremely careful when editing the compose styles, as it is easy to introduce regressions. Make sure you run the following tests against any changes: #12669
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/arosiclair in version: 9.2.96-1 🚀
|
|
🚀 Deployed to production by https://github.com/arosiclair in version: 9.2.96-6 🚀
|
|
🚀 Deployed to production by https://github.com/arosiclair in version: 9.2.96-6 🚀
|
|
🚀 Deployed to production by https://github.com/arosiclair in version: 9.2.96-6 🚀
|
Explanation of Change
Fixed Issues
$ #75496
PROPOSAL:#75496 (comment)
Tests
Offline tests
Same as Tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2025-12-13.at.3.31.03.AM.mp4
Android: mWeb Chrome
Screen.Recording.2025-12-13.at.3.39.00.AM.mp4
iOS: Native
Screen.Recording.2025-12-13.at.4.09.30.AM.mp4
iOS: mWeb Safari
Screen.Recording.2025-12-13.at.4.10.53.AM.mp4
MacOS: Chrome / Safari
Screen.Recording.2025-12-13.at.3.44.20.AM.mp4