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

[HOLD for payment 2025-01-02] [$250] Android - Workspace - White space below next button & keypad not closed on tapping outside #53910

Closed
2 of 8 tasks
IuliiaHerets opened this issue Dec 11, 2024 · 24 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Dec 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.74-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Yes, reproducible on both
Issue reported by: Applause Internal Team

Action Performed:

Pre- condition: Have control workspace with 2 members invited, enabled workflow and add approval

  1. Launch app in both mweb and android
  2. Go to workspace settings - workflow
  3. Tap add approval workflow
  4. Note the white space between next button and keypad in mweb and android
  5. Tap outside on top
  6. Note keypad behavior on mweb and android

Expected Result:

In add approval workflow page, the white space between next button and keypad must be consistent, similarly on tapping outside keypad closing behavior too must be consistent.

Actual Result:

In add approval workflow page, the white space between next button and keypad in less in mweb and more in android. After tapping outside, keypad closed in mweb and remain opened in android.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6690627_1733897506218.as.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021867598579287232853
  • Upwork Job ID: 1867598579287232853
  • Last Price Increase: 2024-12-13
Issue OwnerCurrent Issue Owner: @Christinadobrzyn
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 11, 2024
Copy link

melvin-bot bot commented Dec 11, 2024

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

@bernhardoj
Copy link
Contributor

bernhardoj commented Dec 11, 2024

Proposal

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

There are extra spaces between the button and keyboard on the workflow page.

What is the root cause of that problem?

On the Expenses From page, we disable the padding bottom from ScreenWrapper.

<ScreenWrapper
includeSafeAreaPaddingBottom={false}
testID={WorkspaceWorkflowsApprovalsExpensesFromPage.displayName}

The padding bottom is added from the SelectionList using useStyledSafeAreaInsets.

