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: ion-datetime preferWheel does not consistently display current value when presented in an overlay #26234

Closed
4 of 7 tasks
Gkleinereva opened this issue Nov 5, 2022 · 14 comments · Fixed by #29147
Closed
4 of 7 tasks
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@Gkleinereva
Copy link
Contributor

Prerequisites

Ionic Framework Version

  • v4.x
  • v5.x
  • v6.x
  • Nightly

Current Behavior

See reproduction repo below - relevant file is src/app/tab1/tab1.page.html and there's some additional documentation in a comment there.

We want to use an <ion-datetime> Angular component in an <ion-modal> component in conjunction with an <ion-input type="date"> component to provide the ion-datetime for use on mobile, while allowing desktop users to tab into the field and type dates manually.

In this context, everything works fine the first time you open the modal. However, when the modal is subsequently opened (i.e., the second time and every time after) the ion-datetime component fails to respect the [value] property.

This problem occurs when type="date" or when type="datetime-local" on the <ion-input>. It doesn't happen if type="text".

Expected Behavior

The <ion-datetime> component should always respect it's [value] property, regardless of how it's opened.

Steps to Reproduce

  1. Start the ionic application in the repro repo
  2. Click in the input field on the landing page to open the modal - notice that it respects the [value] we gave it
  3. Close the modal (either by clicking cancel or done)
  4. Open the modal again - notice that it no longer respects the [value].
  5. Change the "type" property on the <ion-input> to "text"
  6. Repeat steps 1-4 and notice that the issue no longer appears.

Code Reproduction URL

https://github.com/Gkleinereva/ion-datetime-repro

Ionic Info

[WARN] You are not in an Ionic project directory. Project context may be missing.

Ionic:

Ionic CLI : 6.20.3

Utility:

cordova-res : not installed globally
native-run : not installed globally

System:

NodeJS : v16.16.0
npm : 8.11.0
OS : Linux 5.10

Additional Information

Originally observed in an NX monorepo (hence the sparse ionic info output)

@ionitron-bot ionitron-bot bot added the triage label Nov 5, 2022
@sean-perkins
Copy link
Contributor

Hello @Gkleinereva thanks for this issue.

Can you try updating your ion-modal to use keepContentsMounted?

e.g.:

<ion-input id="target" type="date"></ion-input>
<ion-modal trigger="target" [keepContentsMounted]="true">
  <ng-template>
    <ion-datetime 
      locale="en-US" 
      [showDefaultButtons]="true" 
      [value]="'1992-11-05T20:23:29.685Z'"
      presentation="date" 
      [preferWheel]="true"></ion-datetime>
  </ng-template>
</ion-modal>

This will prevent a new instance of the datetime being created when the modal is presented, preserving the state of the value across interactions.

@sean-perkins sean-perkins added the needs: reply the issue needs a response from the user label Nov 7, 2022
@ionitron-bot ionitron-bot bot removed the triage label Nov 7, 2022
@Gkleinereva
Copy link
Contributor Author

@sean-perkins that worked, thank you very much! I'm still curious why there is a difference in behavior between the text input and the date input though. Can you shed any light on that? I generally try to not use [keepContentsMounted] if I don't have to in order to avoid potential id/state collisions.

Also for any future explorers - I ended up implementing a workaround using the ModalController and opening the modal programmatically. This method also addressed the issue.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Nov 7, 2022
@sean-perkins
Copy link
Contributor

@Gkleinereva I am unable to reproduce the difference between type="text" and type="date" that is described in the reproduction. Both instances result in the same behavior for me:

  • Click ion-input with type="text" or type="date"
  • Modal with ion-datetime opens, value is November 5th, 1922.
  • Cancel/Confirm the modal to dismiss it
  • Click theion-input again
  • Modal withion-datetime opens, value is today.

Thank you for confirming the workaround.

I do think we still need to track this behavior as a bug, but what I am observing is the visual state is being disconnected from the value in this scenario.

e.g.: The ion-datetime value when following my above steps results in 1992-11-05T20:23:29.685Z, but the displayed value is today.

@Gkleinereva
Copy link
Contributor Author

type="text" isn't affected by this issue for me locally for some reason. Also interestingly, my application changes the displayed date to 'January 1 2022' when type="date", rather than showing today's date.

Could it be some sort of difference by platform? I'm viewing it in MS Edge, and the behavior is the same across different device emulations. I'm running Node 16.16.0 (same as in the .nvmrc I included).

