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

bug: datetime confirm method ignores selected month and year when selection dropdown is still open #24962

Closed
NilsFrkal opened this issue Mar 17, 2022 · 15 comments
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@NilsFrkal
Copy link

Ionic version:
[] 4.x
[ ] 5.x
[ x] 6.x

[ x] bug report
[ ] feature request

Current behavior:

When using the date-time component (presentation="date") the selected year and month gets ignored when passed as element value. Example:

current date: 18/03/2022
selection:
day: 15
month: December
year: 1980

console.log(dateinput.value): 15/03/2022

This behavior can be observed with all versions > @ionic/[email protected] i.e. 6.0.1 still works, all later versions up to 6.0.12 do not.

Expected behavior:

Selected year and month should be considered i.e. expected output for example above 15/12/1980

Steps to reproduce:

  1. Update to version > 6.0.1 e.g. latest version @ionic/angular@latest
  2. Create date-time component
  3. Select a new date with month and year and pass in ionChange event via element value (see code snippet below)
  4. Console.log value

To check working version:

  1. npm install @ionic/[email protected]
  2. Create date-time component
  3. Select a new date with month and year and pass in ionChange event via element value (see code snippet below)
  4. Console.log value

Related code:

<ion-datetime id="XXX" [value]="date" presentation="date"
(ionChange)="onSelectDate(dateinput.value)" showDefaultButtons="true"
(ionCancel)="onDatePickerCancel()" #dateinput>

onSelectDate (value) {
console.log(value);
}

Ionic info:

Ionic:

Ionic CLI : 6.19.0 (D:\Development\Nodist\bin\node_modules@ionic\cli)
Ionic Framework : @ionic/angular 6.0.12
@angular-devkit/build-angular : 13.3.0
@angular-devkit/schematics : 13.3.0
@angular/cli : 13.3.0
@ionic/angular-toolkit : 2.3.3

Capacitor:

Capacitor CLI : 3.4.3
@capacitor/android : 3.4.0
@capacitor/core : 3.4.3
@capacitor/ios : 3.4.3

Utility:

cordova-res : 0.15.4
native-run : 1.5.0

System:

NodeJS : v16.13.2 (D:\Development\Nodist\v-x64\16.13.2\node.exe)
npm : 8.1.2
OS : Windows 10

@ionitron-bot ionitron-bot bot added the triage label Mar 17, 2022
@liamdebeasi liamdebeasi added the ionitron: needs reproduction a code reproduction is needed from the issue author label Mar 18, 2022
@ionitron-bot
Copy link

ionitron-bot bot commented Mar 18, 2022

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please reproduce this issue in an Ionic starter application and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.

For a guide on how to create a good reproduction, see our Contributing Guide.

@ionitron-bot ionitron-bot bot removed the triage label Mar 18, 2022
@liamdebeasi
Copy link
Contributor

Thanks for the issue. Can you provide a GitHub repo I can use to verify this? I cannot reproduce this behavior on my end.

@NilsFrkal
Copy link
Author

While putting the test repo together I noticed something which should be relevant. The problem occurs only when you click the "Done" button when the Year / Month view is open. In 6.0.1 this resulted in still passing the correct date, but > 6.0.1 doesn't.

Git Repo: https://github.com/NilsFrkal/date-time-test

Thank you.

@averyjohnston averyjohnston added triage and removed ionitron: needs reproduction a code reproduction is needed from the issue author labels Mar 23, 2022
@averyjohnston
Copy link
Contributor

I'm unable to reproduce the issue using the repo; it looks like the logged value is always what I selected. Could you provide more detailed steps to reproduce? A screencast may help.

@averyjohnston averyjohnston added the needs: reply the issue needs a response from the user label Mar 25, 2022
@ionitron-bot ionitron-bot bot removed the triage label Mar 25, 2022
@NilsFrkal
Copy link
Author

26.03.2022_08.37.44_REC.mp4

Recording attached. Please make sure that the show-default-buttons option is set to true as the error only occurs when using the default "Done" button to confirm the selection.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Mar 25, 2022
@averyjohnston
Copy link
Contributor

Thanks! I'm able to replicate this. It looks like this is the result of a confusing interaction between how the month/year selection works and how the datetime's value selection works.

With the current behavior, changing the month/year doesn't immediately change the value. You can see this by closing the dropdown, and noting that the newly selected month doesn't have any days highlighted. Since clicking "Done" uses the currently selected value, it appears that the month/year selection is being ignored. This is exacerbated by confusing UI, in that the "Done" button appears to apply to the month/year dropdown, rather than the whole datetime.

