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.56k stars 2.9k forks source link

Web- Personal details - Once selected year of birth cannot be changed again #24315

Closed izarutskaya closed 1 year ago

izarutskaya commented 1 year 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!


Action Performed:

  1. Navigate to https://staging.new.expensify.com/
  2. Click on Avatar> Profile> Personal details> Date of birth
  3. Select any Date, Month and Year
  4. Click on "Save" button
  5. Navigate back by hitting the "Back" button
  6. Click on Date of birth
  7. Select any Year different from Step 3
  8. Click on "Save" button

Expected Result:

New input should be updated and displayed in Date of birth section

Actual Result:

Once selected year of birth cannot be changed again. Date of birth section displays the previously entered Date

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

Version Number: v1.3.52-1 Reproducible in staging?: Y Reproducible in production?: Y 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 Notes/Photos/Videos: Any additional supporting documentation

https://github.com/Expensify/App/assets/115492554/d06e6d7d-3d55-49f7-a541-b547edeb60e2

Expensify/Expensify Issue URL: Issue reported by: Applause Internal Team Slack conversation: @

View all open jobs on GitHub

melvin-bot[bot] commented 1 year ago

Triggered auto assignment to @alexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

akinwale commented 1 year ago

It looks like this is a case of confusing UX, and not necessarily a bug. The year picker simply changes the current year for the date picker, and the user still has to select a day from the picker to actually update the date of birth.

aditygrg2 commented 1 year ago

Proposal

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

The state is not changing on only changing variables like year and month.

What is the root cause of that problem?

There is no state updation for variables year and month. Only when a particular date is selected, a state is updated.

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

We can simply add a new setState call to update currentDateView on our state inside the CalendarPicker component i.e,

this.props.onSelected(moment(this.state.currentDateView).format('YYYY-MM-DD'))

updating the state of currentDateView before according to the date selected, on changing year here, and for months here and here

To handle edge cases, for example, Feb has only 28 days most of the time, and 29 days once in 4 years, the states can handle themselves by selecting the last values, so if March has 31 days, selecting Feb will automatically set the state to 28/29.

What alternative solutions did you explore?

N/A

This might be a UX improvement, as the user sometimes selects the incorrect year only, and then changing the year then does not change the state. Though this might not be a bug, indeed a good feature request.

This will look like this once implemented:

https://github.com/Expensify/App/assets/98523623/1e2c5d74-d0a2-44af-9aaa-c5e5625d89c4

StevenKKC commented 1 year ago

Proposal

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

Web- Personal details - Once selected year of birth cannot be changed again.

What is the root cause of that problem?

In date of birth page, if user selected a date, setDate is called and set the date of birth. https://github.com/Expensify/App/blob/58dc0c9576f8c3d3fa8924a7fec0889232c4b537/src/components/NewDatePicker/index.js#L65-L70 However, in CalendarPicker component, only if the day of month is selected, onSelected will be called. https://github.com/Expensify/App/blob/58dc0c9576f8c3d3fa8924a7fec0889232c4b537/src/components/NewDatePicker/CalendarPicker/index.js#L99-L106 If year is changed, onSelected does not be called. https://github.com/Expensify/App/blob/58dc0c9576f8c3d3fa8924a7fec0889232c4b537/src/components/NewDatePicker/CalendarPicker/index.js#L80-L93

Therefore, when user changes the year, date of birth does not be updated.

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

We should call onSelected if year is updated.

    onYearSelected(year) {
        this.setState(
            (prev) => {
                const newCurrentDateView = moment(prev.currentDateView).set('year', year).toDate();

                return {
                    currentDateView: newCurrentDateView,
                    isYearPickerVisible: false,
                    years: _.map(prev.years, (item) => ({
                        ...item,
                        isSelected: item.value === newCurrentDateView.getFullYear(),
                    })),
                };
            },
            () => this.props.onSelected(moment(this.state.currentDateView).format('YYYY-MM-DD')),
        );
    }

What alternative solutions did you explore? (Optional)

None.

Habben101 commented 1 year ago

Proposal

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

Unable to change the selected year of birth once it has been initially selected. The date of birth section displays the previously entered date, and the year selection appears to be locked after the initial selection.

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

The issue arises from how the onYearSelected function is managing the state updates. The state might not be getting fully updated to reflect the new year selection, leading to incorrect rendering and preventing users from changing the year again.

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

we need to ensure that the onYearSelected function correctly updates both the currentDateView and the years array in the state. Here's what needs to be changed:

So, we could change the onYearSelected function to this:

onYearSelected(year) {
     this.setState((prevState) => {
        const newCurrentDateView = moment(prevState.currentDateView).set('year', year).toDate();

        const updatedYears = prevState.years.map((item) => ({
            ...item,
            isSelected: item.value === newCurrentDateView.getFullYear(),
         }));

         return {
             currentDateView: newCurrentDateView,
             years: updatedYears,
             isYearPickerVisible: false,
         };
      }, () => {
          this.props.onSelected(moment(this.state.currentDateView).format('YYYY-MM-DD'));
    });
 }

Result:

https://github.com/Expensify/App/assets/25123389/06d5b9e4-f257-4ced-bdf4-7643c3b42e0f

What alternative solutions did you explore?

NA

alexpensify commented 1 year ago

I've tested and agree with @akinwale

It looks like this is a case of confusing UX, and not necessarily a bug. The year picker simply changes the current year for the date picker, and the user still has to select a day from the picker to actually update the date of birth.

If you change the year, then you need to select the date again. The original date selected does not edit when you change the year. For example, 1979-08-01 does not edit to the new year selected like 1982-08-01.

2023-08-11_16-56-10

I'll need to continue reviewing this one to see if this is on purpose or I should close this GH.

alexpensify commented 1 year ago

I've started a discussion here:

https://expensify.slack.com/archives/C01GTK53T8Q/p1692049999219919

c3024 commented 1 year ago

Dupe of #23328 and it was closed saying this is expected behaviour.

alexpensify commented 1 year ago

Thanks for flagging!