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] Expense - Submit button is missing after switching layout from mobile to web #52883

Closed
2 of 8 tasks
izarutskaya opened this issue Nov 21, 2024 · 13 comments
Closed
2 of 8 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Reviewing Has a PR in review Weekly KSv2

Comments

@izarutskaya
Copy link

izarutskaya commented Nov 21, 2024

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: 9.0.65-1
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): [email protected]
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to DM.
  3. Click + > Submit expense > Manual.
  4. Proceed to confirmation page.
  5. Note that Submit button is present.
  6. Open developer tools and switch to mobile layout.
  7. Note that Submit button is present.
  8. Return to web layout.

Expected Result:

Submit button will be shown.

Actual Result:

Submit button is missing on confirmation page after switching layout from mobile to web.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6671472_1732169889618.20241121_141701.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021859734598640577777
  • Upwork Job ID: 1859734598640577777
  • Last Price Increase: 2024-11-21
Issue OwnerCurrent Issue Owner: @s77rt
@izarutskaya izarutskaya added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 21, 2024
Copy link

melvin-bot bot commented Nov 21, 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.

@cretadn22
Copy link
Contributor

Proposal

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

Submit button is missing after switching layout from mobile to web

What is the root cause of that problem?

This bug occurs because isWindowHeightReducedByKeyboard is set to true and it is preventing the button from rendering

if (isReadOnly || isKeyboardShown || isWindowHeightReducedByKeyboard) {

When switching the layout from web to mobile (reducing the window height), we toggle isWindowHeightReducedByKeyboard to true

if (!isWindowHeightReducedByKeyboard && windowHeight < prevWindowHeight - 100) {
toggleKeyboardOnSmallScreens(true);

However, when switching the layout from mobile to web, we do not reset isWindowHeightReducedByKeyboard to false

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

We should only set isWindowHeightReducedByKeyboard to true if a keyboard is available on the device.

if (!isWindowHeightReducedByKeyboard && windowHeight < prevWindowHeight - 100) {

We should include canUseTouchScreen condition.

const canUseTouchScreen = DeviceCapabilities.canUseTouchScreen();


if (!isWindowHeightReducedByKeyboard && windowHeight < prevWindowHeight - 100 && canUseTouchScreen) { 

@huult
Copy link
Contributor

huult commented Nov 21, 2024

Proposal

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

Submit button is missing after switching layout from mobile to web

What is the root cause of that problem?

The submit button is missing because the footer content is not rendered due to a condition that causes an early return, preventing the submit button from being rendered. This happens because isWindowHeightReducedByKeyboard returns true.

if (isReadOnly || isKeyboardShown || isWindowHeightReducedByKeyboard) {
return;
}

When we open the developer tools and switch to the mobile layout, isWindowHeightReducedByKeyboard returns true.

toggleKeyboardOnSmallScreens(true);

After switching to the desktop layout, isWindowHeightReducedByKeyboard still returns true because it is unable to update. Inside toggleKeyboardOnSmallScreens, we check shouldUseNarrowLayout and return early. On desktop, shouldUseNarrowLayout returns false because it is neither a small screen and not isNarrowPaneModal. As a result, isWindowHeightReducedByKeyboard does not update its value and remains true, causing the footer content to return early and the submit button to be missing.

const toggleKeyboardOnSmallScreens = useCallback(
(isKBOpen: boolean) => {
if (!shouldUseNarrowLayout) {
return;
}
setIsWindowHeightReducedByKeyboard(isKBOpen);
},
[shouldUseNarrowLayout],
);

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

To resolve this issue, we should reset isWindowHeightReducedByKeyboard to false when resizing to desktop. Something like this

const toggleKeyboardOnSmallScreens = useCallback(
(isKBOpen: boolean) => {
if (!shouldUseNarrowLayout) {
return;
}
setIsWindowHeightReducedByKeyboard(isKBOpen);
},
[shouldUseNarrowLayout],
);

to

    const toggleKeyboardOnSmallScreens = useCallback(
        (isKBOpen: boolean) => {
            if (!shouldUseNarrowLayout) {
+                if (!isSmallScreenWidth && isWindowHeightReducedByKeyboard) {
+                    setIsWindowHeightReducedByKeyboard(false);
+                }

                return;
            }
            setIsWindowHeightReducedByKeyboard(isKBOpen);
        },
        [shouldUseNarrowLayout],
    );
POC

What alternative solutions did you explore? (Optional)

We can update the condition in the footer content to return when isWindowHeightReducedByKeyboard is true and the screen is small. This ensures it will not execute on desktop. Something like this:

if (isReadOnly || isKeyboardShown || isWindowHeightReducedByKeyboard) {
return;
}

to

-   if (isReadOnly || isKeyboardShown || isWindowHeightReducedByKeyboard) {
+   if (isReadOnly || isKeyboardShown || (isSmallScreenWidth && isWindowHeightReducedByKeyboard)) {
        return;
    }

@greg-schroeder greg-schroeder added the External Added to denote the issue can be worked on by a contributor label Nov 21, 2024
@melvin-bot melvin-bot bot changed the title Expense - Submit button is missing after switching layout from mobile to web [$250] Expense - Submit button is missing after switching layout from mobile to web Nov 21, 2024
Copy link

melvin-bot bot commented Nov 21, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 21, 2024
Copy link

melvin-bot bot commented Nov 21, 2024

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

@s77rt
Copy link
Contributor

s77rt commented Nov 22, 2024

This is a regression from #51318 and I think it's still within the regression period. It's also causing other bugs e.g. #52828. It may be best to revert the PR

cc @truph01 @eh2077 @thienlnam

@thienlnam thienlnam added the DeployBlockerCash This issue or pull request should block deployment label Nov 22, 2024
Copy link

melvin-bot bot commented Nov 22, 2024

Triggered auto assignment to @aldo-expensify (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

Copy link

melvin-bot bot commented Nov 22, 2024

💬 A slack conversation has been started in #expensify-open-source

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Nov 22, 2024
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@thienlnam
Copy link
Contributor

It says this is also reproducible on production, so wouldn't it be a different issue?

@thienlnam
Copy link
Contributor

Ah wait nvm, that PR has been on prod for a while

@thienlnam thienlnam added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Nov 22, 2024
@melvin-bot melvin-bot bot added Reviewing Has a PR in review and removed Daily KSv2 labels Nov 22, 2024
@melvin-bot melvin-bot bot added the Weekly KSv2 label Nov 22, 2024
@aldo-expensify
Copy link
Contributor

@thienlnam I'll assign you here since you are investigating and have a fix. 🙇

@s77rt
Copy link
Contributor

s77rt commented Nov 24, 2024

Revert PR is merged. Let's close this one

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. Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants