[$500] Chat -In offline, while edit comment, tapping on emoji picker moves cursor to compose box.

[izarutskaya]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.2-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause-Internal Team
Slack conversation: @

Action Performed:

1. Go to https://staging.new.expensify.com/
2. Tap profile icon----Preferences
3. Toggle on forced offline
4. Navigate to LHN
5. Tap on a report
6. Send a message
7. Long press the sent message and select edit comment
8. Tap emoji picker and note the cursor

Expected Result:

In offline, while edit comment, tapping on emoji picker must not move cursor to compose box

Actual Result:

In offline, while edit comment, tapping on emoji picker moves cursor to compose box.

Workaround:

Unknown

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

Bug6287169_1700659269186.cursor.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f8d143448ba4dda0
• Upwork Job ID: 1727338662062862336
• Last Price Increase: 2023-11-29

[melvin-bot]

Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01f8d143448ba4dda0

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[DylanDylann]

Proposal

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

Chat -In offline, while edit comment, tapping on emoji picker moves cursor to compose box

What is the root cause of that problem?

App/src/pages/home/report/ReportActionItemMessageEdit.js

Line 431 in f1e085b

if (_.contains([messageEditInput, emojiButtonID], relatedTargetId)) {

In here relatedTargetID is null, so we call setShouldShowComposeInputKeyboardAware(true). It makes the main composer appear

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

App/src/pages/home/report/ReportActionItemMessageEdit.js

Line 431 in f1e085b

if (_.contains([messageEditInput, emojiButtonID], relatedTargetId)) {

In here we should check both targetID and relatedTargetID like that

const relatedTargetId = lodashGet(event, 'nativeEvent.relatedTarget.id') || lodashGet(event, 'nativeEvent.target.id');;

What alternative solutions did you explore? (Optional)

[s-alves10]

Proposal

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

In edit composer, tapping on emoji picker button shows the main composer and moves cursor to it

What is the root cause of that problem?

App/src/pages/home/report/ReportActionItemMessageEdit.js

Lines 431 to 434 in 34d165e

	if (_.contains([messageEditInput, emojiButtonID], relatedTargetId)) {
	return;
	}
	setShouldShowComposeInputKeyboardAware(true);

As you can see above, we don't show the main composer when the edit composer gets blurred by emoji button or by tapping on other edit composer.

The problem here is that the id of emoji button was not properly set. When emoji button is tapped, relatedTargetId is empty.
Please check the following code

App/src/components/Pressable/GenericPressable/index.tsx

Lines 10 to 19 in 34d165e

	<GenericPressable
	// eslint-disable-next-line react/jsx-props-no-spreading
	{...props}
	ref={ref}
	// change native accessibility props to web accessibility props
	focusable={focusable}
	tabIndex={!accessible || !focusable ? -1 : 0}
	role={props.accessibilityRole as Role}
	id={props.nativeID}
	aria-label={props.accessibilityLabel}

We removed the nativeID in this PR and used id instead intentionally, but we didn't remove the above code. so id props is redefined as undefined

This is the root cause

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

Simply removing the below code would solve the issue

App/src/components/Pressable/GenericPressable/index.tsx

Line 18 in 34d165e

id={props.nativeID}

Note: We might need to remove all nativeID from the project

Result

31709.mp4

What alternative solutions did you explore? (Optional)

[abdulrahuman5196]

I am not able to understand the issue itself. Could someone explain more?

In offline, while edit comment, tapping on emoji picker moves cursor to compose box.
what does this mean actually? Should the cursor be in search or something else?

[s-alves10]

@abdulrahuman5196

Try the following steps in mobile web platforms

1. Navigate to any chat
2. Send a message
3. Switch the message just sent to edit mode
4. Tap on the emoji button in the edit composer
5. Check if the main composer is shown and the emoji picker is opened

[abdulrahuman5196]

Just back from weekend. Will check again tonight on the repro steps.

[abdulrahuman5196]

Got it. Proposal from @DylanDylann here #31709 (comment) and proposal from @s-alves10 here #31709 (comment) is working.

@s-alves10 As part of RNW migration, it seems we replaced the usage of nativeID with id. But the mentioned code you had suggested Simply removing the below code would solve the issue removes the value id props to pressable not nativeID. Why do you want that? Could you explain more in detail.

[s-alves10]

@abdulrahuman5196
Please check the code below

App/src/pages/home/report/ReportActionItemMessageEdit.js

Line 445 in 5db665c

id={emojiButtonID}

As you can see, we are already passing id props to the EmojiPickerButton

And please check the below code

App/src/components/Pressable/GenericPressable/index.tsx

Lines 12 to 18 in 5db665c

	{...props}
	ref={ref}
	// change native accessibility props to web accessibility props
	focusable={focusable}
	tabIndex={!accessible || !focusable ? -1 : 0}
	role={props.accessibilityRole as Role}
	id={props.nativeID}

In line 12, {...props} would pass id props through to GenericPressable component, but line 18 redefine the id props to non-existing props.nativeID

We need to pass through the id props, and that's why I suggest to remove the line 18

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[abdulrahuman5196]

I need to confirm which proposal suits, will provide update by tomorrow.

[abdulrahuman5196]

Missed the above, Will provide in my morning

[melvin-bot]

@mallenexpensify @abdulrahuman5196 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[abdulrahuman5196]

@s-alves10 's proposal here #31709 (comment) looks good and works well.
Also, @s-alves10 kindly add the explanation you mentioned to the original proposal.

🎀 👀 🎀
C+ Reviewed

[melvin-bot]

Triggered auto assignment to @yuwenmemon, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[DylanDylann]

@s77rt I noticed that if applying @s-alves10's proposal, after selecting emoji in edit composer, the main composer will be focus

@s-alves10 Could you also check this case?

[s-alves10]

@DylanDylann Which platform did you test on?

[DylanDylann]

I tested Safari-web on an iPhone emulator. Oh, wait..... It seems I can't reproduce this issue. Please check again. If you can reproduce it, please also check the case: after selecting emoji in edit composer, ensure that edit composer will be focus instead of main composer

[s-alves10]

yeah, I'm not able to reproduce

[mallenexpensify]

If y'all can't reproduce, I'm going to close. If you disagree, please comment

[s-alves10]

@mallenexpensify

This issue is still reproducible. What I meant in #31709 (comment) is that I wasn't able to reproduce what @DylanDylann mentioned in #31709 (comment). This issue is reproducible in mWeb chrome

bandicam.2023-12-09.19-11-27-465.mp4

[mallenexpensify]

Reopening to try to reproduce via Browserstack. Will get to soon.

[mallenexpensify]

@s-alves10

In offline, while edit comment, tapping on emoji picker moves cursor to compose box.

So, the issue is that the cursor briefly shows in the compose box before showing the emoji?

If so, I don't think this is a high-priority bug.

[s-alves10]

@mallenexpensify

This issue isn't related to whether offline or online. I think this is a bug worth fixing

[mallenexpensify]

the issue is that the cursor briefly shows in the compose box before showing the emoji?

@s-alves10 this is the bug though, correct?

[s-alves10]

@mallenexpensify

yes

[melvin-bot]

@mallenexpensify @abdulrahuman5196 this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[abdulrahuman5196]

This issue was already C+ approved. @mallenexpensify can we assign @yuwenmemon for internal engineer review?

[melvin-bot]

@mallenexpensify, @abdulrahuman5196 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[abdulrahuman5196]

@mallenexpensify to check on this - #31709 (comment)

[mallenexpensify]

I'm going to close, I think this is low priority and not worth investing internal engineering resources.
More info here https://expensify.slack.com/archives/C01GTK53T8Q/p1702496349813859