[Payment due 1-25][$250] Scan - "Drop file here" modal does not dismiss when file is no longer dragged over the area

[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.20-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:

Action Performed:

1. Go to workspace chat > + > Request money
2. Go to Scan
3. Open a file explorer window next the the drag & drop area
4. Drag a file over the area and remove it from the area repeatedly (and very quickly)

Expected Result:

"Drop file here" modal will dimiss when the file is no longer dragged over the area

Actual Result:

"Drop file here" modal does not dismiss when the file is no longer dragged over the area

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

Bug6329833_1704112789223.bandicam_2023-12-30_11-57-55-618.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01d19df269e522389b
• Upwork Job ID: 1741888378699694080
• Last Price Increase: 2024-01-26
• Automatic offers:
• s77rt | Reviewer | 28077927
• tienifr | Contributor | 28077928

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[samilabud]

Proposal

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

"Drop file here" modal does not dismiss when file is no longer dragged over the area

What is the root cause of that problem?

The drag-and-drop hook is based on a reference counter to do the overlay works well, but when we use a component or window that lives outside the browser we get this bug because the counter increases more than expected.

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

We should modify the drag-and-drop hook to take into consideration scenarios when we are dragging outside the drag-and-drop zone to decrease the reference counter.

The code should be like:

                 case DRAG_OVER_EVENT:
                    handleDragEvent(event);
                    if (isDraggingOver && dragCounter.current > 1) {
                        dragCounter.current--;
                    }
                    break;

Result - after changes:

drag-and-drop.test.after.changes.mp4

[bernhardoj]

Proposal

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

The scan request drop view doesn't disappear after we stop the drag.

What is the root cause of that problem?

This can easily be reproduced when you start dragging your file to the instruction view (that is the Upload receipt text and image).

Screen.Recording.2024-01-02.at.12.18.07.mov

The drop file here view will be shown if isDraggingOver state is true.

App/src/components/DragAndDrop/Consumer/index.tsx

Lines 7 to 18 in fe49e4f

	const {isDraggingOver, setOnDropHandler, dropZoneID} = useContext(DragAndDropContext);
	useEffect(() => {
	setOnDropHandler?.(onDrop);
	}, [onDrop, setOnDropHandler]);
	if (!isDraggingOver) {
	return null;
	}
	return <Portal hostName={dropZoneID}>{children}</Portal>;
	}

The isDraggingOver state itself will be set based on dragCounter value.

App/src/hooks/useDragAndDrop.ts

Lines 84 to 99 in fe49e4f

	case DRAG_ENTER_EVENT:
	dragCounter.current++;
	handleDragEvent(event);
	if (isDraggingOver) {
	return;
	}
	setIsDraggingOver(true);
	break;
	case DRAG_LEAVE_EVENT:
	dragCounter.current--;
	if (!isDraggingOver || dragCounter.current > 0) {
	return;
	}
	setIsDraggingOver(false);
	break;

The problem in this issue is, that the isDraggingOver value is stuck at true even though we already stopped the drag because the dragCounter is not 0.

The reason we use drag counter is explained in the comment here.

App/src/hooks/useDragAndDrop.ts

Lines 33 to 37 in fe49e4f

	// This solution is borrowed from this SO: https://stackoverflow.com/questions/7110353/html5-dragleave-fired-when-hovering-a-child-element
	// This is necessary because dragging over children will cause dragleave to execute on the parent.
	// You can think of this counter as a stack. When a child is hovered over we push an element onto the stack.
	// Then we only process the dragleave event if the count is 0, because it means that the last element (the parent) has been popped off the stack.
	const dragCounter = useRef(0);

In short, when you move from one element to another element, the ENTER (for the new element) and LEAVE (for the current element) events will be called. So, the ENTER and LEAVE events should cancel out each other. However, if the element disappears, the LEAVE event won't be called.

In our case, if we start dragging the file to the instruction view, let's say the "Upload receipt" text,

the ENTER event will be triggered for the "Upload receipt" text. Then, the drop file here view shows AND the instruction view disappears.

App/src/pages/iou/request/step/IOURequestStepScan/index.js

Line 325 in cb4fc64

{!isDraggingOver && (Browser.isMobile() ? mobileCameraView() : desktopUploadView())}

Thus, the LEAVE event for "Upload receipt" text is never fired leaving 1 drag counter behind.

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

