[HOLD for payment 2024-06-28] [$250] mWeb - Attachment - Invisible send button above keyboard when PDF password focused

[lanitochka17]

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.76-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: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Log in to New Expensify
2. Navigate to any chat
3. Tap on the "+" button
4. Tap on Add attachment and select password protected PDF
5. Tap on "enter the password"
6. Tap on the space above keyboard

Expected Result:

Send button should remain at the bottom as it is in native apps. The password field should lose focus, and keyboard should close

Actual Result:

There is invisible Send button above keyboard, so attachments gets sent if you tap space above keyboard

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

Bug6493229_1716835290373.Record_2024-05-27-18-46-51.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014e3e280007080993
• Upwork Job ID: 1795469304244633600
• Last Price Increase: 2024-05-28
• Automatic offers:
• tienifr | Contributor | 102690986

Issue Owner

Current Issue Owner: @garrettmknight

[melvin-bot]

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

@garrettmknight 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

[Krishna2323]

Proposal

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

mWeb - Attachment - Invisible send button above keyboard when PDF password focused

What is the root cause of that problem?

When the input is blurred updateConfirmButtonVisibility runs before onPress of button and updates isConfirmButtonDisabled to false.

App/src/components/AttachmentModal.tsx

Lines 359 to 371 in 563f8c7

	const updateConfirmButtonVisibility = useCallback(
	(shouldFadeOut: boolean) => {
	setIsConfirmButtonDisabled(shouldFadeOut);
	const toValue = shouldFadeOut ? 0 : 1;
	Animated.timing(confirmButtonFadeAnimation, {
	toValue,
	duration: 100,
	useNativeDriver,
	}).start();
	},
	[confirmButtonFadeAnimation],
	);

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

We can use InteractionManager.runAfterInteractions to update the isConfirmButtonDisabled state.

Optional: We can only use InteractionManager.runAfterInteractions/timeout when shouldFadeOut is true.

What alternative solutions did you explore? (Optional)

We can use timeout instead of InteractionManager.runAfterInteractions. By using timeout with 0ms or any small value we will make sure that the onPress event is called before the state update.

setTimeout(() => {
    setIsConfirmButtonDisabled(shouldFadeOut);
}, 0);

[tienifr]

Proposal

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

There is invisible Send button above keyboard, so attachments gets sent if you tap space above keyboard

What is the root cause of that problem?

• When we are focusing on the input and the keyboard is opening. And then click on the space above the keyboard, it means we click on the "Send" button but it is disabled and has opacity: 0, it will blur the input on onMouseDown , and then updateConfirmButtonVisibility in here is called. It will call setIsConfirmButtonDisabled(false). Now, the "Send" button is enabled, so the submitAndClose function is called.

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

• We can use fullDisabled={isConfirmButtonDisabled} in here. fullDisabled can be passed down to PressableWithFeedback.
• Or we should use onMouseDown={(e)=>e.preventDefault()} in here

What alternative solutions did you explore? (Optional)

• In mobile web, we are using 2 ways to set the keyboard state, one based on onFocus/onBlur input in here and one based on the window height in here. And because the onFocus/onBlur event is triggered before the window height change, so leads to the behavior below (The "Send" button is displayed when the keyboard is not fully hidden):

Screen.Recording.2024-06-08.at.09.15.21.mov

• To fix it, we can just remove this line.

• And, because previously, the input password component's position depends on the window height, so we need to keep the "Send" button when the keyboard is displayed or not to make sure there is no "Jump" effect. But now, the input password is always at the top, so there is no jump effect. Based on it, we can update this to:

    const updateConfirmButtonVisibility = useCallback(
        (shouldHide) => {
            setIsConfirmButtonDisabled(shouldHide);
        },
        [],
    );

and this to:

                    {!!onConfirm && !isConfirmButtonDisabled && (

[Krishna2323]

Proposal updated

• Minor typo fix in RCA

[tienifr]

Updated alternative solution

[bernhardoj]

Proposal

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

The send button is invisible above keyboard and still pressable.

What is the root cause of that problem?

When a keyboard is toggled, we set the opacity of the button to 0 to make it invisible and disable it.

App/src/components/AttachmentModal.tsx

Lines 359 to 365 in 2514f29

	const updateConfirmButtonVisibility = useCallback(
	(shouldFadeOut: boolean) => {
	setIsConfirmButtonDisabled(shouldFadeOut);
	const toValue = shouldFadeOut ? 0 : 1;
	Animated.timing(confirmButtonFadeAnimation, {
	toValue,

If we disable our custom pressable, it will just pass an empty handler to the component.

So, the pressable is still responding to click/touch. The pressable will fire onPress if we touch down and release the pressable. When we touch the pressable, it will close the keyboard, receive the touch down event, and reenable the pressable. When we release our finger from the pressable, the pressable then fires the onPress.

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

We can use the fullDisabled property to make the pressable unable to respond to any event when disabled, but I think instead of making it invisible (opacity: 0), it would be easier to just not render the button at all.

{!!onConfirm && !isConfirmButtonDisabled && ...}

FYI, the decision to hide the button is from this discussion

What alternative solutions did you explore? (Optional)

If we want to show both button, then we just need to remove the opacity logic and disable.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~014e3e280007080993

[melvin-bot]

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

[melvin-bot]

@garrettmknight, @mananjadhav Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mananjadhav]

I'll review the proposals on Monday.

[mananjadhav]

While all the proposals do resolve the problem, I think it makes sense to not render the button at all. I am going to let the assigned engineer also confirm the behavior.

@dangrous Can you confirm?

Also tagging @Expensify/design. Do we need a button to stay with opacity: 0 or we not render the button at all?

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[shawnborton]

Not rendering it at all works for me, though I'm not sure if that has any repercussions with animation/etc.

[tienifr]

@shawnborton Here is why we want to apply the fade solution instead of hiding the button.

[shawnborton]

Got it, I thought there was a reason. Can we just add user-select: none to it while we have it hidden?

[tienifr]

@shawnborton Yeah, we can use fullDisabled as mentioned in here, so the button still takes a space but click on it does not trigger any event

[melvin-bot]

@garrettmknight @mananjadhav @dangrous 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!

[dangrous]

@tienifr's alternate seems pretty simple and cleans up the existing code a bit. Let's move forward with that!

[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 📖

[tienifr]

@mananjadhav PR #43544 is ready

[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 9.0.0-9 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix Invisible send button above keyboard #43544

If no regressions arise, payment will be issued on 2024-06-28. 🎊

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

• @mananjadhav requires payment through NewDot Manual Requests
• @tienifr requires payment automatic offer (Contributor)

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

• [@mananjadhav] The PR that introduced the bug has been identified. Link to the PR:
• [@mananjadhav] 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:
• [@mananjadhav] 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:
• [@mananjadhav] Determine if we should create a regression test for this bug.
• [@mananjadhav] 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.
• [@garrettmknight] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[mananjadhav]

I think the issue existed since a long time. I couldn't trace a specific offending PR for this one. I also don't think we need a regression test for this one.

@garrettmknight This is ready for payout today.

[garrettmknight]

Payment Summary:

• Contributor: @tienifr $250 paid via upwork
• Reviewer: @mananjadhav $250

[JmillsExpensify]

$250 approved for @mananjadhav

[tienifr]

I'm actually paid via NewDot now (eligible from 2024-04-06). I've refunded the Upwork payment and requested again on NewDot

[JmillsExpensify]

$250 approved for @tienifr