Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$250] mWeb/Safari - Expense - Submit button hangs in the middle of expense details page for 2 sec #50663

Closed
1 of 6 tasks
IuliiaHerets opened this issue Oct 11, 2024 · 47 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Oct 11, 2024

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: v9.0.48-0
Reproducible in staging?: YY
Reproducible in production?:
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5075760
Issue reported by: Applause Internal Team

Action Performed:

  1. Open staging.new.expensify.com
  2. Tap FAB and select Submit expense.
  3. Select Manual option.
  4. Enter the amount, tap Next.
  5. Select Workspace name.
  6. Tap the Merchant field, enter merchant name.
  7. Tap Save.
  8. Observe the expense details page.

Expected Result:

User returned back to expense details page, the page is displayed correctly.

Actual Result:

Submit button hangs in the middle of expense details page for 2 sec after entering and saving Merchant.

Workaround:

Unknown

Platforms:

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6632006_1728663459480.Submit_button.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021845948123449591250
  • Upwork Job ID: 1845948123449591250
  • Last Price Increase: 2024-10-21
  • Automatic offers:
    • truph01 | Contributor | 104543192
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 11, 2024
Copy link

melvin-bot bot commented Oct 11, 2024

Triggered auto assignment to @greg-schroeder (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.

@IuliiaHerets
Copy link
Author

@greg-schroeder 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

@truph01
Copy link
Contributor

truph01 commented Oct 12, 2024

Edited by proposal-police: This proposal was edited at 2024-10-12 01:50:22 UTC.

Proposal

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

  • Submit button hangs in the middle of expense details page for 2 sec after entering and saving Merchant.

What is the root cause of that problem?

  • When the keyboard is opened and you navigate back to the confirmation screen, the keyboard remains visible, causing the confirmation view to display in the remaining space (full screen height minus the keyboard height).

  • Once the keyboard is hidden, the container height doesn't update immediately, resulting in the submit button floating in the middle for a brief moment.

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

  • We should hide the button if the keyboard is still shown. To do it, add:
const [isKeyboardOpen, setIsKeyboardOpen] = useState(false);
    const {windowHeight} = useWindowDimensions();
    const prevWindowHeight = usePrevious(windowHeight);
    const {shouldUseNarrowLayout} = useResponsiveLayout();
    const toggleKeyboardOnSmallScreens = useCallback(
        (isKBOpen: boolean) => {
            if (!shouldUseNarrowLayout) {
                return;
            }
            setIsKeyboardOpen(isKBOpen);
        },
        [shouldUseNarrowLayout],
    );
    useEffect(() => {
        if (!isKeyboardOpen && windowHeight < prevWindowHeight - 100) {
            toggleKeyboardOnSmallScreens(true);
        } else if (isKeyboardOpen && windowHeight > prevWindowHeight) {
            toggleKeyboardOnSmallScreens(false);
        }
    }, [isKeyboardOpen, prevWindowHeight, toggleKeyboardOnSmallScreens, windowHeight]);

and early return in this if isKeyboardOpen is true.

  • Optinal: We can display empty screen/loading screen in money request confirmation page if the isKeyboardOpen is true

What alternative solutions did you explore? (Optional)

  • Beside using isKeyboardOpen state as above, me can use hook const {keyboardHeight: isKeyboardOpen} = useKeyboardState().

@truph01
Copy link
Contributor

truph01 commented Oct 12, 2024

Proposal updated

@melvin-bot melvin-bot bot added the Overdue label Oct 14, 2024
Copy link

melvin-bot bot commented Oct 14, 2024

@greg-schroeder Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@greg-schroeder greg-schroeder added the External Added to denote the issue can be worked on by a contributor label Oct 14, 2024
@melvin-bot melvin-bot bot changed the title mWeb/Safari - Expense - Submit button hangs in the middle of expense details page for 2 sec [$250] mWeb/Safari - Expense - Submit button hangs in the middle of expense details page for 2 sec Oct 14, 2024
Copy link

melvin-bot bot commented Oct 14, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 14, 2024
@greg-schroeder
Copy link
Contributor

Sending through to External

Copy link

melvin-bot bot commented Oct 14, 2024

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

@eh2077
Copy link
Contributor

eh2077 commented Oct 18, 2024

@truph01 Thanks for your proposal!

Is it possible to dismiss the keyboard before returning / navigating to the submit page?

@truph01
Copy link
Contributor

truph01 commented Oct 18, 2024

Is it possible to dismiss the keyboard before returning / navigating to the submit page?

We can implement it, but it makes the app feel very slow since, after the user clicks the Save button in step 7, the screen doesn't close immediately

@eh2077
Copy link
Contributor

eh2077 commented Oct 20, 2024

@truph01 As this issue only happen on mobile Safari, I have concerns about your solution to introduce several states that will be applied to other platforms as well. Would you like to explore a simpler solution?

@truph01

This comment was marked as outdated.

@truph01
Copy link
Contributor

truph01 commented Oct 21, 2024

@eh2077 I saw that we already have the same solution to address the same issue in here:

useEffect(() => {
// Use window height changes to toggle the keyboard. To maintain keyboard state
// on all platforms we also use focus/blur events. So we need to make sure here
// that we avoid redundant keyboard toggling.
// Minus 100px is needed to make sure that when the internet connection is
// disabled in android chrome and a small 'No internet connection' text box appears,
// we do not take it as a sign to open the keyboard
if (!isKeyboardOpen && windowHeight < prevWindowHeight - 100) {
toggleKeyboardOnSmallScreens(true);
} else if (isKeyboardOpen && windowHeight > prevWindowHeight) {
toggleKeyboardOnSmallScreens(false);
}
}, [isKeyboardOpen, prevWindowHeight, toggleKeyboardOnSmallScreens, windowHeight]);

The above useEffect is used to determine if the keyboard is currently visible. It checks the keyboard status by monitoring the window height, as the keyboardDidShow event does not function properly on the web.

Copy link

melvin-bot bot commented Oct 21, 2024

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

@eh2077
Copy link
Contributor

eh2077 commented Oct 22, 2024

@truph01 Thanks for your comment. I tested in iOS native App and also found it has same issue but less noticeable than mobile Safari.

I think we can go with @truph01 's proposal - adding an extra state of keyboard opening status to decide whether to hide the submit button or not. Maybe we should consider creating a new hook for this use case. We can discuss more in the PR.

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Oct 22, 2024

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

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 22, 2024
@truph01
Copy link
Contributor

truph01 commented Dec 3, 2024

@eh2077 I found the solution for the native platform, that is we will make sure the keyboard is dismissed successfully before navigating to another screen. We can do it by using KeyboardUtils.dismiss().then(() => // navigate logic). But in web, I cannot find a solution because we don't have keyboardDidHide in this platform.

@greg-schroeder
Copy link
Contributor

@thienlnam what do you think about the above? Do you have any thoughts on how we could proceed here?

@truph01
Copy link
Contributor

truph01 commented Dec 4, 2024

We can hold this issue until issue 53182 is fixed. I saw the PR to fix that issue is promising.

@melvin-bot melvin-bot bot added the Overdue label Dec 4, 2024
@eh2077
Copy link
Contributor

eh2077 commented Dec 5, 2024

We can hold this issue until issue 53182 is fixed. I saw the PR to fix that issue is promising.

@truph01 Thank you for linking to that related issue.

@greg-schroeder I think we can put this issue on hold for the other issue #53182

@melvin-bot melvin-bot bot removed the Overdue label Dec 5, 2024
@greg-schroeder
Copy link
Contributor

Updating to on hold

@greg-schroeder greg-schroeder changed the title [REVERTED] [$250] mWeb/Safari - Expense - Submit button hangs in the middle of expense details page for 2 sec [HOLD for #53182] [$250] mWeb/Safari - Expense - Submit button hangs in the middle of expense details page for 2 sec Dec 6, 2024
@greg-schroeder
Copy link
Contributor

Held for #53182

@greg-schroeder
Copy link
Contributor

Same as above

@greg-schroeder
Copy link
Contributor

Same as above

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Dec 17, 2024
@melvin-bot melvin-bot bot added the Overdue label Dec 26, 2024
@eh2077
Copy link
Contributor

eh2077 commented Jan 3, 2025

Held for #53182

Same

@melvin-bot melvin-bot bot removed the Overdue label Jan 3, 2025
@melvin-bot melvin-bot bot added the Overdue label Jan 13, 2025
@greg-schroeder
Copy link
Contributor

greg-schroeder commented Jan 13, 2025

Hold removed as 53182 seems to be complete

@melvin-bot melvin-bot bot removed the Overdue label Jan 13, 2025
@greg-schroeder greg-schroeder changed the title [HOLD for #53182] [$250] mWeb/Safari - Expense - Submit button hangs in the middle of expense details page for 2 sec [$250] mWeb/Safari - Expense - Submit button hangs in the middle of expense details page for 2 sec Jan 13, 2025
@greg-schroeder
Copy link
Contributor

@eh2077 are we good to proceed here?

@eh2077
Copy link
Contributor

eh2077 commented Jan 13, 2025

@greg-schroeder The issue has been fixed! I think we can close it.

Screen.Recording.2025-01-13.at.9.47.42.PM.mov

@eh2077
Copy link
Contributor

eh2077 commented Jan 13, 2025

Payment summary:

Contributor: @truph01 - $250 - Upwork C+: @eh2077 - $250 - Paid via NewDot manual request

Next up @eh2077 can you complete the checklist? Thank you!

Just realized that we have worked on a PR which was reverted later. During the discussion to fix the regression, @truph01 suggested to hold this for another issue #50663 (comment). And now the issue has been fixed by the other one.

@greg-schroeder In this case, are we still eligible for 50% of the bounty?

@huult
Copy link
Contributor

huult commented Jan 14, 2025

@greg-schroeder This issue has been fixed by my pull request. Can I receive payment for this?

@greg-schroeder
Copy link
Contributor

Hmm. Let me review what happened here and I'll explain the payment situation

@greg-schroeder
Copy link
Contributor

Timeline

  • 2024-10-23: PR created by @truph01
  • 2024-11-15: PR deployed to prod
  • 2024-11-22: PR is reverted by @thienlnam
  • 2024-12-04: PR created by @huult
  • 2024-12-06: This issue is put on hold for the PR above
  • 2024-12-31: Final version of this PR deployed to prod

Payments

@greg-schroeder
Copy link
Contributor

Payment summary:

Contributor: @truph01 - $125 - Upwork
C+: @eh2077 - $125 - NewDot Manual Request

@github-project-automation github-project-automation bot moved this from Hold for Payment to Done in [#whatsnext] #expense Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
Status: Done
Development

No branches or pull requests

7 participants