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 2022-10-04] [$250] Android - "Close account" button partially covers the text information when keypad opened #10903

Closed
kavimuru opened this issue Sep 8, 2022 · 29 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review

Comments

@kavimuru
Copy link

kavimuru commented Sep 8, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Open the App/Android and login
  2. Go to Settings->Security->Close account
  3. Focus on "Enter message" field and then "Enter default method" field

Expected Result:

  1. "Close account" button covers the text information when focusing in "Enter message" field;
  2. "Close account" button is very near with "Enter default method" field.

Actual Result:

The input fields and the button should not overlap and be too close to each other

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Android

Version Number: v1.1.98-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:
https://user-images.githubusercontent.com/43996225/189174390-bcca3295-8cf3-4937-9835-68aa80bd9192.mp4
Upwork job URL: https://www.upwork.com/jobs/~015819339bfc73c40e
Issue reported by: Applause internal team
Slack conversation:

View all open jobs on GitHub

@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2022

Triggered auto assignment to @AndrewGable (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@AndrewGable AndrewGable removed their assignment Sep 9, 2022
@AndrewGable AndrewGable added Engineering External Added to denote the issue can be worked on by a contributor and removed Engineering labels Sep 9, 2022
@melvin-bot
Copy link

melvin-bot bot commented Sep 9, 2022

Triggered auto assignment to @michaelhaxhiu (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot melvin-bot bot added the Overdue label Sep 12, 2022
@melvin-bot
Copy link

melvin-bot bot commented Sep 13, 2022

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

@kakajann
Copy link
Contributor

Proposal

This issue occurs on iOS too.

1. "Close account" button covers the text information when focusing in "Enter message" field;

This is IMO expected behaviour, but I suggest reducing number of lines of Enter message here input to be 4. Current value is 6.

This is for non-iOS devices.

- numberOfLines={6}
+ numberOfLines={4}

This is for iOS

closeAccountMessageInput: {
- height: 153,
+ height: 103,
},

Before

Screen.Recording.2022-09-13.at.9.26.25.PM.mov

After

Screen.Recording.2022-09-13.at.9.27.21.PM.mov

2. "Close account" button is very near with "Enter default method" field.

Because the button has no padding-top. Add this:

- <FixedFooter>
+ <FixedFooter style={[styles.pt5]}>
Before After
Screen Shot 2022-09-13 at 9 47 36 PM Screen Shot 2022-09-13 at 9 47 28 PM

@michaelhaxhiu michaelhaxhiu changed the title Android - Close account - "Close account" button partially covers the text information when keypad opened Android - "Close account" button partially covers the text information when keypad opened Sep 14, 2022
@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 Overdue labels Sep 14, 2022
@melvin-bot
Copy link

melvin-bot bot commented Sep 14, 2022

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 14, 2022
@melvin-bot
Copy link

melvin-bot bot commented Sep 14, 2022

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

@melvin-bot melvin-bot bot changed the title Android - "Close account" button partially covers the text information when keypad opened [$250] Android - "Close account" button partially covers the text information when keypad opened Sep 14, 2022
@michaelhaxhiu
Copy link
Contributor

Note @mananjadhav -- we have a solution proposal above already, which was provided by @kakajann. Let's start there in terms of reviewing proposals.

@mananjadhav
Copy link
Collaborator

@marcochavezf @michaelhaxhiu I am fine with @kakajann's proposal if we're okay to reduce the height of the text input. But please note that this issue might still persist when we have smaller height devices. @kakajann Did you check if your proposal works on smaller devices like iPhone SE?

@kakajann
Copy link
Contributor

1st issue is expected behavior though. Here's how it appears on iPhone SE (3rd generation)
Screen Shot 2022-09-14 at 7 01 50 PM

@mananjadhav
Copy link
Collaborator

1st issue is expected behavior though.

Exactly. Thanks @kakajann. I think we should just focus on the second part of the issue. We can still reduce the number of lines so that it works fine for most devices (off lately all iOS devices from 8 onwards have a good large screen, excluding minis).

Thoughts? @michaelhaxhiu @marcochavezf. Nevertheless, @kakajann's proposal is good for me.

🎀 👀 🎀 
C+ reviewed

@michaelhaxhiu
Copy link
Contributor

cc @Expensify/design for input on adjusting the text input to accomplish this change. Does that seem kosher?

@shawnborton
Copy link
Contributor

Hmm I think we had some open discussion about not making the bottom-docked button cover any of the content? Basically this conversation here.

cc @parasharrajat

@kakajann
Copy link
Contributor

@shawnborton is this what we want here?

Screen.Recording.2022-09-16.at.7.38.32.AM.mov

@shawnborton
Copy link
Contributor

That looks perfect, thanks!

@mananjadhav
Copy link
Collaborator

That looks perfect, thanks!

I've taken @shawnborton's comment here as a 🟢 to @kakajann's proposal and the fact that he has updated the proposals on feedback and raised the PR.

@melvin-bot melvin-bot bot added the Overdue label Sep 26, 2022
@marcochavezf
Copy link
Contributor

Oh I forgot to assign @kakajann to the issue here, thanks for the proposal and the PR!

@melvin-bot
Copy link

melvin-bot bot commented Sep 26, 2022

📣 @kakajann You have been assigned to this job by @marcochavezf!
Please apply to this job in Upwork 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 removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 26, 2022
@marcochavezf marcochavezf added the Reviewing Has a PR in review label Sep 26, 2022
@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Sep 27, 2022
@melvin-bot melvin-bot bot changed the title [$250] Android - "Close account" button partially covers the text information when keypad opened [HOLD for payment 2022-10-04] [$250] Android - "Close account" button partially covers the text information when keypad opened Sep 27, 2022
@melvin-bot
Copy link

melvin-bot bot commented Sep 27, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.7-2 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 2022-10-04. 🎊

@michaelhaxhiu
Copy link
Contributor

@kakajann invited you to the job in upwork, can you accept when you have a chance?

@kakajann
Copy link
Contributor

Just accepted

@melvin-bot
Copy link

melvin-bot bot commented Sep 28, 2022

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@melvin-bot
Copy link

melvin-bot bot commented Oct 4, 2022

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

1 similar comment
@melvin-bot
Copy link

melvin-bot bot commented Oct 4, 2022

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@michaelhaxhiu
Copy link
Contributor

@kakajann is paid.

@mananjadhav please apply to this new job today (link) , since the OG job expired

@michaelhaxhiu
Copy link
Contributor

Everyones paid. 😎✌️ Keep it coming folks!

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 Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

9 participants