@sean-perkins
Copy link
Contributor

sean-perkins commented Nov 7, 2022

@Gkleinereva if you open and close the modal 3-6 times, does the problem reproduce for each example?

I am getting consistent reproduction, but at odd intervals with this reproduction example: https://stackblitz.com/edit/angular-eozbgu?file=src%2Fapp%2Fexample.component.html

The picker columns are likely trying to restore the scroll position (incorrectly), resulting in the first index of each column to be selected: "January 1, 2022".

I'm going to rework the title a bit, as I think this relates to preferWheel being enabled.

Edit:

Note for team, I am unable to reproduce against the core implementation. This appears to be an issue isolated to the framework implementations (Angular).

@sean-perkins sean-perkins changed the title bug: ion-datetime doesn't respect [value] property in certain scenarios bug: ion-datetime preferWheel does not consistently display current value when presented in an overlay Nov 7, 2022
@sean-perkins sean-perkins added package: core @ionic/core package package: angular @ionic/angular package type: bug a confirmed bug report labels Nov 7, 2022
@ionitron-bot ionitron-bot bot removed the triage label Nov 7, 2022
@Gkleinereva
Copy link
Contributor Author

@sean-perkins I can reproduce it very intermittently in your stackblitz (it took me a few minutes of trying to see it for the first time - it worked correctly for both inputs most of the time). In contrast, I reproduce it very predictably in my application and in my reproduction on my local machine (for type="date"). Makes me think it's a race condition of some kind?

@Gkleinereva
Copy link
Contributor Author

@sean-perkins I thought that both the [keepContentsMounted] and the ModalController workarounds were working, but I went back to my branch this afternoon and the issue is there intermittently with both workarounds.

We decided to just avoid using [preferWheel] until this issue is resolved. Thanks for your response and help!

@sean-perkins
Copy link
Contributor

Note: This also applies in scenarios with presentation="month-year", basically where ever ion-picker-internal is being used: #26604

@mapsandapps
Copy link
Contributor

mapsandapps commented Nov 13, 2023

When I was investigating #28516 I was able to reproduce this in Vue but not in Angular, React, or JS. Then, I updated Chrome from 118 to 119, and now I can reproduce it in React and Vue, but not in Angular. It seems unlikely to be framework-specific, so I'll remove that label.

The workaround of adding keepContentsMounted is still working.

Update: Upon further investigation, I was able to reproduce this in Angular when using standalone components, but not when using NgModules. This suggests that the bug relates to the custom elements build. (Although, again, since it's sporadic, it's hard to be sure.)

@Andr9651
Copy link

Andr9651 commented Dec 11, 2023

I'm having the same issue with version 7.6.0 of ionic in vue 3

I've made a repro of it, if it's any help.
https://stackblitz.com/~/github.com/Andr9651/ionic-datetime-repro
reproduction video.webm

@liamdebeasi
Copy link
Contributor

Here's a dev build if anyone is interested in testing a proposed fix: 7.7.5-dev.11710347256.1954cae9

github-merge-queue bot pushed a commit that referenced this issue Mar 13, 2024
Issue number: resolves #26234

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

We use an IntersectionObserver to know when each picker column is
visible so we can properly scroll the active option in each column into
view. The IO callback passes an array of entries. Up until this PR, we
have always grabbed the first entry in the array. The problem is that
browsers will sometimes group multiple events into a single array. This
means it's possible to have an event with `isIntersecting: false` and
then another event with `isIntersecting: true` in the same callback.
Since we always grabbed the first event we did not account for the
instances where events were coalesced.

This resulted in column options sometimes not scrolling into view when
presented via an overlay.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Picker column now grabs the last event in the entries array. This
represents the most recent threshold change.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer
for more information.
-->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Dev build: `7.7.5-dev.11710347256.1954cae9`
@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #29147, and a fix will be available in an upcoming release of Ionic Framework. Please feel free to continue testing the dev build, and let me know if you have any questions.

@capc0
Copy link
Contributor

capc0 commented Apr 4, 2024

Thanks for the fix. I can confirm that the fix is working for us.
Using month-and-year picker inside ion-modals was previously broken on android browsers or even in chrome dev tools with an android emulated device.

Copy link

ionitron-bot bot commented May 4, 2024

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 May 4, 2024
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

Successfully merging a pull request may close this issue.

6 participants