Fix text in compose box blending with background on theme change#39872
Fix text in compose box blending with background on theme change#39872tomekzaw wants to merge 2 commits intoExpensify:mainfrom
Conversation
|
@hoangzinh 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] |
|
@hoangzinh Yeah that's correct |
|
@hoangzinh I've resolved conflicts by bumping |
| "@babel/plugin-proposal-private-methods": "^7.18.6", | ||
| "@babel/plugin-proposal-private-property-in-object": "^7.21.11", | ||
| "@dotlottie/react-player": "^1.6.3", | ||
| "@expensify/react-native-live-markdown": "github:Expensify/react-native-live-markdown#f762be6fa832419dbbecb8a0cf64bf7dce18545b", |
There was a problem hiding this comment.
Hi @tomekzaw are you sure if this commit has been merged to the latest version? I couldn't find it here Expensify/react-native-live-markdown@f762be6
There was a problem hiding this comment.
@hoangzinh Yep, all necessary changes are already on Live Markdown's main.
There was a problem hiding this comment.
Is it possible to bump to 0.1.40 only?
There was a problem hiding this comment.
Unfortunately not, because 0.1.40 does not support new architecture
There was a problem hiding this comment.
but we can close this PR in favor of #40102 which bumps Live Markdown to 0.1.47
There was a problem hiding this comment.
@thienlnam what do you think about #39872 (comment)? Or we can go ahead with this PR?
There was a problem hiding this comment.
I think we just close this in favor of the PR #40102 0.1.47 and then verify it's fixed there.
There was a problem hiding this comment.
I'm realizing though since we're making a lot of changes to react-native-live-markdown, we end up having multiple PRs that bump the version. We'll probably want a better way to handle as opposed to having each outdated minor version bump reviewed
There was a problem hiding this comment.
@thienlnam Fair point, we might want to batch updates on our side and just submit one PR with version bump that fixes multiple issues (unless there are other changes in E/App code). What do you think?
There was a problem hiding this comment.
I think that's a good idea - perhaps we group a few different bugs together so we don't end up with multiple PRs that are reviewing an outdated version
|
Closing since we bumped to .47 |
Details
This PR fixes the color of the text in Live Markdown Preview when toggling theme (e.g. via shift+cmd+a shortcut in iOS simulator).
The root cause of the issue was fixed in Expensify/react-native-live-markdown#269 which was released in 0.1.40 – this PR is just a version bump.
before.mp4
after.mp4
Fixed Issues
$ #39348
PROPOSAL:
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop