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, showing calendar body after intersection observer callback causes issues on safari #24542

Closed
4 of 6 tasks
elbugs opened this issue Jan 9, 2022 · 22 comments · Fixed by #29163
Closed
4 of 6 tasks
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@elbugs
Copy link

elbugs commented Jan 9, 2022

Prerequisites

Ionic Framework Version

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

Current Behavior

Animations are choppy, feels like need more frames. (Have seen it opening a date time and action sheet menu
ezgif-6-1b0a9adcd5
)

Expected Behavior

It should open smooth, without lag, without choppy frames. Like in material design, or google chrome:
ezgif-3-39c1c976ab

Steps to Reproduce

Just install the blank starter project, add a ionic date-time for exampl.
Open in Safari with ios theme and click to open the modal. Be aware of the choppy frames at the end of the animation and the lag at showing the days of the month.

All of this issues do not happen in Chrome, neither with md or ios theme or Android device.
Also, does not happen with Safari with md theme.

Code Reproduction URL

https://github.com/gpiccon/ionic_playground

Ionic Info

Ionic:

Ionic CLI : 6.18.1 (/usr/local/lib/node_modules/@ionic/cli)
Ionic Framework : @ionic/react 6.0.1

Capacitor:

Capacitor CLI : 3.3.4
@capacitor/android : 3.3.4 (/Users/jordipicon/node_modules/@capacitor/android)
@capacitor/core : 3.3.4
@capacitor/ios : 3.3.4 (/Users/jordipicon/node_modules/@capacitor/ios)

Utility:

cordova-res : 0.15.4
native-run : 1.5.0

System:

NodeJS : v16.13.1 (/usr/local/bin/node)
npm : 8.1.2
OS : macOS Monterey

Additional Information

Note: Can't reproduce with Angular framework.

@ionitron-bot ionitron-bot bot added the triage label Jan 9, 2022
@elbugs elbugs changed the title bug: WebKit choppy and freeze frames bug: WebKit choppy and freeze frames at Ionic6 React Jan 9, 2022
@liamdebeasi liamdebeasi self-assigned this Jan 10, 2022
@liamdebeasi
Copy link
Contributor

Thanks for the issue. There appear to be two reported problems:

  1. The entering animation in iOS mode on Safari has poor performance.
  2. There is a delay in showing the body of the calendar picker in iOS mode on Safari.

Did I understand the issue correctly? I can reproduce issue 2, but I cannot reproduce issue 1. What device are you testing on to get issue 1 to reproduce?

@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Jan 10, 2022
@ionitron-bot ionitron-bot bot removed the triage label Jan 10, 2022
@liamdebeasi liamdebeasi removed their assignment Jan 10, 2022
@elbugs
Copy link
Author

elbugs commented Jan 10, 2022

Hello. Yes, you are right.
Issue 1 happens on iPad and iOS when I compile using capacitor, MacBook Pro M1 with Safari and narrow screen... Everywhere. How can I help?

EDIT: I record a video, maybe it helps: https://vimeo.com/664362198/c7125e4af1

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Jan 10, 2022
@liamdebeasi
Copy link
Contributor

Thanks! I think the jump you are seeing in Safari is related to the repaint that happens when we show the calendar body. This behavior was added to workaround another issue, but I think we can improve on this: https://github.com/ionic-team/ionic-framework/blob/main/core/src/components/datetime/datetime.tsx#L867-L876

My current thinking is that by removing this additional repaint the animation jump in Safari will disappear as well.

@liamdebeasi liamdebeasi changed the title bug: WebKit choppy and freeze frames at Ionic6 React bug: datetime, showing calendar body after intersection observer callback causes issues on safari Jan 10, 2022
@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report labels Jan 10, 2022
@ionitron-bot ionitron-bot bot removed the triage label Jan 10, 2022
@elbugs
Copy link
Author

elbugs commented Jan 10, 2022

Thanks! I do not think that's the problem, well maybe it's part of it. Why?
Because with the ActionSheet animation happens the same behaviour :/

@liamdebeasi
Copy link
Contributor

What version of Safari are you testing this on? It might be a matter of device performance or a Safari bug.

@elbugs
Copy link
Author

elbugs commented Jan 10, 2022

I'm on Safari 15.1 (17612.2.9.1.20) in Mac OS Monterey 12.0.1 (21A559). I don't think so because in the iPhone when compiled with capacitor happens the same... That's the big problem here, the iOS performance, I can't ship the app with that performance. (Still work to do for about 5 months, but we have to find the way to solve it) I post this in the forums and had no reply, it's strange to me that nobody has the same bad performance, so maybe it's something from my side... I don't know what as it happens with a blank template. I'm open to work and help you in whatever you need to improve ionic even more, despite this issues I'm fall in love with it. Congrats and keep up the good work!!

@liamdebeasi
Copy link
Contributor

Are you able to help me better understand how to reproduce the performance issue? I cannot reproduce it on my end, and I am unable to see any animation jumping in the video in #24542 (comment).

@elbugs
Copy link
Author

elbugs commented Jan 11, 2022

