[$500] Chat - Compose box is shown again after user is in edit mode and dismiss keyboard

[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.32-2
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:

Issue found when executing PR #34404

Action Performed:

Pre-requisite: user must be logged in and have sent a message to any chat

1. Go to a chat where you have sent a message
2. Tap and hold on the message
3. Select "Edit comment"
4. Tap on the box to edit the comment for the keyboard to appear
5. Tap "Done" button on the top right corner of the keyboard

Expected Result:

tapping the "done" button on the keyboard should close the edited chat message and the keyboard should be dismissed

Actual Result:

The compose box is shown again after the keyboard is dismissed

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

Add any screenshot/video evidence

Bug6355036_1706210544427.Gftm1132_1_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ade0e4b570684292
• Upwork Job ID: 1750604363283750912
• Last Price Increase: 2024-02-08
• Automatic offers:
• ishpaul777 | Contributor | 0

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[lanitochka17]

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

[ZhenjaHorbach]

Proposal

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

Chat - Compose box is shown again after user is in edit mode and dismiss keyboard

What is the root cause of that problem?

when we click on the done button, we trigger onBlur

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

Lines 443 to 451 in caf91ae

	onBlur={(event: NativeSyntheticEvent<TextInputFocusEventData>) => {
	setIsFocused(false);
	// @ts-expect-error TODO: TextInputFocusEventData doesn't contain relatedTarget.
	const relatedTargetId = event.nativeEvent?.relatedTarget?.id;
	if (relatedTargetId && [messageEditInput, emojiButtonID].includes(relatedTargetId)) {
	return;
	}
	setShouldShowComposeInputKeyboardAware(true);
	}}

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

Actually, this is Quite a difficult task

I checked documentation RN and StackOverflow and couldn't find a listener who was able to track done button (Including onSubmitEditing is not suitable as in the second proposition)

All propositions from StackOverflow are associated with a focusout event
But to call this event just press on the area outside the input

https://stackoverflow.com/questions/4971061/capture-done-button-click-in-iphones-virtual-keyboard-with-javascript

I also thought about using InputAccessoryViewwhich replaces the original accessory view with a custom one

https://reactnative.dev/docs/inputaccessoryview

And it works perfect for native IOS. We can add just custom done button and use it

But unfortunately
This is a very crude component in React-native-web

But since this problem is relevant only for iOS web

I decided to compare the events of pressing outside the input and pressing the done button

And notice that these events differ event.nativeEvent?.relatedTarget
It's actually logical
Because when we press outside the input
We are interacting with another component on the screen anyway
Unlike the done button where interaction is limited to the keyboard

So I think we can update onBlur function and add new condition
And when relatedTargetId is null
We will close draft input

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

Lines 439 to 447 in efd81f5

	onBlur={(event: NativeSyntheticEvent<TextInputFocusEventData>) => {
	setIsFocused(false);
	// @ts-expect-error TODO: TextInputFocusEventData doesn't contain relatedTarget.
	const relatedTargetId = event.nativeEvent?.relatedTarget?.id;
	if (relatedTargetId && [messageEditInput, emojiButtonID].includes(relatedTargetId)) {
	return;
	}
	setShouldShowComposeInputKeyboardAware(true);
	}}

What alternative solutions did you explore? (Optional)

NA

[Christinadobrzyn]

oh yep, I can reproduce this following the steps in the OP. I wonder if this is a regression but I can't find any recently closed GHs that might be linked to this. @parasharrajat what do you think?

[Christinadobrzyn]

I think we're waiting for proposals unless we can link this as a regression to another job

[Christinadobrzyn]

@parasharrajat can you review this proposal to see if it will work? #35179 (comment)

[parasharrajat]

I don't think this is a bug. And same goes for the issue related to the linked PR #33466.

When we blur the focus the composer pops back.

@lanitochka17 Could you please confirm if this bug was found via old regression steps or its a new one?

[parasharrajat]

@Christinadobrzyn Could you please raise this internally and confirm if this is a valid bug?

[Christinadobrzyn]

Yep! I'll reach out to the team and get their thoughts on what we want here. I think this is a valid bug because the "done" button essentially doesn't respond or close the edited chat.

screenRecording-31-1-2024-13-0.mp4

asking here - https://expensify.slack.com/archives/C066HJM2CAZ/p1706732856940989

[lanitochka17]

Issue found when executing PR #34404

[parasharrajat]

So it means it is a new find. @Christinadobrzyn Yes please get this confirmed. I don't suggest making unnecessary changes to the focus flow. More here #33466 (comment)

[melvin-bot]

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

[Christinadobrzyn]

Slack convo with David- we DO want to fix this but it can be a low priority to #vip-vsb.

@parasharrajat can you let me know if any proposals will work for this?

[ZhenjaHorbach]

@Christinadobrzyn

Sorry for the stupid question
But what exactly is the result we expect?)

[Christinadobrzyn]

