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] "Next" and "split" button moves a bit when navigate back #41855

Closed
1 of 6 tasks
m-natarajan opened this issue May 8, 2024 · 22 comments
Closed
1 of 6 tasks

[$250] "Next" and "split" button moves a bit when navigate back #41855

m-natarajan opened this issue May 8, 2024 · 22 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 Reviewing Has a PR in review Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented May 8, 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: 1.4.71-4
Reproducible in staging?: Yes
Reproducible in production?: Yes
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: @youssef-lr
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1715122550547929

Action Performed:

  1. Launch the app
  2. Tap the fab > Split expense
  3. Enter amount and Choose from contacts
  4. Tap Next > Tap the back arrow
  5. Tap Next again

Expected Result:

No visual issues when navigating back and forth

Actual Result:

After step 4 and 5 "Next" and "Split" button moves

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

RPReplay_Final1715122397.MP4
RPReplay_Final1715186635.MP4

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01419f469e7473b289
  • Upwork Job ID: 1789073733749583872
  • Last Price Increase: 2024-05-10
  • Automatic offers:
    • Ollyws | Reviewer | 0
    • bernhardoj | Contributor | 0
Issue OwnerCurrent Issue Owner: @Ollyws
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels May 8, 2024
Copy link

melvin-bot bot commented May 8, 2024

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

Proposal

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

The next and split button moves down when navigating between pages while a keyboard is open.

What is the root cause of that problem?

The button is wrapped with FixedFooter.

function FixedFooter({style, children}: FixedFooterProps) {
const {isKeyboardShown} = useKeyboardState();
const insets = useSafeAreaInsets();
const styles = useThemeStyles();
if (!children) {
return null;
}
const shouldAddBottomPadding = isKeyboardShown || !insets.bottom;
return <View style={[styles.ph5, shouldAddBottomPadding && styles.pb5, styles.flexShrink0, style]}>{children}</View>;
}

The confirmation page uses BaseOptionsSelector which uses FixedFooter.

In FixedFooter, we add a bottom padding if the keyboard shown state is true. When we move to the confirmation page while the keyboard is shown, both the padding-bottom from FixedFooter and the confirmation page itself are added. After the keyboard state is fully hidden, the padding-bottom from FixedFooter is gone, so the button moves down a bit.

In the money request participants page, we are using BaseSelectionList and only add the padding-bottom if the keyboard is hidden. The list footer also uses FixedFooter.

{({safeAreaPaddingBottomStyle}) => (
<View style={[styles.flex1, !isKeyboardShown && safeAreaPaddingBottomStyle, containerStyle]}>

So, when we open/close the keyboard, you will see the button move up/down based on the keyboard state.

I believe the reason we add the padding-bottom in FixedFooter only when the keyboard is shown is because the padding-bottom in BaseSelectionList is gone when the keyboard is shown. It's to replace the removed padding-bottom so the list footer content has a padding-bottom.

Without padding-bottom (if we remove the padding-bottom logic from FixedFooter):
image

The reason we don't apply padding-bottom in BaseSelectionList when the keyboard is shown is to fix a small blank space above the keyboard. This is because of the padding bottom that we apply to the list. If we apply it (includeSafeAreaPaddingBottom) to the ScreenWrapper, the issue won't happen

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

Remove the padding-bottom logic based on the keyboard state from FixedFooter. Then, we will apply the padding-bottom in BaseSelectionList if there is a footer.

<View style={[styles.flex1, !isKeyboardShown && safeAreaPaddingBottomStyle, containerStyle]}>

(!isKeyboardShown || !!footerContent || showConfirmButton) && safeAreaPaddingBottomStyle

What alternative solutions did you explore? (Optional)

Remove the safe area padding bottom from BaseSelectionList and enable includeSafeAreaPaddingBottom to every page that uses BaseSelectionList. Currently, all pages that use BaseSelectionList disables includeSafeAreaPaddingBottom to not have double padding bottom.

@melvin-bot melvin-bot bot added the Overdue label May 10, 2024
@alexpensify
Copy link
Contributor

On my testing list, I'll get to it soon.

@melvin-bot melvin-bot bot removed the Overdue label May 10, 2024
@youssef-lr
Copy link
Contributor

youssef-lr commented May 10, 2024

@alexpensify I think this fits as a general improvement to the UI and not particularly tied to any project. It makes the UI looks glitchy and unpolished. Can we get a C+ assigned to review the proposal? Also I think we need to tackle this everywhere in the UI where it happens, and not just the two pages mentioned in this issue.

@alexpensify alexpensify added the External Added to denote the issue can be worked on by a contributor label May 10, 2024
Copy link

melvin-bot bot commented May 10, 2024

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

@melvin-bot melvin-bot bot changed the title "Next" and "split" button moves a bit when navigate back [$250] "Next" and "split" button moves a bit when navigate back May 10, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label May 10, 2024
Copy link

melvin-bot bot commented May 10, 2024

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

@alexpensify
Copy link
Contributor

Thanks, @youssef-lr, for the context! Assigning the external label now.

@Ollyws please review the proposal and identify if it will fix the issue. Thanks!

@Ollyws
Copy link
Contributor

Ollyws commented May 13, 2024

@bernhardoj's proposal LGTM.

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented May 13, 2024

Triggered auto assignment to @yuwenmemon, 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 May 13, 2024
Copy link

melvin-bot bot commented May 13, 2024

📣 @Ollyws 🎉 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

Copy link

melvin-bot bot commented May 13, 2024

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

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels May 14, 2024
@bernhardoj
Copy link
Contributor

PR is ready

cc: @Ollyws

@youssef-lr
Copy link
Contributor

@bernhardoj are you seeing this?

image

@bernhardoj
Copy link
Contributor

@youssef-lr yes, I'm seeing that on main too.

@alexpensify alexpensify removed their assignment May 17, 2024
@alexpensify alexpensify added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels May 17, 2024
Copy link

melvin-bot bot commented May 17, 2024

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels May 17, 2024
@alexpensify alexpensify added Weekly KSv2 and removed Daily KSv2 labels May 17, 2024
@alexpensify alexpensify self-assigned this May 17, 2024
@alexpensify
Copy link
Contributor

Heads up, I will be offline until Tuesday, May 28, 2024, and will not actively watch over this GitHub during that period.

@kevinksullivan - While I'm offline, I need help here completing the payment process after this one clears the 7-day period. Thanks!

@alexpensify
Copy link
Contributor

Thanks, @kevinksullivan for the help here. I'm back online and taking over again as the BZ member. Tomorrow, I'll start the payment process since it looks like automation failed in this GH.

@alexpensify
Copy link
Contributor

alexpensify commented May 29, 2024

Payouts due: 2024-05-29

Upwork job is here.

@alexpensify
Copy link
Contributor

I've paid @bernhardoj via Upwork. @Ollyws, please accept the invite and I can complete the payment process. Thanks!

@Ollyws
Copy link
Contributor

Ollyws commented May 30, 2024

Requested in ND.

@alexpensify
Copy link
Contributor

Awesome, and sorry for the confusion. I didn't see you listed in the SO. Closing since the final payment will happen in Chat.

@JmillsExpensify
Copy link

$250 approved for @Ollyws

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 Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

8 participants