[HOLD for payment 2023-09-06] [$1000] Pasting on Web isn't automatically focusing on composer

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Open a chat in newDot Web/Desktop
2. Remove focus from composer
3. Copy some text into your clipboard (CMD+C/CTRL+C)
4. With the chat open and the composer unfocused, paste the text you have in your clipboard (CMD+V/CTRL+V)

Expected Result:

The composer should be focused and the text should be pasted into it.

Actual Result:

Nothing happens

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 / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.55-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
Notes/Photos/Videos: Any additional supporting documentation

Recording.5943.mp4

Expensify/Expensify Issue URL:
Issue reported by: @PauloGasparSv
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1691623314942259

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ed32306975a2bc2a
• Upwork Job ID: 1693708827153092608
• Last Price Increase: 2023-08-21
• Automatic offers:
• s77rt | Reviewer | 26293575
• joh42 | Contributor | 26293576

[melvin-bot]

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

[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

[rayane-djouah]

Proposal

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

When a user tries to paste content into the chat composer without it being focused, the expected behavior is that the composer should automatically gain focus and the content should be pasted. However, currently, nothing happens.

What is the root cause of that problem?

There isn't any global event listener that listens for the paste event (CMD+V/CTRL+V) when the composer is unfocused. This means that when the composer is not in focus and the user tries to paste content, there's no mechanism in place to detect this action and subsequently focus the composer and paste the content.

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

To address this issue, we need to add a global event listener that listens for the paste event, focuses the composer when it's triggered, and pastes the content into the composer. Here's a step-by-step solution using React hooks:

• Use the useEffect hook to add a global event listener for the paste event.
• When the paste event is detected, check if the composer is focused.
• If the composer is not focused, set the focus to the composer.
• Paste the content into the composer.

Here's a code snippet that demonstrates the proposed changes in ReportActionCompose.js file:

useEffect(() => {
    const handlePaste = (e) => {
        // Check if the composer is focused
        if (textInputRef.current && !textInputRef.current.isFocused()) {
            // Focus the composer
            textInputRef.current.focus();

            // Allow some time for the focus event to complete before pasting the content
            setTimeout(() => {
                // Trigger a paste event on the composer
                if (textInputRef.current) {
                    textInputRef.current.value = e.clipboardData.getData('text');
                }
            }, 0);
        }
    };

    // Add global event listener for paste event
    document.addEventListener('paste', handlePaste);

    // Cleanup: Remove the global event listener when the component is unmounted
    return () => {
        document.removeEventListener('paste', handlePaste);
    };
}, []);

Result:

2023-08-20.19-49-34.mp4

What alternative solutions did you explore? (Optional)

[tienifr]

Proposal

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

Pasting on Web isn't automatically focusing on composer

What is the root cause of that problem?

we already have the logic to handle paste event, but we are preventing paste if the target is not textInput

In

App/src/components/Composer/index.js

Lines 275 to 277 in 03cf0b1

	if (textInput.current !== event.target) {
	return;
	}

That is not correct, because when the input is blurred, then users paste text, the event.target should not be the input. That why paste functionality does not work correctly

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

we can remove these lines

App/src/components/Composer/index.js

Lines 275 to 277 in 03cf0b1

	if (textInput.current !== event.target) {
	return;
	}

Then focus into the compose before executing insertText

Change

App/src/components/Composer/index.js

Lines 224 to 225 in 03cf0b1

	const paste = useCallback((text) => {
	try {

to

 const paste = useCallback((text) => {
        try {
            textInput.current.focus();
            document.execCommand('insertText', false, text);

Result

Screen.Recording.2023-08-21.at.14.59.14.mov

[getusha]

Proposal

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

Pasting text using Ctrl + v, will not focus the composer

What is the root cause of that problem?

Right now, we're handling key presses to make the composer focused and input the key. However, we're not yet managing the onKeyPress function to both focus the composer and input text when using the shortcut Ctrl + v/CMD + v.

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

We should focus the input if the key is KeyV with metaKey or ctrlKey pressed.
in here:

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

Lines 459 to 465 in 03cf0b1

	const focusComposerOnKeyPress = useCallback(
	(e) => {
	const isComposerVisible = checkComposerVisibility();
	if (!isComposerVisible) {
	return;
	}

\We'll include a check with a condition: e.metaKey && e.nativeEvent.code === 'KeyV'. This check will adapt based on the platform. For Windows, we'll account for e.ctrlKey being equivalent to the metaKey. Additionally, we need to avoid focusing on the main composer when editing a report action. To ensure this, we'll add !['INPUT', 'TEXTAREA'].includes(e.target.nodeName). additionally handle edge cases if any inside the condition.

        if(e.metaKey && e.nativeEvent.code === 'KeyV' && !['INPUT', 'TEXTAREA'].includes(e.target.nodeName)) {
            focus();
            return;
        }

Screen.Recording.2023-08-21.at.10.55.55.AM.mov

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[melvin-bot]

Current assignee @NicMendonca is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[b4s36t4]

I think this intended, we've stopped it. #25409 (comment)

[NicMendonca]

@b4s36t4 the feature was suppressed in this PR: #23525

[b4s36t4]

@NicMendonca So do we want to enable it now?

[s77rt]

@rayane-djouah Thanks for the proposal. Your RCA is not exactly right, we do have a global listener for paste events even if the composer is not focused (as long as its screen is focused). However we just return early and do nothing in this case..

Please note that the use of setTimeout is discouraged as it's mostly used as a workaround. I think your use case is a workaround here as we are waiting for an undefined amount of time - We are just pushing the focus call to the stack.

[s77rt]

@tienifr Thanks for the proposal. Your RCA is correct. However your proposed solution is outdated. Please update it to reflect the current main - We should also support pasting files when pressing CTRL/CMD+V.

[s77rt]

@getusha Thanks for the proposal. Your RCA makes sense. However the solution does not seem to be working. I think the proposal is outdated. Please update it and tag me again..

[joh42]

Proposal

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

When the composer isn't focused, paste events are not handled by the composer. This means that no text is pasted into the composer/the composer is not focused.

What is the root cause of that problem?

There is an early return that stops paste event handling if the composer was not the target of the paste event.

App/src/components/Composer/index.js

Lines 275 to 277 in 8f772ea

	if (textInput.current !== event.target) {
	return;
	}

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

I think we should modify the aforementioned early return:

App/src/components/Composer/index.js

Lines 275 to 277 in 8f772ea

	if (textInput.current !== event.target) {
	return;
	}

We only need to return if another text input was the target element of the paste. Otherwise we can focus the composer, and let the regular paste handling proceed:

if (textInput.current !== event.target) {
    const targetIsInput = ['INPUT', 'TEXTAREA'].includes(event.target && event.target.nodeName);
    if (targetIsInput) {
        return;
    }

    if (textInput.current) {
        textInput.current.focus();
    }
}

This works for both pasting via the context menu:

Screen.Recording.2023-08-22.at.02.24.59.mov

And via keyboard shortcuts:

Screen.Recording.2023-08-22.at.02.25.59.mov

Crucially this approach won't reintroduce other pasting bugs:

720Screen.Recording.2023-08-22.at.11.14.50.mov

For example it will not reintroduce the bug of not being able to paste into the emoji picker:

Screen.Recording.2023-08-22.at.11.26.24.mov

Edit: we might also want to add a prop to the composer, called something like isMainComposer. If we are catching the paste event in any composer other than the main composer, we want to return early, so the early return would look something like:

if (textInput.current !== event.target) {
    const targetIsInput = ['INPUT', 'TEXTAREA'].includes(event.target && event.target.nodeName);
    if (targetIsInput || !props.isMainComposer) {
        return;
    }

    if (textInput.current) {
        textInput.current.focus();
    }
}

What alternative solutions did you explore? (Optional)

[b4s36t4]

Proposal

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

Pasting on Web isn't automatically focusing on composer

What is the root cause of that problem?

We're manually blocking the paste events if the input is focused. Also we're blocking the any meta related events to trigger the focus here

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

Line 472 in d09faa6

if (e.metaKey || e.ctrlKey || e.altKey) {

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

Here we're checking for

App/src/components/Composer/index.js

Line 268 in d09faa6

const isVisible = checkComposerVisibility();

for the composer focus.

We can update the function to focus if it the event is paste event which we can trigger by using the event from variable from here

App/src/components/Composer/index.js

Lines 266 to 268 in d09faa6

	const handlePaste = useCallback(
	(event) => {
	const isVisible = checkComposerVisibility();

and pass that event to the function here

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

Line 454 in d09faa6

const checkComposerVisibility = useCallback(() => {

.

Changes

const isVisible = checkComposerVisibility(event);

// Focus the composer if it is not focused or it's not coming from paste event (e can be empty) && check for the paste event details
if (!textInputRef.current.isFocused() && !_.isEmpty(e) && (e.metaKey || e.ctrlKey) && e.key === 'v') {
    focus();
    return true;
}

Also we should update the dependencies of checkComposerVisibility function.

This won't break any previous issues like

• Email pasted twice
• Pasting in edit composer get's pasted into main composer
• Able to copy paste a file.

What alternative solutions did you explore? (Optional)

NA

Result

Kapture.2023-08-22.at.10.41.01.mp4

[getusha]

@s77rt since the component was updated to function component, i updated the code snippet accordingly.

Proposal

Updated

[tienifr]

@s77rt Sorry if I miss something, Why is my proposal outdated?

We should also support pasting files when pressing CTRL/CMD+V.

It works well in this case too.

[joh42]

Updated my proposal by adding two videos showing that it will not break existing paste functionality

[s77rt]

@joh42 Thanks for the proposal. Your RCA makes sense and the solution too. However I feel that we may be missing something. In #23135 we had the early return check and it was working. It seems that something else has been changed that broke this functionality. Any idea on that?

[s77rt]

@b4s36t4 Thanks for the proposal. I think it's pretty much the same as @getusha's proposal.

[s77rt]

@getusha Thanks for the update. Unfortunately the solution does not seem to be working for me. The composer gets focused but the text is not pasted. Can you please double check? (and test on main)

Kooha-2023-08-22-22-30-35.webm

[s77rt]

@tienifr Sorry, my bad here. Your proposal is not outdated, I just misread the function callback (paste vs handlePaste). Yet I'm afraid that your solution would introduce a regression, if you type in an edit message input and paste a message it will get pasted on the main composer instead.

Kooha-2023-08-22-22-32-41.webm

[melvin-bot]

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

[melvin-bot]

📣 @PauloGasparSv! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[melvin-bot]

The BZ member will need to manually hire PauloGasparSv for the Reporter role. Please store your Upwork details and apply to our Upwork job so this process is automatic in the future!

[joh42]

Thanks! You can expect a PR later today.

[joh42]

The PR is now ready for review @s77rt

[melvin-bot]

🎯 ⚡️ Woah @s77rt / @joh42, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @joh42 got assigned: 2023-08-23 21:47:06 Z
• when the PR got merged: 2023-08-25 17:56:32 UTC

On to the next one 🚀

[b4s36t4]

Raised a bug here about the regression this would be causing. https://expensify.slack.com/archives/C049HHMV9SM/p1692987504856949 @s77rt

[s77rt]

@b4s36t4 It does not seem to be a regression from this PR. I can reproduce even with the PR reverted.

Screen.Recording.2023-08-25.at.7.36.11.PM.mov

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.58-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fixed composer not catching paste events when unfocused #25900

If no regressions arise, payment will be issued on 2023-09-06. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

• @s77rt requires payment offer (Reviewer)
• @joh42 requires payment offer (Contributor)
• @PauloGasparSv requires payment

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@s77rt] The PR that introduced the bug has been identified. Link to the PR:
• [@s77rt] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@s77rt] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@s77rt] Determine if we should create a regression test for this bug.
• [@s77rt] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@NicMendonca] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[s77rt]

• The PR that introduced the bug has been identified: [CP Staging] hotfix: avoid early prevent default to copy in normal inputs #23525
• The offending PR has been commented on: [CP Staging] hotfix: avoid early prevent default to copy in normal inputs #23525 (comment)
• A discussion in #expensify-bugs has been started: I don't think this is needed for this case.
• Determine if we should create a regression test for this bug: Yes

---

Regression Test Proposal (Web/Desktop Only)

1. Open App
2. Go to any chat
3. Copy any message
4. Click anywhere outside the composer
5. Verify the composer is not focused
6. Press CMD+V
7. Verify the composer is focused and text is pasted correctly

[NicMendonca]

• Reporter: N/A
• Contributor: @joh42 - $1,500
• Contributor +: @s77rt - $1,500

[NicMendonca]

@joh42 you've been paid! 🎉

[NicMendonca]

@s77rt can you remind me, are you paid via Expensify or Upwork?

[NicMendonca]

Reporter: N/A
Contributor: @joh42 - $1,500
Contributor +: @s77rt - $1,500 (via Upwork)

[NicMendonca]

all set here ✅