The simplest solution is to make it invisible by setting the opacity to 0 when dragging instead of unmounting it from the view.

<View style={[styles.flex1, !Browser.isMobile() && styles.uploadReceiptView(isSmallScreenWidth), isDraggingOver && styles.opacity0]}>
    {(Browser.isMobile() ? mobileCameraView() : desktopUploadView())}
</View>

What alternative solutions did you explore? (Optional)

The other way is:

1. Expose the drag counter from the drag and drop context
2. Create a separate element for the desktop scan view
3. When the desktop scan view element unmounts, decrement the drag counter from the drag and drop context

[tienifr]

Proposal

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

"Drop file here" modal does not dismiss when the file is no longer dragged over the area

What is the root cause of that problem?

We're updating isDraggingOver to false if dragCounter is 0

App/src/hooks/useDragAndDrop.ts

Lines 35 to 36 in cb4fc64

	// You can think of this counter as a stack. When a child is hovered over we push an element onto the stack.
	// Then we only process the dragleave event if the count is 0, because it means that the last element (the parent) has been popped off the stack.

We can think of this counter as a stack, so we will decrease dragCounter when dragLeave event is fired.
Before users drag file, we have 3 components in IOU scan view (one of them is instruction view) -> dragEnter is triggered 3 times -> dragCounter =3
When users start to drag file, the dragging view is present and the instruction view disappears so the dragLeave for instruction view is never triggered -> dragCounter just decease to 1 when users leave the window

-> isDraggingOver is not set to false

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

We should not lean on dragCounter because of the above reason. To solve the problem dragging over children will cause dragleave to execute on the parent. we need to check if the currentTarget contains relatedTarget in dragLeave then we will early return

                case DRAG_LEAVE_EVENT:
                    if (event?.currentTarget?.contains(event.relatedTarget)) {
                        return;
                    }

                    setIsDraggingOver(false);

Ref: https://stackoverflow.com/a/54271161

What alternative solutions did you explore? (Optional)

NA

Result

Screen.Recording.2024-01-02.at.11.45.38.mov

[Christinadobrzyn]

might be related to: #27744

I think this could be part of Wave 6. It seems like this issue only happens if you drag the file back and forth very quickly for multiple attempts - so I think this can be low priority.

I think let's continue to collect proposals until we decide to pause this after the Wave 6 inclusion review

[s77rt]

@samilabud Thanks for the proposal. Unfortunately I don't think your RCA is valid.

[s77rt]

@bernhardoj Thanks for the proposal. Your RCA is about right. However the solution is a workaround since we are not fixing the root cause and it may only fix one particular case (same goes with the alternative solution).

[s77rt]

@tienifr Thanks for the proposal. Your RCA is about right as well (the reason on why the leave event is not being fired may be related to more than the fact that the element is removed from the dom - not a blocker). Your suggested solution makes sense to me. If we are dragging(moving) out of an element to another element that is still within the drop zone then the dragging is effectively not over.

🎀 👀 🎀 C+ reviewed
Link to proposal

[melvin-bot]

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

[tgolen]

I agree with @s77rt and 🟢 the proposal by @tienifr.

[melvin-bot]

📣 @s77rt 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

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

[situchan]

Seems like regression came from this issue

[s77rt]

• The PR that introduced the bug has been identified: Migrate DragNDrop #23589
• The offending PR has been commented on: Migrate DragNDrop #23589 (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: No. This is an edge case and I think the already existing test is enough https://expensify.testrail.io/index.php?/cases/view/2730088

[s77rt]

@Christinadobrzyn Can you please add the Awaiting Payment label here? Looks like Melvin is OOO

[Christinadobrzyn]

@s77rt can you confirm if we're working on this PR - #23589 or if this needs to be paid out now?

[s77rt]

@Christinadobrzyn Ah sorry for not clarifying. This should be paid on 25th. The payment due here should be $250.

(No work is being done on any PR. All is done already)

[melvin-bot]

Upwork job price has been updated to $250

[Christinadobrzyn]

paying this out

Payouts due:

Contributor: $250 @tienifr (paid in Upwork)
Contributor+: $250 @s77rt (paid in Upwork)

Eligible for 50% #urgency bonus? NA

Upwork job is here.

No regression test. Closing this out but let me know if you need anything!