const {safeAreaPaddingBottomStyle} = useStyledSafeAreaInsets();
// TODO: test _every_ component that uses SelectionList
return (
<View style={[styles.flex1, (!isKeyboardShown || !!footerContent) && includeSafeAreaPaddingBottom && safeAreaPaddingBottomStyle, containerStyle]}>

Also, the button is wrapped with FixedFooter which has its own padding bottom already.

{showConfirmButton && (
<FixedFooter style={[styles.mtAuto]}>
<Button

This looks fine when the keyboard isn't visible because the padding fills the device navigation bar space, but when the keyboard shows, it looks like too much space. It doesn't happen on another page that uses SelectionList too because useStyledSafeAreaInsets returns 0 padding if it's already applied by the ScreenWrapper.

Why it's not a problem if the padding bottom is added from the ScreenWrapper? It's because the padding bottom is applied to the View that wraps the KeyboardAvoidingView.

<View
fsClass="fs-unmask"
style={[styles.flex1, paddingStyle, style]}
// eslint-disable-next-line react/jsx-props-no-spreading, react-compiler/react-compiler
{...keyboardDismissPanResponder.panHandlers}
>
<KeyboardAvoidingView
style={[styles.w100, styles.h100, {maxHeight}, isAvoidingViewportScroll ? [styles.overflowAuto, styles.overscrollBehaviorContain] : {}]}
behavior={keyboardAvoidingViewBehavior}
enabled={shouldEnableKeyboardAvoidingView}
>

But the padding in SelectionList is added as the children of the KeyboardAvoidingView.

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

Enable the padding bottom from the ScreenWrapper for the ExpensesFrom page.

<ScreenWrapper
includeSafeAreaPaddingBottom={false}
testID={WorkspaceWorkflowsApprovalsExpensesFromPage.displayName}

(we can check for other pages that uses BaseSelectionList too, like in Approver page for example)

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/A

@Christinadobrzyn
Copy link
Contributor

Christinadobrzyn commented Dec 11, 2024

Hum this is a really minor change. I'm not sure if this is something that is very important to update at this time. @bernhardoj do you this is important for any reason?

@Christinadobrzyn
Copy link
Contributor

Just a heads up that I'm going to be ooo Dec 12 - 13th. Back on Monday 16th. I'm not going to assign this to a BZ teammate but if anything is urgent, please reach out to the team for a volunteer.

@bernhardoj
Copy link
Contributor

It's not something critical, but seeing those extra spaces is very unpleasant. (this happens on some pages too)

@Christinadobrzyn Christinadobrzyn added the External Added to denote the issue can be worked on by a contributor label Dec 13, 2024
@melvin-bot melvin-bot bot changed the title Android - Workspace - White space below next button & keypad not closed on tapping outside [$250] Android - Workspace - White space below next button & keypad not closed on tapping outside Dec 13, 2024
Copy link

melvin-bot bot commented Dec 13, 2024

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

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

melvin-bot bot commented Dec 13, 2024

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

@Christinadobrzyn
Copy link
Contributor

Okay, I think if you think this should be resolved, we'll move it forward. Adding external for a review of #53910 (comment).

@Christinadobrzyn Christinadobrzyn removed the External Added to denote the issue can be worked on by a contributor label Dec 13, 2024
@c3024
Copy link
Contributor

c3024 commented Dec 16, 2024

Is this a problem for all pages with ScreenWrapper with includeSafeAreaPaddingBottom false and with BaseSelectionList with search bar enabled as child?

@bernhardoj
Copy link
Contributor

Yes, except if the list doesn't have a footer. For example, it happens on the Approver page too (the Approver search input only shows if there is >8 item)

@c3024
Copy link
Contributor

c3024 commented Dec 17, 2024

@bernhardoj 's proposal here LGTM!

🎀 👀 🎀 C+ Reviewed

Copy link

melvin-bot bot commented Dec 17, 2024

Triggered auto assignment to @cristipaval, 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 Dec 17, 2024
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Dec 17, 2024
@bernhardoj
Copy link
Contributor

PR is ready

cc: @c3024

@muttmuure muttmuure moved this to MEDIUM in [#whatsnext] #quality Dec 23, 2024
@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Dec 26, 2024
@melvin-bot melvin-bot bot changed the title [$250] Android - Workspace - White space below next button & keypad not closed on tapping outside [HOLD for payment 2025-01-02] [$250] Android - Workspace - White space below next button & keypad not closed on tapping outside Dec 26, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Dec 26, 2024
Copy link

melvin-bot bot commented Dec 26, 2024

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

Copy link

melvin-bot bot commented Dec 26, 2024

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

If no regressions arise, payment will be issued on 2025-01-02. 🎊

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

  • @bernhardoj requires payment through NewDot Manual Requests
  • @c3024 requires payment through NewDot Manual Requests

Copy link

melvin-bot bot commented Dec 26, 2024

@c3024 @Christinadobrzyn @c3024 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jan 2, 2025
Copy link

melvin-bot bot commented Jan 2, 2025

Payment Summary

Upwork Job

  • Contributor: @bernhardoj owed $250 via NewDot
  • Reviewer: @c3024 owed $250 via NewDot

BugZero Checklist (@Christinadobrzyn)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1867598579287232853/hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@Christinadobrzyn
Copy link
Contributor

Payment summary here - #53910 (comment)

@c3024 do we need a regression test for this?

@melvin-bot melvin-bot bot added the Overdue label Jan 6, 2025
@Christinadobrzyn
Copy link
Contributor

DMd @c3024 to see if we need a regression test

@c3024
Copy link
Contributor

c3024 commented Jan 7, 2025

BugZero Checklist:

  • [Contributor] Classify the bug:
Bug classification

Source of bug:

  • 1a. Result of the original design (eg. a case wasn't considered)
  • 1b. Mistake during implementation
  • 1c. Backend bug
  • 1z. Other:

Where bug was reported:

  • 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
  • 2b. Reported on staging (eg. found during regression or PR testing)
  • 2d. Reported on a PR
  • 2z. Other:

Who reported the bug:

  • 3a. Expensify user
  • 3b. Expensify employee
  • 3c. Contributor
  • 3d. QA
  • 3z. Other:
  • [Contributor] 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: https://github.com/Expensify/App/pull/37391/files#r1904879930

  • [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: This is a specific edge case and it does not need a discussion.

  • [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

Regression Test Proposal Template

Regression Test Proposal

Precondition:

Have a "Control" workspace with

  1. 2 members invited.
  2. "Workflows" enabled.
  3. "Add approvals" enabled in "Workflows".

Test:

  1. Go to Workspace Settings > Workflows.
  2. Press "Add approval workflow".
  3. On mobile, make sure the keyboard is shown, if not, press on the input.
  4. Verify there are no extra spaces below the button

Do we agree 👍 or 👎

@bernhardoj
Copy link
Contributor

Requested in ND.

@Christinadobrzyn
Copy link
Contributor

Awesome! Thank you @bernhardoj and @c3024! Closing this out as complete.

@JmillsExpensify
Copy link

$250 approved for @bernhardoj

@JmillsExpensify
Copy link

$250 approved for @c3024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2
Projects
Status: Done
Development

No branches or pull requests

6 participants