There are a few ways we could go about fixing this, some of which overlap with #24998. We could:

  • Make the UI less confusing
  • Make the month/year selection immediately change the value
  • Have the "Done" button force the value to update to the new month/year if the dropdown is still open

... or some combination of the three. Regardless, I agree that this is buggy-looking behavior and should be tightened up somehow.

@averyjohnston averyjohnston changed the title date-time component ignores selected month and year with @ionic/angular > 6.0.1 bug: datetime confirm method ignores selected month and year when selection dropdown is still open Mar 29, 2022
@averyjohnston averyjohnston added package: core @ionic/core package type: bug a confirmed bug report labels Mar 29, 2022
@ionitron-bot ionitron-bot bot removed the triage label Mar 29, 2022
@DavidStrausz
Copy link
Contributor

Is there a workaround or a dev build we can use until this is properly fixed? Option 3 from @amandaesmith3 suggestions sounds like a viable solution for us!

@dodomui
Copy link

dodomui commented Apr 1, 2022

Agree, it is really confusing to our user. Looking forward to a solution soon.

@ikosta
Copy link

ikosta commented Apr 5, 2022

@amandaesmith3 I've fixed it right now with an own confirm button but it would be also nice to have access to the toggleMonthAndYearView method because right now the state is not set correctly and the icon is therefore wrong.

It will work for us right now but it is ugly.

  public async confirm(): Promise<void> {
    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    const nativeElement = (this.ionDatetime as any).el;

    /**
     * Since Ionic does not give us acess to the month and year toggle we need to get the native element
     * and check if the month and year is shown. If so we just close the month and year.
     */
    if (nativeElement.classList.contains('show-month-and-year')) {
      nativeElement.classList.remove('show-month-and-year');

      return;
    }

    await this.ionDatetime?.confirm(true);
  }

@SidiBecker
Copy link

SidiBecker commented Apr 8, 2022

@amandaesmith3 I've fixed it right now with an own confirm button but it would be also nice to have access to the toggleMonthAndYearView method because right now the state is not set correctly and the icon is therefore wrong.

It will work for us right now but it is ugly.

  public async confirm(): Promise<void> {
    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    const nativeElement = (this.ionDatetime as any).el;

    /**
     * Since Ionic does not give us acess to the month and year toggle we need to get the native element
     * and check if the month and year is shown. If so we just close the month and year.
     */
    if (nativeElement.classList.contains('show-month-and-year')) {
      nativeElement.classList.remove('show-month-and-year');

      return;
    }

    await this.ionDatetime?.confirm(true);
  }

Hello @ikosta , I was use your code, but still exists a little bug with toggle month/year . I able to improve UX, simulating the click on element, working in a better way.

public async confirm(): Promise<void> {
    const nativeElement = (this.ionDatetime as any).el;

    if (nativeElement.classList.contains('show-month-and-year')) {
      const el: any =  nativeElement.shadowRoot.querySelector('.calendar-month-year');
      el.querySelector('ion-item').shadowRoot.querySelector('button').click()
      return;
    }

    await this.ionDatetime?.confirm(true);
  }

@averyjohnston
Copy link
Contributor

Hi folks, I've merged in #25205 which hides the datetime footer when the month/year picker is open, solving this issue indirectly. You can see more info in the PR description. The update will be available in a future release of Ionic 👍

@ikosta
Copy link

ikosta commented Apr 30, 2022

Oh dear... hope our UX team is happy with that

@NilsFrkal
Copy link
Author

I was also hoping we could simply re-implement the behavior from @ionic/[email protected] as it does work correctly in this version.

@liamdebeasi
Copy link
Contributor

liamdebeasi commented May 2, 2022

Hi everyone,

We made this change for a couple reasons:

  1. Better consistency across platforms.

We have found that maintaining separate layouts/behaviors across iOS and Android results in a lot of unexpected behaviors (for example, a behavior found only on iOS that users are expecting to also be found on Android). We are also a small team. We feel that managing two different implementations of datetime, a very complex component, is not in the best interest of the project.

  1. Less confusion about what the Ok/Cancel buttons do.

We received feedback in #24998 that showing the Ok/Cancel buttons when the month/year picker is open was confusing. Developers were expecting that the Ok/Cancel buttons would confirm only the wheel picker selector and then return you to the main calendar interface.

We explored simply adding a divider line between the picker and buttons, but ultimately decided not to go that route for accessibility reasons.


I will also note that there is no change when using the date, month, year, month-year, or time presentation property values values.

We understand this change may not be what every developer is expecting, but we think this change will result in a better experience for most users. Thanks!

@ionitron-bot
Copy link

ionitron-bot bot commented Jun 1, 2022

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Jun 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

No branches or pull requests

7 participants