[$250] Android - Live mark down is not rendered when Description field is opened after saving it
If you havenβt already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 9.0.72-0 Reproducible in staging?: Y Reproducible in production?: N If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://github.com/Expensify/App/pull/53367 Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause - Internal Team
Action Performed:
- Launch ND or hybrid app.
- Go to FAB > Submit expense > Manual
- Enter amount > Next
- Select a recipient
- Tap Description
- Enter text with mark down and save
- Tap Description again
Expected Result:
The description field will show live mark down for the content with mark down
Actual Result:
Live mark down is not rendered when Description field is opened after saving it The same issue also happens in Private notes, room description and workspace description
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [x] Android: Standalone
- [x] Android: HybridApp
- [ ] Android: mWeb Chrome
- [ ] iOS: Standalone
- [ ] iOS: HybridApp
- [ ] iOS: mWeb Safari
- [ ] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/user-attachments/assets/7b361c8a-89de-47ef-b3a7-ff253ec10fcb
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~021864791346170591685
- Upwork Job ID: 1864791346170591685
- Last Price Increase: 2024-12-05
Issue Owner
Current Issue Owner: @eVoloshchak
Triggered auto assignment to @arosiclair (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.
π¬ A slack conversation has been started in #expensify-open-source
:wave: Friendly reminder that deploy blockers are time-sensitive β± issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
- Identify the pull request that introduced this issue and revert it.
- Find someone who can quickly fix the issue.
- Fix the issue yourself.
I reproduced on Android v9.0.72-0. Looks like iOS is unaffected.
None of the PRs in the checklist seem to be the issue.
I reproduced in dev with the latest on main, but I also reproduced with the last version, v9.0.71.2 checked out. I'm getting the sense that this never worked as expected. I already updated my staging app from the Play Store so I'm having trouble finding a way to revert it to the last version to confirm.
Looks like markdown doesn't render at all in the Description field on v9.0.70-9. This looks like a new feature and not a regression. Removing the blocker label
Triggered auto assignment to @zanyrenney (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
Job added to Upwork: https://www.upwork.com/jobs/~021864791346170591685
Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak (External)
Edited by proposal-police: This proposal was edited at 2024-12-11 09:47:37 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue
Live markdown is not rendered when Description field is opened after saving it (the same thing happens in Private notes, room description and workspace description fields).
What is the root cause of that problem?
The root of this issue comes from the Android: Native implementation of the applyMarkdownFormatting function call here in the MarkdownTextWatcher.java, specifically the fact that we only call it on afterTextChanged:
@Override
public void afterTextChanged(Editable editable) {
if (editable instanceof SpannableStringBuilder) {
mMarkdownUtils.applyMarkdownFormatting((SpannableStringBuilder) editable);
}
}
the problem with this being evident from the method name -> the live markdown styling is only applied when the after input changes and not when the input is mounted as it happens on iOS: Native for example.
This is also confirmed by the fact that if we change the text or remove a markdown symbol and add it back -> the live markdown will be applied again and show up as expected.
What changes do you think we should make in order to solve the problem?
[!note] I don't have a lot of experience with android native code, but I'll give this a shot and hopefully the
react-native-live-markdownmaintainers would be kind enough to chime in and correct any technical oversight if we are to move forward with this solution.
What we want here is to apply the live markdown formatting when the input is mounted (regardless of its focus state), to do this we need to apply the following change in the @expensify/react-native-live-markdown repo:
- In
MarkdownTextInputDecoratorView.javawithin theonAttachedToWindowmethod, add the following logic inside, at the end of theif (previousSibling instanceof ReactEditText)block which will apply the markdown formatting to the input on mount:
// add the imports at the top
import android.text.Editable;
import android.text.SpannableStringBuilder;
// add this inside, at the end of this if block
if (previousSibling instanceof ReactEditText) {
AssetManager assetManager = getContext().getAssets();
MarkdownUtils.maybeInitializeRuntime(assetManager);
mMarkdownUtils = new MarkdownUtils(assetManager);
mMarkdownUtils.setMarkdownStyle(mMarkdownStyle);
mReactEditText = (ReactEditText) previousSibling;
mTextWatcher = new MarkdownTextWatcher(mMarkdownUtils);
mReactEditText.addTextChangedListener(mTextWatcher);
Editable editable = mReactEditText.getText(); // <- new line
if (editable instanceof SpannableStringBuilder) { // <- new line
mMarkdownUtils.applyMarkdownFormatting((SpannableStringBuilder) editable); // <- new line
} // <- new line
}
The reason we add it there is because at that point the input is mounted and the next logical step would be to apply the markdown formatting. What the new logic does is it extracts the text from the input and applies the markdown formatting.
Or, to make things simpler we can add 1 line instead of the ones above, as suggested in https://github.com/Expensify/App/issues/53645#issuecomment-2535309095:
// add this inside, at the end of this if block
if (previousSibling instanceof ReactEditText) {
AssetManager assetManager = getContext().getAssets();
MarkdownUtils.maybeInitializeRuntime(assetManager);
mMarkdownUtils = new MarkdownUtils(assetManager);
mMarkdownUtils.setMarkdownStyle(mMarkdownStyle);
mReactEditText = (ReactEditText) previousSibling;
mTextWatcher = new MarkdownTextWatcher(mMarkdownUtils);
applyNewStyles(); // <- new line
}
[!important] This solution will fix the issue in all inputs where the issue is present, as required by the Expected result.
If there are any other details I missed or some technical oversight, I think that can be further discussed during review.
cc @tomekzaw @BartoszGrajdek for visibility since this is react-native-live-markdown related
Result video
Android: Native
https://github.com/user-attachments/assets/d5c08639-f5e2-423e-8488-348fc9f524cc
I agree with solution proposed by @ikevin127 in https://github.com/Expensify/App/issues/53645#issuecomment-2522101175 but I'd like to understand the issue a bit more since in the live-markdown example app, the Markdown formatting is already applied immediately during first render even without this change.
What I'm thinking is that on native, the screen which contains this input remains in the stack (for navigation purposes) and when reopening it and since there was no change, the android native code does not include logic to applyMarkdownFormatting when the screen containing the input is mounted again (revived from the stack).
This is the only explanation that would make sense to me when comparing our case with the live-markdown example app where there's no navigation stack involved as the markdown input is added directly in the App.tsx component.
It's just a hunch though, not entirely sure how to debug this to see why, other than looking at the code and figuring that it's because applyMarkdownFormatting is currently only called in afterTextChanged as mentioned in the proposal.
@arosiclair, @eVoloshchak, @zanyrenney Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Someone from SWM will also take a look at this to make sure we understand the root cause properly
cc @j-piasecki @Kicu
@arosiclair, @eVoloshchak, @zanyrenney Huh... This is 4 days overdue. Who can take care of this?
but I'd like to understand the issue a bit more since in the live-markdown example app, the Markdown formatting is already applied immediately during the first render even without this change.
It seems like in the live-markdown example, there's a (native) state update happening immediately after onAttachedToWindow is called. This state update causes the text to change, which in turn triggers the markdown text watcher.
The difference in the New Dot is that the state update doesn't happen or is received before onAttachedToWindow is called, not sure which. Though, I can see navigation delaying onAttachedToWindow call.
It may make sense to apply formatting in onAttachedToWindow as well, as not to rely on the state update triggering a text change.
Thanks @j-piasecki for checking this!
@ikevin127 We can proceed with your solution proposed in https://github.com/Expensify/App/issues/53645#issuecomment-2522101175.
One more thing β what do you think if instead of
Editable editable = mReactEditText.getText(); // <- new line
if (editable instanceof SpannableStringBuilder) { // <- new line
mMarkdownUtils.applyMarkdownFormatting((SpannableStringBuilder) editable); // <- new line
} // <- new line
we simply call
applyNewStyles();
? This way we prevent code duplication as well as make sure that text selection is preserved.
Looks good to me, thanks a lot everybody for chiming in and helping validate the root cause and improve the solution!
Updated proposal
- included solution improvement suggested in #53645 (comment)
cc @eVoloshchak for proposal review and if everything checks out, assignment
Once that happens, I'll be able to move forward with opening a PR on react-native-live-markdown then once merged there, will follow-up with the version bump on E/App.
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
@arosiclair, @eVoloshchak, @zanyrenney 6 days overdue. This is scarier than being forced to listen to Vogon poetry!
@eVoloshchak can you review please
Current assignee @arosiclair is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.
π£ @ikevin127 π An offer has been automatically sent to your Upwork account for the Contributor role π Thanks for contributing to the Expensify app!
Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review π§βπ» Keep in mind: Code of Conduct | Contributing π
Will open PR today!
Opened https://github.com/Expensify/react-native-live-markdown/pull/590 PR, tagged @tomekzaw as I'm not sure how review is handled there. Once that PR is merged, will sync up with the team in order to follow up with the version bump PR on E/App.
E/App PR https://github.com/Expensify/App/pull/54326 is open and ready for review! π
Note: From react-native-live-markdown version 0.1.209 to 0.1.210 there's only 1 PR and it's the fix mentioned in the comment above, therefore this is a straight-forward bump with only 1 change on the library's side.
@arosiclair I think an Android: Native build on the PR would facilitate testing and help ensure the fix works as expected before we merge the PR.
Reviewing label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.79-5 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
- https://github.com/Expensify/App/pull/54326
If no regressions arise, payment will be issued on 2025-01-07. :confetti_ball:
For reference, here are some details about the assignees on this issue:
- @eVoloshchak requires payment through NewDot Manual Requests
- @ikevin127 requires payment automatic offer (Contributor)
@eVoloshchak / @ikevin127 @zanyrenney @eVoloshchak / @ikevin127 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]