It's like all the animations in general at the end of the animation breaks too hard, so that's why it does not look smooth. I can record a video, but the FPS are not helping to show the issue... :/

@elbugs
Copy link
Author

elbugs commented Jan 11, 2022

Okay, here is a comparative between native Objetive-c action sheet, and ionic6 with react.
Look at the end of the animation, you will see a little jump.
image

Thats what is happening with iOS theme. If I compile the app forcing the MD design, everything runs smooth.

https://vimeo.com/664717008/a6bf5a9b50

@elbugs
Copy link
Author

elbugs commented Jan 19, 2022

Hello. Are you able to see the performance issue now? Is there anything else I can help with? Thanks 🙂

@andrerds
Copy link

I'm on Safari 15.1 (17612.2.9.1.20) in Mac OS Monterey 12.0.1 (21A559). I don't think so because in the iPhone when compiled with capacitor happens the same... That's the big problem here, the iOS performance, I can't ship the app with that performance. (Still work to do for about 5 months, but we have to find the way to solve it) I post this in the forums and had no reply, it's strange to me that nobody has the same bad performance, so maybe it's something from my side... I don't know what as it happens with a blank template. I'm open to work and help you in whatever you need to improve ionic even more, despite this issues I'm fall in love with it. Congrats and keep up the good work!!

Hey man, how are you?
I'm having the same problem, apart from that there are some strange behaviors using the time, sometimes it selects it right, sometimes it has the time zeroed out of the performance that was shown there.
I had to resort to other date pickers, until this date time was better.

I liked it a lot, but with these gaps and delays it was really bad.
Thanks man, you're not alone.

@liamdebeasi
Copy link
Contributor

I finally found the issue with the animations. Applying an easing to the overall animation effect (like Ionic does) does not produce the same visual results as applying an easing to the first of two keyframes. While effect and keyframes animations are different, both should produce the same visual results in the case of an animation with 2 keyframes.

I reported this to Apple (https://bugs.webkit.org/show_bug.cgi?id=241020) and will look into a possible workaround in Ionic.

@liamdebeasi
Copy link
Contributor

This issue is dedicated to an issue in datetime and is unrelated to the animation issues being reported on this thread. As a result, I created #25369 which will track updates on the animation issue.

When the datetime issue is resolved, this thread will be closed out. Please follow the linked thread for updates on the animation issue.

@elbugs
Copy link
Author

elbugs commented Jun 12, 2022

Hey Liam, one quick question, why this is not happening with Angular and how is it possible that no one has detect this before? Looks weird to me that we are in version 6 of ionic with this issue that seems stupid, but affects the overall look of the apps, making a sensation of not a native app. Thanks.

EDIT: Also, looking at bugzilla seems that Apple is not considering this something important, from their last comment of 12 days ago...

@liamdebeasi
Copy link
Contributor

If this is an important bug for you, I recommend posting on the WebKit issue thread with a link to an app of yours running in production that displays this issue. Apple needs to understand impact in order to prioritize issues. Just because they did not fix the issue immediately does not mean they do not consider it to be important -- they may just not have a good understand of impact quite yet.

@elbugs
Copy link
Author

elbugs commented Aug 25, 2022

I know it is a bug from WebKit but my question is, why this is. It happening with angular for example? Or in Inter words, all the apps made with ionic and react are facing this issue and from ionic aren't offering any solution? I consider that a workaround from your side or at least a guide on how to solve it would be helpful. In the end it's the image of ionic.

@capc0
Copy link
Contributor

capc0 commented Oct 5, 2022

has there been any update on this? explicitly the delayed animation for the datetime picker.

@liamdebeasi
Copy link
Contributor

Updates on the animation issue are being tracked in #25369.

@hedinasr
Copy link

hedinasr commented Mar 4, 2024

any news or a workaround on this issue ?

@liamdebeasi
Copy link
Contributor

Hi everyone,

Here is a dev build with a proposed fix: 7.8.1-dev.11710449785.14ebd5a0. This patch should resolve the original issue where the calendar body does not show until the modal animation finishes on iOS.

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

---------

<!-- 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. -->

WebKit has a quirk where IntersectionObserver callbacks are delayed
until after an accelerated animation finishes if the "root" specified in
the config is the browser viewport (the default behavior if "root" is
not specified) This means that when presenting a datetime in a modal on
iOS the calendar body appears blank until the modal animation finishes.

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

- We can work around this issue by observing an element inside of the
datetime component and using the datetime component itself as the root.
To do this, I added an `.intersection-tracker` element inside of
datetime. This element has a dimension of 0x0 so it should not affect
component layout or functionality.

I opted to add this element instead of re-using an existing element
because the existing elements are not guaranteed to always be in the DOM
due to different datetime presentation styles.

| `main` | branch |
| - | - |
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/e84d111d-b156-4f45-887a-d68a1097e5dd"></video>
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/3dccf1e5-cf79-46ab-b542-0537fd46fa76"></video>
|


## 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.8.1-dev.11710449785.14ebd5a0`

---------

Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #29163, 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.

Copy link

ionitron-bot bot commented Apr 14, 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 Apr 14, 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.

5 participants