[HOLD /react-native-live-markdown#366] [$250] IOU - one character descriptions are not saved

[lanitochka17]

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: 1.4.64-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Issue found when executing PR #40176

Action Performed:

1. Navigate to staging.new.expensify.com
2. Click on FAB > Submit expense > Manual
3. Input an amount
4. Select a user
5. Click on "Description"
6. Insert one character letter without any spaces afterwards and should be any character except numbers (e.g "A" or "B")
7. Click on Save

Expected Result:

One character letter should be saved

Expected Result:

One character letter is not saved as a Description but works fine with one digit numbers and 2 character letters. In order to save the description user has to insert whitespace by tapping on space after the character and hit save

Actual Result:

Unknown

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6458722_1713873714439.Screen_Recording_20240423_145724_Chrome.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0160ab7a88d7f75bcd
• Upwork Job ID: 1782806229081673728
• Last Price Increase: 2024-05-07

Issue Owner

Current Issue Owner: @sobitneupane

[melvin-bot]

Triggered auto assignment to @sakluger (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.

[lanitochka17]

@sakluger FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

We think that this bug might be related to #vip-vsp

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0160ab7a88d7f75bcd

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @sobitneupane (External)

[sakluger]

I'm adding this to the Wave-Collect project since it's related to submitting expenses.

[gijoe0295]

I couldn't reproduce this issue even on staging.

[josh-prof]

I couldn't reproduce this issue either.
I think that issue is related to the encoding, so the one character wasn't recognized as finished.
I am not sure about it.

[proverbed]

Same here, unable to reproduce locally, or on staging

[charles-liang]

Proposal

Please re-state the problem that we are trying to solve in this issue.

IOU - one character descriptions are not saved

What is the root cause of that problem?

the bug is from https://github.com/Expensify/react-native-live-markdown

When the system launches with an input method that has autocomplete, it triggers the onCompositionStart event, setting compositionRef.current to true. This causes handleOnChangeText to return early.

Refer to: #40799 (comment)

The same issue occurs when you input one character and then select a word from the keyboard's autocomplete suggestions—the entire word does not update afterwards.

2024-04-26.21.13.07.mov

What changes do you think we should make in order to solve the problem?

I suggest removing these two lines in https://github.com/Expensify/react-native-live-markdown

      const nativeEvent = e.nativeEvent; // move this line from below to here

    if (compositionRef.current 
         && e.nativeEvent.inputType === 'insertCompositionText' // add this condition not effect the PR https://github.com/Expensify/react-native-live-markdown/pull/210
        ) {
          updateTextColor(divRef.current, e.target.innerText);
          compositionRef.current = false;
          return;
        }

What alternative solutions did you explore? (Optional)

I think this solution is not very elegant, but it indeed solves two problems simultaneously.
I suggest other two solution:

1. Mid-term solution: Save the previous character, and if there's no subsequent input, use a timer to restore the following characters, and recall the remaining code that follows.

2. Long-term solution: redesin below. The problem lies in the following code, which clears the input sequence, thus losing memory of the previous inputs. so the PR Diacritics fix react-native-live-markdown#210 need to add a early rerturn to stop this code and make this issue.

        let text = '';
        switch (nativeEvent.inputType) {
          case 'historyUndo':
            text = undo(divRef.current);
            break;
          case 'historyRedo':
            text = redo(divRef.current);
            break;
          default:
            text = parseText(divRef.current, e.target.innerText, processedMarkdownStyle).text;
        }

Then, the PR Expensify/react-native-live-markdown#210 used a temporary solution, and this time another short-term solution is added. This will become difficult to modify later and turn into technical debt if not redesign.

[sakluger]

It seems like some people are having trouble reproducing the issue. @charles-liang were you able to reproduce the behavior? If so, did you do anything that wasn't outlined in the reproduction steps above?

[charles-liang]

1. Navigate to staging.new.expensify.com
2. Click on FAB > Submit expense > Manual
3. Input an amount
4. Select a user
5. Click on "Description"
6. "The system keyboard must be set to predictive text mode, or what's called autocomplete mode. Just type a few characters."
7. Click save when the system's predictive text is active.

In my case, entering a single character and then clicking save works fine. However, when entering two characters, setValue is not triggered, and onChangeText only receives the value of the first character, but onBlur can capture the value of two characters. Then, clicking save will result in the loss of the second value. I believe the problem is the same, that sometimes setValue is not triggered during the system keyboard's autocomplete, causing the value not to be updated.

[sobitneupane]

Thanks for the proposal and reproduction steps @charles-liang. I was able to reproduce the issue with single character.

Can you please add more details in your RCA? Why does the setValue function only receives part of the input?

"When the onBlur event triggers, the text value is 'g1', but the setValue function only receives 'g'."

[charles-liang]

Finally, I turned off isMarkdownEnabled and set it to false, which fixed the issue

App/src/pages/iou/request/step/IOURequestStepDescription.tsx

Lines 164 to 184 in 34434a5

	<InputWrapperWithRef
	InputComponent={TextInput}
	inputID={INPUT_IDS.MONEY_REQUEST_COMMENT}
	name={INPUT_IDS.MONEY_REQUEST_COMMENT}
	defaultValue={currentDescription}
	label={translate('moneyRequestConfirmationList.whatsItFor')}
	accessibilityLabel={translate('moneyRequestConfirmationList.whatsItFor')}
	role={CONST.ROLE.PRESENTATION}
	ref={(el) => {
	if (!el) {
	return;
	}
	inputRef.current = el;
	updateMultilineInputRange(inputRef.current);
	}}
	autoGrowHeight
	maxAutoGrowHeight={variables.textInputAutoGrowMaxHeight}
	shouldSubmitForm
	isMarkdownEnabled
	/>
	</View>

If isMarkdownEnabled must be set to true, I found the root cause in react-native-live-markdown.

https://github.com/Expensify/react-native-live-markdown/blob/328053892371e9dd22590fdd8cda31a1c4881871/src/MarkdownTextInput.web.tsx#L326-L336

In some cases, when compositionRef.current is true, the function returns early without output handleOnChangeText3, and it does not fire the onChangeText and onChange events

[sobitneupane]

If isMarkdownEnabled must be set to true, I found the root cause in react-native-live-markdown.

Great finding @charles-liang

In that case I am pretty sure, the issue lies on Composer as well. So, setting isMarkdownEnabled to false is not an option. What's your proposal for the solution?

[charles-liang]

Proposal

Updated

[sobitneupane]

Thanks for the update @charles-liang. Those changes were introduced by Expensify/react-native-live-markdown#210 PR to solve #36942 issue. Can you please verify that the change you are suggesting do not cause any regression?

[charles-liang]

@sobitneupane Received. I have revised the proposal and proposed three solutions at different levels.

[charles-liang]

Proposal

Updated

[sobitneupane]

@charles-liang Proposal is not updated. Also, please consider #40799 (comment) in your solution.

[melvin-bot]

Triggered auto assignment to @twisterdotcom (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.

[sakluger]

@twisterdotcom assigning you while I'm OOO through May 31. The latest is that the PR is being worked on here: Expensify/react-native-live-markdown#343.

[melvin-bot]

@sakluger, @twisterdotcom, @charles-liang, @blimpich, @sobitneupane Whoops! This issue is 2 days overdue. Let's get this updated quick!

[sobitneupane]

The PR is being reviewed.

[melvin-bot]

@sakluger, @twisterdotcom, @charles-liang, @blimpich, @sobitneupane 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[blimpich]

@charles-liang what is the status of the PR? Still coming along?

[charles-liang]

@blimpich I submit the pr for Expensify/react-native-live-markdown#343 and waiting approval

[blimpich]

@sobitneupane any update since your last comment? Have you had a chance to review the PR?

[sakluger]

I'm back - thanks @twisterdotcom for watching, I'll unassign you now.

@sobitneupane what is the latest?

[BartoszGrajdek]

Hi! I have a question regarding this issue - is this only happening on mWeb?

[sobitneupane]

@BartoszGrajdek I could reproduce it only in mWeb/Chrome

[BartoszGrajdek]

Good, I know that your problems are likely related to the composition event types. We're thinking about disabling the composition handling on mWeb due to multiple bugs that are related to it. Can you check if the bug is still appearing using this version of the library?

[sobitneupane]

I am pretty sure it should solve the issue. I will let you know after testing it.

cc: @charles-liang

[charles-liang]

@sobitneupane Thank you very much for your review. If you need me to follow up, please let me know.

[sobitneupane]

I tested Expensify/react-native-live-markdown#366 PR and the issue will not be reproducible with the changes introduced by the PR.

[sobitneupane]

@sakluger We can hold this issue for now and retest it after Expensify/react-native-live-markdown#366 gets merged and the version of react-native-live-markdown gets bumped in the app.

[sakluger]

Added the hold, but leaving as daily since that PR was merged and we just need to wait for the app version to be bumped.

[sakluger]

@sobitneupane are we good to retest yet? Or do we still need to wait for the version of RNLM to be bumped?

[melvin-bot]

@sakluger, @charles-liang, @blimpich, @sobitneupane Whoops! This issue is 2 days overdue. Let's get this updated quick!

[sobitneupane]

are we good to retest yet? Or do we still need to wait for the version of RNLM to be bumped?

I can no longer reproduce the issue in staging.

[sakluger]

Nice! I will go ahead and close. I don't think any payment is due here, but please bring up in Slack if you disagree.