ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
50.89k stars 13.52k forks source link

bug: CSS transform is not applied to popovers correctly in chrome after updating to ionic 5 #21244

Closed timKraeuter closed 4 years ago

timKraeuter commented 4 years ago

Bug Report

Ionic version:

[x] 5.x

Current behavior: I am applying (global.scss)

ion-popover {
    div.popover-content {
        top: 50% !important;
        left: 50% !important;
        transform: translate(-50%, -50%) !important;
    }
}

to my Popovers to center them as described in https://github.com/ionic-team/ionic/issues/15036. The CSS transform is not applied in chrome. It is applied in firefox tho. Expected behavior: CSS transform should apply consistently in firefox and chrome. Somehow it is not applying in chrome with ionic 5. In ionic 4 it worked.

Steps to reproduce:

  1. Apply transform in the global.scss as shown above.
  2. Open a popover.

Related code: Clone the prepared GitHub repository and have a look yourself with chrome and firefox. https://github.com/timKraeuter/popover-transform-ionic5

Other information: I have opened this thread in the forum: https://forum.ionicframework.com/t/popover-centering-not-working-after-ionic-4-to-5-update/189017

But I think it is a bug as it worked in ionic 4 but not in 5.

Ionic info:

Ionic:

   Ionic CLI                     : 5.4.4 (C:\Users\TimKräuter\AppData\Roaming\npm\node_modules\ionic)
   Ionic Framework               : @ionic/angular 5.1.0
   @angular-devkit/build-angular : 0.803.26
   @angular-devkit/schematics    : 8.3.26
   @angular/cli                  : 8.3.26
   @ionic/angular-toolkit        : 2.2.0

Utility:

   cordova-res : not installed
   native-run  : 1.0.0

System:

   NodeJS : v10.15.3 (C:\Program Files\nodejs\node.exe)
   npm    : 6.4.1
   OS     : Windows 10
liamdebeasi commented 4 years ago

Thanks for the issue. The popover seems to be centered properly in your repo. Can you double check to make sure you included the correct code?

image

timKraeuter commented 4 years ago

I'm sorry I have not phrased that clearly. The popover should be shifted to the left in chrome as it is in firefox. Here is a screenshot in firefox where the translate gets applied correctly. This also worked for chrome in ionic 4. popover_firefox

liamdebeasi commented 4 years ago

Hmm maybe I am not fully understanding what the issue is. Using the following code:

.popover-content {
  top: 50% !important;
  left: 50% !important;
  transform: translate(-50%, -50%) !important;
}

the popover should be centered horizontally and vertically.

I tried the CSS on a regular div and got the same result: https://codepen.io/liamdebeasi/pen/qBOoKOw

Is the screenshot linked in https://github.com/ionic-team/ionic/issues/21244#issuecomment-625853510 what you expect to see?

timKraeuter commented 4 years ago

I have updated the repo to use your code. Before I just had: .popover-content { transform: translate(-50%, -50%) !important; } Which should cause the popover to not be centered.

Now I am using the code you provided. But in chrome, the transform CSS is not applied. As you can see in this screenshot from chrome the popover is not centered:

grafik

In firefox, the popover is centered.

liamdebeasi commented 4 years ago

Thanks for the follow up. I was able to reproduce the behavior you are experiencing; however, I am not sure this is an Ionic Framework bug. I tested this in an older version of Ionic 4, and I got the same result as in Ionic 5. This appears to be a browser quirk between Chrome and Firefox.

Can you provide an example of this working in an Ionic 4 application with Chrome?

timKraeuter commented 4 years ago

https://github.com/timKraeuter/popover-transform-ionic5/tree/test worked for me. But I did not have the time to really clean the dependencies up atm.

liamdebeasi commented 4 years ago

Thanks for the follow up. I see what the difference is now.

With Ionic 5 we introduced a new more performant animation engine built on Web Animations. The old engine applied inline CSS styles that you could override. In your Ionic 4 example, setting transform: translate(-50%, -50%) worked because the animation's transform: scale(1) was applied inline, meaning you could use !important to override it.

When applying the scale via Web Animations, the animation's styles take priority and cannot be overridden using !important. As a result, trying to override the transform via an !important will not work anymore.

I am going to close this as this is how Web Animations work and is not a bug in Ionic Framework. If you would like to force the popover to be centered on the screen, the recommended approach is to provide a custom animation to the popover via the enterAnimation property that excludes the use of scale.

Here are docs on how to override a component's animations: https://ionicframework.com/docs/utilities/animations#overriding-ionic-component-animations

Here is a small example I made to show this working with the popover: https://codepen.io/liamdebeasi/pen/ExVEdNG

Thanks!

timKraeuter commented 4 years ago

For future readers. My working solution (Angular/Typescript) with a custom animation is:

public async showPopover() {
    const popover: HTMLIonPopoverElement = await this.popoverCtrl.create({
      component: PopoverTransformedComponent,
      backdropDismiss: true,
      enterAnimation: this.enterAnimation,
  });

    await popover.present();
  }

  private enterAnimation = (baseEl: any) => {
    const backdropAnimation = this.animationCtrl.create()
      .addElement(baseEl.querySelector('ion-backdrop'))
      .fromTo('opacity', 0.01, 'var(--backdrop-opacity)')
      .beforeStyles({
        'pointer-events': 'none'
      })
      .afterClearStyles(['pointer-events']);

    // You could alos define the popover-content
    // transform here if you wanted it to
    // still have a scale effect
    const transformAnimation = this.animationCtrl.create()
      .addElement(baseEl.querySelector('.popover-content'))
      .afterStyles( {
        top: '50%',
        left: '50%',
        transform: 'translate(-50%, -50%)'
      });

    const wrapperAnimation = this.animationCtrl.create()
    .addElement(baseEl.querySelector('.popover-wrapper'))
    .addElement(baseEl.querySelector('.popover-viewport'))
    .fromTo('opacity', 0.01, 1);

    return this.animationCtrl.create()
      .addElement(baseEl)
      .easing('cubic-bezier(0.36,0.66,0.04,1)')
      .duration(300)
      .addAnimation([backdropAnimation, wrapperAnimation, transformAnimation]);
  }

Thanks @liamdebeasi !

ionitron-bot[bot] commented 4 years ago

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.