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

[v4.x] general page transition issues and inconsistencies in animation logic #17728

Closed
olivermuc opened this issue Mar 8, 2019 · 6 comments
Closed
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@olivermuc
Copy link

olivermuc commented Mar 8, 2019

Bug Report

This is meant to cover all current issues with page transitions. Standard pages (with extended ion-header components) and full screen pages with background images.

Since this is a lingering issue imho, I attempt to consolidate the issues and be as thorough as I can be. Find git repos (branches) for all tests below.

ISSUE 1a: ion-header > ion-toolbar

Ionic version:
[x] 4.x

Current behavior:
<ion-toolbar> only selectively animates content, not allowing additional <ion-toolbar> elements to be properly animated during page transition.

The below code currently looks like this:

2-ion-toolbars
Note: 2nd ion-toolbar not animated,

Expected behavior:
This should be allowed and animated properly:

<ion-header>
    <ion-toolbar>
        <ion-title>Page 1</ion-title>
    </ion-toolbar>
    <ion-toolbar>
        <ion-searchbar></ion-searchbar>
    </ion-toolbar>
</ion-header>

Steps to reproduce:
Just rebuild below git repo https://github.com/olivermuc/pages-with-background-images/tree/standard-setup-with-custom-header

Related code:
https://github.com/olivermuc/pages-with-background-images/tree/standard-setup-with-2-ion-toolbars

Related source code
Here is where the animation logic handles ion-toolbar differently for headers & footers, why??
It'd be much cleaner if it simply would animate the entire ion-header, like it does for ion-footer

https://github.com/ionic-team/ionic/blob/ad20bd6a70088767644ee21f84851e3619ebb0d5/core/src/utils/transition/ios.transition.ts#L43

Ionic info:

Ionic:
   ionic (Ionic CLI)             : 4.11.0 (/Users/.../.nvm/versions/node/v10.13.0/lib/node_modules/ionic)
   Ionic Framework               : @ionic/angular 4.1.1
   @angular-devkit/build-angular : 0.10.7
   @angular-devkit/schematics    : 7.0.7
   @angular/cli                  : 7.0.7
   @ionic/angular-toolkit        : 1.2.0

Cordova:
   cordova (Cordova CLI) : 8.1.2 (cordova-lib@8.1.1)
   Cordova Platforms     : not available
   Cordova Plugins       : not available

System:
   Android SDK Tools : 26.1.1 (/Users/.../Library/Android/sdk)
   ios-deploy        : 1.9.4
   NodeJS            : v10.13.0 (/Users/.../.nvm/versions/node/v10.13.0/bin/node)
   npm               : 6.7.0
   OS                : macOS Mojave
   Xcode             : Xcode 10.1 Build version 10B61

ISSUE 1b: ion-header > div

Using DIV in ion-header (instead of 2nd ion-toolbar to match animation content selector)

Same general setup as above, but see this branch for code changes: ie. swapping out 2nd ion-toolbar for div.

GIT repo branch:
https://github.com/olivermuc/pages-with-background-images/tree/standard-setup-with-custom-header

Current behavior:
ion-header-with-div-comp
This doesn't look right either! Watch the red <ion-searchbar> lingering around too long!

ISSUE 2a: host: background: ...

Full-screen background images - regardless of whether it is deemed best practice or not, imho this is a pretty common request (login screens, etc).

Other open posts on same topic:
#17494
#16678
#16639

Current behavior:
Implementing it using the host: object as recommended by @mhartington isn't satisfying as the page itself is not animated:

full-screen-background-image-on-host
Note: Page bg flickers, no animation.
Unless the page host is also animated, this approach doesn't work!

GIT repo branch:
https://github.com/olivermuc/pages-with-background-images/tree/full-screen-background-on-host

ISSUE 2b: ion-content: --background:

Current behavior:
Implementing the bg image using CSS variable --background: url();:
full-screen-background-image-on-ion-content
Note: Page bg flickers (on iOS!), UX wise, not acceptable
Note2: This requires the

GIT repo branch:
https://github.com/olivermuc/pages-with-background-images/tree/full-screen-background-on-ion-content

Workaround: ion-content: background:

The below now does what would be expected, but requires ugly hacks in the CSS, (see also git branch) eg:

:host {
    ion-header {
        position: absolute;
        top: 0;
        ion-toolbar {
            --border-width: 0;
            --background: none;
        }
    }
    ion-content {
        --background: none;
        background: url(../../assets/imgs/page-two.jpg) 0 0 no-repeat;
        --offset-top: -44px !important;
        --offset-bottom: -44px !important;
    }
    ion-footer {
        position: absolute;
        bottom: 0;
    }
}

(Above code from git repo, but reduced to a minimum)

Current behavior:
Implementing the bg image using CSS' (not the variable!) background: url(); and absolute positioning of header/footer:
full-screen-background-image-on-ion-content-direct
Note: Entire page animates nicely, even on iOS.

GIT repo branch:
https://github.com/olivermuc/pages-with-background-images/tree/full-screen-background-on-ion-content-direct

TL;DR & Ask:

ISSUE 1: ion-header/ion-toolbar animation
Can we allow for any ion-toolbar content to be properly animated, just like we do for ion-footer?

ISSUE 2: full-screen page bg images
Can we officially support full screen page image backgrounds?

  • Either by animating the host element background as well (unlikely I guess)?
  • Or by allowing more control over the ion-content > inner-scroll element?
    • eg via passthrough of background: url() not using CSS vars!
    • and supporting custom content offsets to avoid text under ion-header/ion-footer?
@liamdebeasi
Copy link
Contributor

liamdebeasi commented Mar 12, 2019

Hi there,

Thanks for opening an issue with us!

Issue 1 has a PR that I am currently cleaning up: #17224. I hope to merge this in soon. Feel free to test this PR and see if you notice any issues.

Issue 2 has a known (similar) issue as you noted: #17494. I will link the two together for when we look into background images in ion-content.

Thanks for using Ionic!

@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report labels Mar 12, 2019
@ionitron-bot ionitron-bot bot removed the triage label Mar 12, 2019
@fpassa
Copy link

fpassa commented Mar 22, 2019

Has this issue been fixed? If so, should we update what exactly on our ionic projects?

@liamdebeasi
Copy link
Contributor

Hi there,

Issue 1 has been resolved and a fix is available in Ionic v4.1.2. I will update this thread when issue 2 has been resolved.

Thanks!

@EnzoDLP
Copy link

EnzoDLP commented Apr 19, 2019

Any news on this issue ? :/ I have some white screen when i push a page which have a background image and an another bug which do a laggy animation when push a page (only on ios)

@liamdebeasi
Copy link
Contributor

liamdebeasi commented Apr 19, 2019

Hi there,

Since issue 1 was fixed in Ionic 4.1.2 and issue 2 is a duplicate of #17494, I am going to close this thread.

For any updates regarding issue 2, please feel free to follow the thread I just linked to.

If you are experiencing any other bugs, please open a new bug report.

Thanks!

@ionitron-bot
Copy link

ionitron-bot bot commented May 19, 2019

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 19, 2019
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

4 participants