-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
Animation on page transition on iOS is broken #17649
Comments
There may be an open PR for this already: |
Hi there, Thanks for the issue! Would you be able to provide a repository with the code required to reproduce this issue? Additionally, which devices/iOS versions have you been testing on? Thanks! |
It seems like upgrade to Ionic 4.1.0 solved the issue. If it means anything I tested on iPhone 8 Plus iOS 12.1.4 |
I'm still seeing this on 4.1.0. You can sometimes see the issue if you limit the CPU power and navigate into a page using Chrome DevTools. Finding it hard to consistently reproduce the issue though. |
I am willing to help but I'd have a problem reproducing error now that my code works. Maybe if @rossholdway shares code and I can see if there is a difference to my code. |
I just wanted to confirm that it started to happen again... not sure why it was working good for 1 day. I will try to create repository that reproduces the problem. |
First I added When loading pages I usually protect components from showing up before data is loaded with variable When I removed It'd be great if @rossholdway or somebody else to test this. I am not able to reproduce it on a fresh app yet, but I did not try to make API call. I tried to simulate loading with setTimeout which is not the same. |
@liamdebeasi I can substitute ion-buttons to be just a div and the problem persists. I would like to think that any element within the toolbar should transition with the rest of the elements in that toolbar. It looks peculiar to see a header and the rest of the ion-content do a slide transition while the buttons appear with no slide and sit statically while the transitions occurs. |
@liamdebeasi @jayordway It's not a problem just with toolbar. when I put |
@aces-tm Are you testing with the new development branch of ionic though? |
@jayordway no, should I update to @latest or something else? |
Ah, I was looking at the proposed PR for this and read this note: |
Hi everyone, We have shipped some updates to the page transitions on iOS. Please try updating to Ionic 4.1.2 ( Thanks! |
|
@liamdebeasi i'm not really convinced that this is the real issue here. this was working before in the same app without any issue (can't tell for sure now when it started, it definitely still worked with Ionic 3). also it doesn't look like only being about performance - it looks more like the animation is interrupted at some point, like if some events or animations interfere with each other. additionally the issue doesn't go away on a super fast dev machine. using *ngIf on a transitioning page should work and did work. it's not just about preloading data but about how we setup angular templates and components. delaying observables, async-pipes / ngIf until a page has finished entering (or even being forced to not use those basic things anymore if you want a fast UI) can't really be the solution for Ionic going forward. |
@liamdebeasi , thank you for the detailed response. I understand the general concept, but I still think there might be an issue here. In my case, I am e.g. switching from a list of clients to the detailed view of a client. Client data (about 20 datapoints, so nothing major) is fetched in a GraphQL query. However, as soon as the data is received, the animation interrupts (if I using observables and the async pipe). The client detail template is not large, and there is exactly one |
@chriswep So for some context as to what's happening under the hood -- We are using CSS Animations/Web Animations (depending on browser support) to create these navigation animations. We then pass that work off to the browser to run/render. We do not run these animations in a Additionally, the iOS navigation transition animates the I agree that there's a lot more than just using *ngIf/a hidden class. My examples were a subset of different optimizations you can make (I don't remember them all off the top of my head 🙂). If you can provide a working example of this running well in an Ionic 3 app I'm more than happy to take a look. edit: I also wanted to clarify that I didn't intend to suggest you never use things like *ngIf, I just meant that there's a time and a place for everything. For example, if you don't need an element on the page it's a great idea to use *ngIf to remove it from the DOM; however, the act of removing an element from the DOM is an expensive operation by itself so it's important to be mindful of what you're doing as well as how often you're doing it. |
@iherger Can you provide a Timeline profile from your app? It's hard to say what's going on without seeing the data itself. In Safari Dev Tools, click the "Timelines" tab and click the red circle in the top left to start recording. When done, click the square button in the top left. Then, click the "Export" button and attach the file to this thread. I'm mostly curious to see if there are any additional optimizations we could make in Ionic Framework apart from the animations. Of course a working reproduction would be ideal, but it sounds like there's a server side component here so I'm not sure if that would be possible from your end. |
@liamdebeasi thanks for looking into this. Attached is a timeline. There was noticeable interruption in the transition. |
@iherger Thanks for the timeline data! I’ve identified two points during the transition that likely cause your lag: The call trees for “Click Event Dispatched” and “Load Event Dispatched” both take quite a long time. For context, for a 60fps animation, each frame should take ~16.7ms, and the entire operations for these are taking 86.11ms and 29.02ms respectively. As a result, these tasks are being spread out across multiple frames since they cannot be completed in 16.7ms. This is where your animation lag is going to come into play. You can also visualize this in terms of frames: Notice that there are 3 times that frames drop below 30fps. Presumably, these are the 3 events you see in the first image. For context, the 2nd event “Full Garbage Collection” is just cleanup and appears to happen after the animation has completed, so I’m not too concerned about that (I expect there needs to be some cleanup once navigation has completed). I am also assuming that the network requests (the light blue parts on the timeline) are your GraphQL results. If you right click the first event and click “Expand All” you will notice quite a bit of processing is happening on the Angular side. I won’t post the screenshot here since the call tree is quite large, but you should notice calls from Angular zone, Angular Core, Angular Router, utility functions, and others. I did not see any Ionic-specific tasks taking up more than ~1ms of processing time. Ionic Angular is typically the You can also see that after the first event (which is the biggest purple block on the timeline) there are several Paint, Composite, and Style recalculations going on (likely due to Angular change detection trying to rerender components). It’s hard to make definitive conclusions without seeing the actual code and running the app, but as of now nothing from the Ionic side sticks out as contributing significantly to the lag you are experiencing. Hope this clears things up! |
@liamdebeasi thank you for all the background information. i'm also sure that Ionic does a great job at optimising everything for good performance. i was just suggesting the possibility that this is not related to performance or (missing) optimisation but that this is (simply) a bug. of course i might be wrong here. however since i feel i can't simply settle on this being the status quo now, i took the effort to create two blank Ionic projects, one with Ionic 4 and one with version 3. I added a second page "Page2" and a button on the home page that navigates there. "Page2" has this in the controller:
and this in the template:
The Ionic 4 version clearly shows the issue (tested on Safari Reponsive Design mode on a fast dev machine, as well as iOS Sim), the Ionic 3 version works just fine. |
Oh yeah, there certainly could be a bug! Are you able to share those 2 projects with me? |
|
Thanks for the follow up. I am able to reproduce the lag in the v4 version of the app in Safari. The lag appears to come from NgZone running in the timeout. The v3 app uses Angular 5.x, but the v4 app uses Angular 8.x so it's hard to say what is different in Angular/NgZone that is causing this change. To demonstrate that this lag is related to Angular/NgZone, you can try the following: import { NgZone } from '@angular/core';
...
constructor(private zone: NgZone) {}
...
ngOnInit() {
this.zone.runOutsideAngular(() => {
setTimeout(() => {
this.showDiv = true;
}, 50);
});
} By running outside of NgZone the lag should disappear entirely. I'm still digging through all of the NgZone traces when running a Timeline recording on Safari, so I'll post any additional information here when I have it. |
That is probably because then the *ngIf is not triggered which is what effectively causes the ionic animation to be interrupted. Also, if you leave the controller code as is and just replace *ngIf with [hidden], all is fine. |
Right, so ngIf will add/remove the div from the DOM whereas the hidden directive will just add "display: none" to the div. The DOM removal/addition is typically more expensive than just changing the display. |
The one thing that’s interesting is I can't reproduce this in Chrome (or any other browser) on the same machine. I wonder if we're hitting some edge case in Safari? Or maybe since Chrome supports css containment it's not as much of an issue? |
(Just to add to this, I have been able to repo this in Chrome in my own app by dropping the CPU performance to its slowest in devtools) |
Ah interesting. I have CPU set to 6x slowdown in dev tools and the performance still seems pretty good. I'll keep digging around. |
So I wanted to try one thing. Can you do the following and let me know if the performance issue goes away or persists in Safari?
ion-content {
position: absolute;
top: 0;
bottom: 0;
left: 0;
right: 0;
.inner-scroll {
padding-top: 44px;
background: white;
height: 100%;
}
}
You app content will look a little funky, but I'm mostly interested in seeing if the performance issue went away. If it does, I have a theory as to why. |
the transition runs without issues with this (at least on Safari) |
Agreed. Tested this in Safari on iOS with the example @chriswep provided. The transition runs without issue after installing the dev build (and adding the CSS). 🎉 What's your theory / fix @liamdebeasi |
My current guess is the performance issue you were seeing is actually related to this bug in WebKit: https://bugs.webkit.org/show_bug.cgi?id=201048 Animations in the Shadow DOM tend to freeze/be janky when the layout is invalidated. In this case I am discussing with the team as to whether there may be an appropriate workaround we could go with for now. |
Any update Liam? Thanks! |
Liam, as developers, we can't move forward with iOS builds until this issue gets resolved. There are a few bandaids available but the deterioration in the performance of the app makes it almost unusable for clients. This issue has been hanging out there for 9 months now, when can we give birth to a permanent and complete solution? |
Hi @pvroom, Unfortunately this is not an issue with Ionic, but rather an issue in WebKit/Safari. We have reached out to some contacts on the WebKit team to see if they can take a look at the issue. The best thing I can recommend is to hold off on updating the view until the animation ends. I realize this is not an ideal workaround, but there is not much we are able to do from the Ionic Framework-side as of now. I will update this thread when I have more info to share. Thanks! |
Hi Liam, |
We appreciate the feedback! We moved to the Shadow DOM because of the performance and encapsulation improvements it brings. This move also allowed us to dynamically theme your app at runtime without having multiple CSS bundles. Having multiple bundles slowed down build times, startup times, etc. You can learn more about the benefits of the Shadow DOM here: https://developers.google.com/web/fundamentals/web-components/shadowdom. I understand that this is frustrating, and we are eager to find a good resolution to this issue. Additionally, I have provided a workaround above that should allow you to avoid this performance issue entirely. I am going to lock this thread for now. I think the current state of the issue is pretty clear, and I don't want to add any additional unwanted emails to the inboxes of other users who have interacted with this thread. Thanks! |
Hi everyone, I wanted to provide an update regarding this issue. I am closing this thread as the main issue has been resolved in WebKit (https://bugs.webkit.org/show_bug.cgi?id=201048). The Ionic Team does not know exactly when this fix will be available as that is not under our control. For any other bugs, please open a new issue. Thanks! |
Quick update: The WebKit issue I linked to has been fixed and released as part of the latest Safari Technology Preview (release 101 at the time of writing). As it turns out, this bug was not solely related to the Shadow DOM as the WebKit team was able to reproduce this issue outside of the Shadow DOM. We do not have a timeline as to when the fix will become available for all users as that release schedule is controlled by Apple. |
Bug Report
Ionic version:
[x] 4.x
Current behavior:
Page transition is breaking on iOS. Android is working good.
Expected behavior:
Transition smooth onto next page.
Steps to reproduce:
Related code:
Typically I'd use
this.navCtrl.navigateForward(['route', id]);
to navigate to the next page, but 90% of time the animation would stop.Other information:
Ionic info:
The text was updated successfully, but these errors were encountered: