Use simple styles for code blocks in Native platforms#56924
Use simple styles for code blocks in Native platforms#56924grgia merged 7 commits intoExpensify:mainfrom
Conversation
|
@allgandalf 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] |
|
Will trigger a test build for us, cc @Expensify/design |
|
@shawnborton can you trigger a build please |
|
Oops sorry about that, I had forgotten the label. Trying again now! |
|
🚧 @shawnborton has triggered a test build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
Probably need to merge main on this one? |
|
working on it – will prepare the PR for review and add screenshots/test steps |
|
@shawnborton @allgandalf PR is ready for complete review. |
|
🚧 @shawnborton has triggered a test build. You can view the workflow run here. |
|
I triggered a new test build, let's see what happens. In the meantime, @fabioh8010 one thing I notice here is that it feels like the text sits far down within the highlighted area: Meaning, it feels like there is more BG color above the text than below it. Anything we can do to adjust that? Does the inline code have the same line height as the other text (20px)? |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
|
Dang, I think all iOS adhoc builds have been busted today... might want to check in the open source room to see what is going on there. |
@shawnborton These are the styles that are being passed to the code block Text in native: As you can see line height is 20px. I think this impression you had is because of the different font we use for inline code blocks (
|
|
@shawnborton Ok, now I understand what you are mentioning, I'm going to have a look today! |
|
@shawnborton The problem seems to happen when you have emojis inside the code blocks. It will happen even if I remove all styles from the code block Unfortunately I couldn't find the root cause yet. Do you think this is a blocker for now or we can proceed with this fix? |
|
Hmm I think we can proceed, but let's definitely keep investigating and see if we can solve it. |
|
@fabioh8010 is this ready for final review ? |
|
@allgandalf Yes |
Reviewer Checklist
Screenshots/VideosMacOS: Chrome / SafariScreen.Recording.2025-02-26.at.2.19.36.PM.mov |
|
@allgandalf The background color should be the same as before, I didn't change them in my PR. I think they look lighter because we don't have the borders anymore. |
|
ahh, now that you point it out, yes, it makes sense! |
There was a problem hiding this comment.
Simplified!!!, thanks for the hard work @fabioh8010
grgia
left a comment
There was a problem hiding this comment.
Changes LGTM, @shawnborton do you want to do a final pass?
|
All good on my end. And then do we have follow up GHs in place to make sure we don't lose track of progress here? Seems like we still want to investigate a few things regarding emoji line heights, as well as borders. |
I think we can create a follow-up issue to proceed directly to investigate the correct fix in the native side? |
|
Works for me! |
|
✋ 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/grgia in version: 9.1.7-1 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.1.7-2 🚀
|














Explanation of Change
This PR fixes code block misalignments in native platforms by simplifying the stylings for these platforms. For a follow-up we will work in a proper fix for it inside
react-native.Fixed Issues
$ #53458
PROPOSAL: #53458 (comment)
Tests
For Android/iOS
Lorem ipsum `dolor 🚀` sit amet, con`s``e`ctetur `adipiscing elit, sed do eiusmod tempor incididunt` ut labore et dolore `magna` ali`q`u`a`. 🚀For the other Platforms
Lorem ipsum `dolor 🚀` sit amet, con`s``e`ctetur `adipiscing elit, sed do eiusmod tempor incididunt` ut labore et dolore `magna` ali`q`u`a`. 🚀Offline tests
Same as Tests.
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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-02-18.at.18.45.09-compressed.mov
Android: mWeb Chrome
Screen.Recording.2025-02-18.at.18.50.46-compressed.mov
iOS: Native
Screen.Recording.2025-02-18.at.19.04.51-compressed.mov
iOS: mWeb Safari
Screen.Recording.2025-02-18.at.19.08.04-compressed.mov
MacOS: Chrome / Safari
Screen.Recording.2025-02-18.at.19.09.15-compressed.mov
Screen.Recording.2025-02-18.at.19.10.22-compressed.mov
MacOS: Desktop
Screen.Recording.2025-02-18.at.19.13.16-compressed.mov