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

show-page class not always added to pushed pages #8769

Closed
andersfa87 opened this issue Oct 18, 2016 · 23 comments
Closed

show-page class not always added to pushed pages #8769

andersfa87 opened this issue Oct 18, 2016 · 23 comments
Assignees
Milestone

Comments

@andersfa87
Copy link

Short description of the problem:

So I have only experienced this on iOS devices, it happens very rarely on an iPhone6 but almost every time on an iPhone4S.

I tried to install the Ionic Conference app to see if I could recreate it, and after clicking around the app a bit it happened. I guess it happens more often in my own app because there is more content on the pushed page.

http://i.imgur.com/mmaJnyr.jpg - iPhone4S screenshot
http://i.imgur.com/vhTtgws.png - Inspector screenshot

What behavior are you expecting?

That a pushed page is always shown, even on old / low end devices.

Steps to reproduce:

  1. Install Ionic Conference App on an iPhone4S
  2. Click around for a few minutes,
  3. Boom

Which Ionic Version? 1.x or 2.x
Ionic 2 RC.1

@manucorporat
Copy link
Contributor

I have recently fixed a lot of blank page issues in NavController due inconsistent states.
These inconsistent states are usually caused by race conditions.

We will release RC2 soon!

@infinnie
Copy link

A similar problem occurs on my Samsung Galaxy S II where pushed pages are not shown and the screen is flushed white. I did not inspect the elements however.

@manucorporat
Copy link
Contributor

Can you guys try against nightly?

npm install ionic-angular@nightly --save

@andersfa87
Copy link
Author

Hey Manu,

The problem still occurs on my iPhone 4s, can't reproduce it anymore on my iPhone6s. Then again it was very rare on the 6s before aswell.

Which commit did you try and fix the issue in?

@MykolaShevchuk
Copy link

Installed:
npm install ionic-angular@nightly --save

But also still have the same issue on old iPhones (4s, 5), iPad mini and on android's as well.

I'm getting the black screen after this.navCtrl.push(FormPage).
As temporary solution helps removing animation:
this.navCtrl.push(FormPage, {}, {'animate':false});

@peterbakonyi05
Copy link

Here is a plunker that explains how you can easily replicate it:
https://run.plnkr.co/plunks/uK8wAQ/
https://plnkr.co/edit/uK8wAQ?p=info

We will report another issue, but I though I will link it here. I hope it helps some of you who are experiencing the white screen issue.

Potential solutions:

  1. don't do long running sync calculation on a mobile device, do the logic on the server
  2. use web workers
  3. do the calculation before or after navigating

@infinnie
Copy link

infinnie commented Nov 8, 2016

This problem seems to occur more frequently in RC2…

@infinnie
Copy link

infinnie commented Nov 8, 2016

@brandyscarney @jgw96 2 possible dups here.

@ataraxus
Copy link
Contributor

ataraxus commented Nov 8, 2016

After reviewing this i would definitely see my issue #9026 is a dup. so it still occurs on RC2 :(

@ataraxus
Copy link
Contributor

ataraxus commented Nov 8, 2016

@peterbakonyi05 sadly i must say I'm experiencing this issue on pages which are nearly static. I do no calculations in the constructor or hooks.
@manucorporat since I'm already on RC2 do you think it makes sense for me to try nightly?

@peterbakonyi05
Copy link

I had the same impression, that RC2 is actually making it happen more frequently.

@ataraxus That is strange, because in our case it only happens when we navigate to a page that contains some expensive calculations. Are you sure that you don't use any third-party libraries that does some heavy stuff? Maybe try to profile it with Chrome if you haven't tried it yet. What kind of device are you using for testing?

@ataraxus
Copy link
Contributor

ataraxus commented Nov 8, 2016

@peterbakonyi05 i'm pretty sure, but maybe I'm wrong anyway. please have a look at my ticket #9026 I've pasted my component there, with which i trigger this issue. the only suspicous part in my eyes is: @ViewChild('letterBar') letterBar: ElementRef;
But maybe I'm missing something else. It seems to be a racecondition and computing load enforces the problem, but i think maybe also low computing power as with my devices. maybe the general angualr and ionic magic is sufficient on low powered devices to trigger this issue.