Ah great question @ZhenjaHorbach!

We would like the 'Done' button on the keyboard to close the edited message and the keyboard should be dismissed. I'll add that to the OP details.

[jeremy-croff]

Ah great question @ZhenjaHorbach!

We would like the 'Done' button on the keyboard to close the edited message and the keyboard should be dismissed. I'll add that to the OP details.

I don't have access to the slacks to read the convo but:

Tieing the 'Keyboard' done to a submit action only for edit is what you're going for?

The main composer doesn't behave this way, and the edit composer did not behave this way either before #33466 (comment) either.

If you tie the keyboard done button to the submit of edit, it will be non standard phone behavior.
It's just used to minimize the keyboard.

Examples: www.google.com, type something. Hit 'done' just dismisses keyboard
www.youtube.com, type something. hit 'done' just dismisses keyboard

[jeremy-croff]

Proposal

Problem

The 'Done' button on the message editing page does not submit the edited message, and there is no action defined for it.

Root Cause

The lack of an implementation for the 'Done' button presses causes the message not to be saved.

Solution

We can use the 'onSubmitEditing' function from the React Native text input component to capture the 'Done' button press event and trigger the 'publishDraft' method that saves the message. We can add this new prop to the 'Edit Component' in the following file:

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

Line 409 in 3ab4e6e

<Composer

[parasharrajat]

@jeremy-croff has a good point. I agree with those points. I will try to capture the possible cases of UX and see if the expected behaviour fits them.

[Christinadobrzyn]

Hey @parasharrajat let me know if I can help with anything here - are we still troubleshooting the issues or reviewing proposals? Can you give me an update?

[melvin-bot]

@parasharrajat @Christinadobrzyn 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!

[melvin-bot]

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

[ishpaul777]

I will be taking this over https://expensify.slack.com/archives/C02NK2DQWUX/p1707413964286239

[parasharrajat]

@ishpaul777 will be taking it over as C+ while I am unavailable.

[melvin-bot]

📣 @ishpaul777 🎉 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 📖

[Christinadobrzyn]

Thanks @ishpaul777 can you provide an update when you are caught up? Thanks!

[ishpaul777]

i'll take look at existing proposal tomorrow (9 feb)

[ishpaul777]

Thanks for your proposal @ZhenjaHorbach @jeremy-croff,

@ZhenjaHorbach Sorry but i dont think you solution is a reliable way, to detect the done button press and looks like a workaround.

@jeremy-croff do you mind providing a test video and working branch with the solution i dont find it working for me, maybe i am missing something?

[jeremy-croff]

Thanks for your proposal @ZhenjaHorbach @jeremy-croff,

@ZhenjaHorbach Sorry but i dont think you solution is a reliable way, to detect the done button press and looks like a workaround.

@jeremy-croff do you mind providing a test video and working branch with the solution i dont find it working for me, maybe i am missing something?

I don't mind, but this can be expected at the end of my working day in approx 8 hours. Thanks!

[jeremy-croff]

Thanks for your proposal @ZhenjaHorbach @jeremy-croff,

@ZhenjaHorbach Sorry but i dont think you solution is a reliable way, to detect the done button press and looks like a workaround.

@jeremy-croff do you mind providing a test video and working branch with the solution i dont find it working for me, maybe i am missing something?

@ZhenjaHorbach's analysis so far seems correct.
#35179 (comment)

The top Done button is not firing the onSubmitEdit event, unfortunately it only fires for the Done button as a submit function ( at the bottom of the keyboard ). And after additional digging, I also have no callback identified to listen to the top of the keyboard's done button.

Alternatives I found to control the keyboard accessories like top 'Done' button: https://wix.github.io/react-native-ui-lib/docs/components/infra/KeyboardAccessoryView
But this is getting crazy because it's going against default IOS behavior. the done button is 100% intended to just hide keyboard.

[ishpaul777]

But this is getting crazy because it's going against default IOS behavior. the done button is 100% intended to just hide keyboard.

somthing which makes sense to me. 👍

with my research i also found same. there are quite a few S/o about this but no good solutions we can rely on.

https://stackoverflow.com/questions/76913389/detecting-done-button-on-ios-safari-number-keyboard#:~:text=The%20'done'%20button%20is%20actually,to%20detect%20it%20like%20that.

https://stackoverflow.com/questions/4971061/capture-done-button-click-in-iphones-virtual-keyboard-with-javascript/75842595#75842595

i'd say this is not a bug but IOS default behaviour. @Christinadobrzyn can you please reconfirm if this really a bug? Should it be worth it invest in a workaround with risk for unidentified regressions ?

[Christinadobrzyn]

Ah okay, thanks for testing that @ishpaul777 - I agree this appears to be a default behaviour of iOS Safari (when testing again vs other platforms). I think we should close this and do nothing for the sake of not creating unidentified regressions. Closing but feel free to let me know if you feel otherwise!