Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.36k stars 2.79k forks source link

[$250] Android – Account settings – Date of birth years list flicks if select any year some times #47384

Closed IuliiaHerets closed 1 month ago

IuliiaHerets commented 1 month ago

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: 9.0.20-0 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): applausetester+jp_e_category_2@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

  1. Go to https://staging.new.expensify.com/
  2. Log in
  3. Open Account settings
  4. Tap on Profile and open Date of birth
  5. Tap on year and select any year
  6. Tap on year again and select any year
  7. Repeat the last step sometimes

Expected Result:

Date of birth years list not flicks

Actual Result:

Date of birth years list flicks

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/837775c3-8cc8-43c8-b126-b5e62d6d1abe

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01dbcdf988965774c4
  • Upwork Job ID: 1823768879808755531
  • Last Price Increase: 2024-08-28
Issue OwnerCurrent Issue Owner: @getusha
melvin-bot[bot] commented 1 month ago

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.

IuliiaHerets commented 1 month ago

We think that this bug might be related to #vip-vsb

IuliiaHerets commented 1 month ago

@Christinadobrzyn 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

nkdengineer commented 1 month ago

Proposal

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

Date of birth years list flicks

What is the root cause of that problem?

The problem is the keyboard isn't dismissed before we close the year picker modal then the submit button flicks

https://github.com/Expensify/App/blob/29fb6bba4cdff75247ee52154359c6925a2390c9/src/components/DatePicker/CalendarPicker/YearPickerModal.tsx#L76

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

We should dismiss the keyboard before we select the year and we also need to wrap the select logic here in InteractionManager.runAfterInteractions to ensure the year picker modal is closed after the keyboard is hidden

onSelectRow={(option) => {
    Keyboard.dismiss();
    InteractionManager.runAfterInteractions(() => {
        onYearChange?.(option.value);
    });
}}

https://github.com/Expensify/App/blob/29fb6bba4cdff75247ee52154359c6925a2390c9/src/components/DatePicker/CalendarPicker/YearPickerModal.tsx#L76

OPTIONAL: Since this bug only happens for the touch device, we can only use InteractionManager.runAfterInteractions if DeviceCapabilities.canUseTouchScreen is true to deny the delay on other platforms

What alternative solutions did you explore? (Optional)

Christinadobrzyn commented 1 month ago

I'm not sure what we mean by flicker - I see the years don't load - is that what we mean by flicker?

If they are different, I don't think we should fix the flickering because it's not consistent and seems very minor but I'll add a C+ to just double-check if they think we should fix the year loading to see if that resolves the flickering.

https://github.com/user-attachments/assets/97775178-5ec8-4fe8-8aaf-06e050675228

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

Christinadobrzyn commented 1 month ago

@getusha can you double-check if you think the flickering = the years not loading or if you're seeing a flicker that's different?

Christinadobrzyn commented 1 month ago

hey @getusha, can you take a peek at this and give your thoughts - if there's more we need to know from the team or if the flickering makes sense to you?

If that's true can you review this proposal?

melvin-bot[bot] commented 1 month ago

@Christinadobrzyn, @getusha Huh... This is 4 days overdue. Who can take care of this?

Christinadobrzyn commented 1 month ago

Hi @getusha can you provide an update when you have a moment? TY!

getusha commented 1 month ago

can you double-check if you think the flickering = the years not loading or if you're seeing a flicker that's different?

No, i don't think so

I managed to notice the flicker (the list kind of distorts for a fraction of a second and returns back) when closing the page, but it's pretty hard to notice. we can close this issue.

https://github.com/user-attachments/assets/52cbddac-ca12-4b5e-9224-fa6a646b5e36

nkdengineer commented 1 month ago

@getusha The bug here you should select a year, and then you can see the keyboard is always hidden after we go back to the previous page

https://github.com/user-attachments/assets/3ff8e979-b1cf-479b-9c65-62f1886d70ce

getusha commented 1 month ago

and then you can see the keyboard is always hidden after we go back to the previous page

It's not what the issue description states but are you sating we need to keep the keyboard open when we go back to that page?

nkdengineer commented 1 month ago

Tap on year and select any year Tap on year again and select any year

@getusha It's mentioned here

getusha commented 1 month ago

@getusha It's mentioned here

That's part of the steps to reproduce the flicker, am i missing something?

nkdengineer commented 1 month ago

That's part of the steps to reproduce the flicker, am i missing something?

Screenshot 2024-08-20 at 21 23 59

@getusha Yes this is the reproduce steps, you also can see it at the end of the video in OP.

Christinadobrzyn commented 1 month ago

Update for Melvin - I think we're reviewing proposals. @getusha @nkdengineer let me know if I can help with anything!

melvin-bot[bot] commented 1 month ago

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

getusha commented 1 month ago

@getusha Yes this is the reproduce steps, you also can see it at the end of the video in OP.

And the issue is a barely noticeable flicker, right?

nkdengineer commented 1 month ago

And the issue is a barely noticeable flicker, right?

We can see the keyboard is not closed before we close the year list modal which makes the flicker bug.

getusha commented 1 month ago

We can see the keyboard is not closed before we close the year list modal which makes the flicker bug.

Yes, but it's about the significance. it doesn't seem to affect any flow.

nkdengineer commented 1 month ago

Yes, but it's about the significance. it doesn't seem to affect any flow.

@getusha Yes, it's a UI issue. We also use the same solution in other places like here to prevent this UI issue.

melvin-bot[bot] commented 1 month ago

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

Christinadobrzyn commented 1 month ago

Hi @getusha just checking in on this to see if we've decided on a proposal or can you provide an update?

getusha commented 1 month ago

@Christinadobrzyn I feel like this is not worth fixing, i don't see how this affects the flow. it's just a minor UI flicker.

melvin-bot[bot] commented 1 month ago

@Christinadobrzyn @getusha this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

Christinadobrzyn commented 1 month ago

I was thinking the same - thanks for the review @getusha! I'll close this without action.