Just too many maybes :(

I would be glad to help on this issue and rule out some of these maybes....

@manucorporat
Copy link
Contributor

I am 90% sure this problem will be closed in next release once this PR is merged:
#9033

@manucorporat manucorporat added this to the 2.0.0-rc.3 milestone Nov 8, 2016
@ataraxus
Copy link
Contributor

ataraxus commented Nov 8, 2016

@manucorporat could i test this changes by using npm i ionic-angular@latest ? Or do i have to cherry pick it? Then we would know for sure if it fixes it.

edit: ah i see/understand just right now it is not merged yet. I will try to patch my rc2 manually and see if it fixes the issues.

@ataraxus
Copy link
Contributor

ataraxus commented Nov 9, 2016

Hey @manucorporat I did include your PR manually, it didnt help much, or anything.

Now it looks like this

 Animation.prototype.play = function (opts) {
        var _this = this;
        var dur = this.getDuration(opts);
        this._isAsync = this._hasDuration(opts);
        this._clearAsync();
        this._playInit(opts);
        this._beforeReadFn();
        this._beforeWriteFn();
        if (this._isAsync) {
            this._asyncEnd(dur, true);
        }
        this._raf && this._raf(function () {
            _this._raf && _this._raf(_this._playDomInspect.bind(_this, opts));
        });
    };

At least now i understand why pages with loading bar do have a bigger chance to be displayed. the loading overlay triggers the validation of the DOM so the chance is higher to win the race ...

Anything I can test?

@ataraxus
Copy link
Contributor

ataraxus commented Nov 9, 2016

@manucorporat Hey i started poking around in ionics source and adding tracing information. I could trace the issue down somewhat at least. for whatever reason animations get lost: this is a trace from a working transition:

Transition.prototype.start
animation.js:42 _addEle: 
animation.js:42 _addEle: loading-wrapper
animation.js:173 _asyncEnd
transition.js:24  Transition.prototype.start
animation.js:42 _addEle: ion-page
animation.js:42 _addEle: cem-style-toolbar toolbar toolbar-md toolbar-md-rewered
animation.js:42 _addEle: back-button disable-hover bar-button bar-button-md back-button-md bar-button-default bar-button-default-md
animation.js:173 _asyncEnd
2xanimation.js:176 _this._playDomInspect.bind
animation.js:194 _this._playDomInspect.run: num Animation 2
animation.js:202 _playProgress: ion-backdrop
animation.js:202 _playProgress: div
animation.js:194 _this._playDomInspect.run: num Animation 3
animation.js:202 _playProgress: highscore
animation.js:353 _before: highscore 1 
animation.js:361 _before add : show-page
animation.js:202 _playProgress: ion-navbar
animation.js:353 _before: highscore 1 
animation.js:202 _playProgress: button
animation.js:353 _before: highscore 1 
animation.js:361 _before add : show-back-button
transition.js:24  Transition.prototype.start
animation.js:42 _addEle: 
animation.js:42 _addEle: loading-wrapper
animation.js:173 _asyncEnd
animation.js:176 _this._playDomInspect.bind
animation.js:194 _this._playDomInspect.run: num Animation 2
animation.js:202 _playProgress: ion-backdrop
animation.js:202 _playProgress: div

and this from a broken one:

Transition.prototype.start
animation.js:42 _addEle: 
animation.js:42 _addEle: loading-wrapper
animation.js:173 _asyncEnd
transition.js:24  Transition.prototype.start
animation.js:42 _addEle: ion-page
animation.js:42 _addEle: cem-style-toolbar toolbar toolbar-md toolbar-md-rewered
animation.js:42 _addEle: back-button disable-hover bar-button bar-button-md back-button-md bar-button-default bar-button-default-md
animation.js:173 _asyncEnd
2xanimation.js:176 _this._playDomInspect.bind
transition.js:24  Transition.prototype.start
animation.js:42 _addEle: 
animation.js:42 _addEle: loading-wrapper
animation.js:173 _asyncEnd
2xanimation.js:194 _this._playDomInspect.run: num Animation 0
animation.js:176 _this._playDomInspect.bind
animation.js:194 _this._playDomInspect.run: num Animation 2
animation.js:202 _playProgress: ion-backdrop
animation.js:202 _playProgress: div

the interesting part is that in the case of the broken transition the second transition is started before the first could be executed completely. Or the first transition's animations are both empty for whatever reason and the second transisition is executed therefore imediately.

hopefully this helps to figure out the issue.

edit: these traces were made with the patch you proposed.

@ataraxus
Copy link
Contributor

ataraxus commented Nov 9, 2016

Okay i got it now! and hacked it so i wont be emberassed at the Exec presentation tomorrow 😂
After adding even more tracing:
Working Part:

this._trnsTm: 1478700787612
nav-controller-base.js:677 this._trnsTm: 1478700787745
animation.js:42 _addEle: 
animation.js:42 _addEle: loading-wrapper
nav-controller-base.js:677 this._trnsTm: 1478700783011
animation.js:173 _asyncEnd
animation.js:42 _addEle: ion-page
animation.js:42 _addEle: cem-style-toolbar toolbar toolbar-md toolbar-md-rewered
animation.js:42 _addEle: back-button disable-hover bar-button bar-button-md back-button-md bar-button-default bar-button-default-md
nav-controller-base.js:677 this._trnsTm: 1478700783215
animation.js:173 _asyncEnd
2animation.js:176 _this._playDomInspect.bind
animation.js:194 _this._playDomInspect.run: num Animation 2
animation.js:202 _playProgress: ion-backdrop
animation.js:202 _playProgress: div
animation.js:194 _this._playDomInspect.run: num Animation 3
animation.js:202 _playProgress: highscore
animation.js:356 _before: highscore 1 
animation.js:364 _before add : show-page
animation.js:202 _playProgress: ion-navbar
animation.js:356 _before: highscore 1 
animation.js:202 _playProgress: button
animation.js:356 _before: highscore 1 
animation.js:364 _before add : show-back-button
animation.js:229 prototype._asyncEnd onTransitionEnd
animation.js:539 .prototype._didFinishAll this._didFinish
nav-controller-base.js:458 this._didEnter(transition.enteringView);
nav-controller-base.js:466 this._cleanup(transition.enteringView
nav-controller-base.js:471 prototype._trnsFinish: enteringNameLoadingCmp undefined true
nav-controller-base.js:677 this._trnsTm: 0
animation.js:229 prototype._asyncEnd onTransitionEnd
animation.js:539 .prototype._didFinishAll this._didFinish
nav-controller-base.js:458 this._didEnter(transition.enteringView);
nav-controller-base.js:463 this._didLeave(transition.leavingView);
nav-controller-base.js:466 this._cleanup(transition.enteringView
nav-controller-base.js:471 prototype._trnsFinish: enteringNameHighScorePage HomePage true
nav-controller-base.js:677 this._trnsTm: 0
nav-controller-base.js:677 this._trnsTm: 1478700790367
animation.js:42 _addEle: 
animation.js:42 _addEle: loading-wrapper
nav-controller-base.js:677 this._trnsTm: 1478700785606
animation.js:173 _asyncEnd
animation.js:176 _this._playDomInspect.bind
animation.js:194 _this._playDomInspect.run: num Animation 2
animation.js:202 _playProgress: ion-backdrop
animation.js:202 _playProgress: div
animation.js:229 prototype._asyncEnd onTransitionEnd
animation.js:539 .prototype._didFinishAll this._didFinish
nav-controller-base.js:463 this._didLeave(transition.leavingView);
nav-controller-base.js:466 this._cleanup(transition.enteringView
nav-controller-base.js:471 prototype._trnsFinish: enteringNameundefined LoadingCmp true
nav-controller-base.js:677 this._trnsTm: 0

broken part

nav-controller-base.js:677 this._trnsTm: 1478700809741
nav-controller-base.js:677 this._trnsTm: 1478700809803
animation.js:42 _addEle: 
animation.js:42 _addEle: loading-wrapper
nav-controller-base.js:677 this._trnsTm: 1478700805032
animation.js:173 _asyncEnd
animation.js:42 _addEle: ion-page
animation.js:42 _addEle: cem-style-toolbar toolbar toolbar-md toolbar-md-rewered
animation.js:42 _addEle: back-button disable-hover bar-button bar-button-md back-button-md bar-button-default bar-button-default-md
nav-controller-base.js:677 this._trnsTm: 1478700805185
animation.js:173 _asyncEnd
2animation.js:176 _this._playDomInspect.bind
animation.js:237 prototype._asyncEnd onTransitionFallback
animation.js:539 .prototype._didFinishAll this._didFinish
nav-controller-base.js:458 this._didEnter(transition.enteringView);
nav-controller-base.js:466 this._cleanup(transition.enteringView
nav-controller-base.js:471 prototype._trnsFinish: enteringNameLoadingCmp undefined true
nav-controller-base.js:677 this._trnsTm: 0
animation.js:237 prototype._asyncEnd onTransitionFallback
animation.js:539 .prototype._didFinishAll this._didFinish
nav-controller-base.js:458 this._didEnter(transition.enteringView);
nav-controller-base.js:463 this._didLeave(transition.leavingView);
nav-controller-base.js:466 this._cleanup(transition.enteringView
nav-controller-base.js:471 prototype._trnsFinish: enteringNameHighScorePage HomePage true
nav-controller-base.js:677 this._trnsTm: 0
2animation.js:194 _this._playDomInspect.run: num Animation 0
nav-controller-base.js:677 this._trnsTm: 1478700812278
animation.js:42 _addEle: 
animation.js:42 _addEle: loading-wrapper
nav-controller-base.js:677 this._trnsTm: 1478700807490
animation.js:173 _asyncEnd
animation.js:176 _this._playDomInspect.bind
animation.js:194 _this._playDomInspect.run: num Animation 2
animation.js:202 _playProgress: ion-backdrop
animation.js:202 _playProgress: div
animation.js:229 prototype._asyncEnd onTransitionEnd
animation.js:539 .prototype._didFinishAll this._didFinish
nav-controller-base.js:463 this._didLeave(transition.leavingView);
nav-controller-base.js:466 this._cleanup(transition.enteringView
nav-controller-base.js:471 prototype._trnsFinish: enteringNameundefined LoadingCmp true
nav-controller-base.js:677 this._trnsTm: 0

One can see that the transition actually fails with: prototype._asyncEnd onTransitionFallback due to a timeout defined here:

self._tm = nativeTimeout(onTransitionFallback, (dur + TRANSITION_END_FALLBACK_PADDING_MS));
...
var TRANSITION_END_FALLBACK_PADDING_MS = 400;

I guess the reason behind this is to stop animations taking to much time on slow devices. The problem is, that the show-page class is set in these animations, so the transition will stop and the user will see a blank page.

I would propose to add the show-page class to the page element in the onTransitionFallback so that the user is still able to use the app, even without fancy transitions.

My WORKAROUND for now is to increase the time waiting for the transition to finish:

node_modules/ionic-angular/animations/animation.js
var TRANSITION_END_FALLBACK_PADDING_MS = 800;

@manucorporat
Copy link
Contributor

manucorporat commented Nov 9, 2016

@ataraxus awesome debugging! but I don't understand how the PR didn't fix the issue for you:
show-page is added as a result of _beforeReadFn().
https://github.com/driftyco/ionic/blob/ba557acb4fbad8040f7cd4e5f537901729197ca5/src/transitions/page-transition.ts#L15

So ensuring _beforeReadFn() is always called ensures the class is added.
am I missing something?

UPDATE #1:
I just saw my mistake! updating PR! Your help has been incredibly valuable. I will also make the TRANSITION_END_FALLBACK_PADDING_MS a little longer.

@ataraxus
Copy link
Contributor

ataraxus commented Nov 9, 2016

glad i could help. keep up the good work!

@ataraxus
Copy link
Contributor

@manucorporat I was unable to figure out where to get your updated PR I would be willing to test your changes on my devices, so we can be "sure" it works as expected. Cheers!

@manucorporat
Copy link
Contributor

@ataraxus just updated the PR, it is WIP though
#9033

manucorporat added a commit to manucorporat/ionic that referenced this issue Nov 11, 2016
manucorporat added a commit to manucorporat/ionic that referenced this issue Nov 11, 2016
manucorporat added a commit to manucorporat/ionic that referenced this issue Nov 11, 2016
manucorporat added a commit to manucorporat/ionic that referenced this issue Nov 14, 2016
@dm-grinko
Copy link

I am sorry for stupid question but
How to implement this in my ionic app?

node_modules/ionic-angular/animations/animation.js
var TRANSITION_END_FALLBACK_PADDING_MS = 800;

@ionitron-bot
Copy link

ionitron-bot bot commented Sep 2, 2018

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 Sep 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants