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
51.02k stars 13.51k forks source link

bug: Can't control ion-fab state manually #21678

Closed Yohandah closed 2 years ago

Yohandah commented 4 years ago

Bug Report

Ionic version: [x] 5.x

Current behavior:

I wanna be able to control the ion-fab state and its children (activated or not) programmatically.

Right now, when binding a click event to a ion-fab, it doesn't override the default behavior resulting in not be able to control the activated property programmatically.

I am in need of this to enable the fab on hover on desktop, and on click on touchscreens ...

Expected behavior:

We should be able to control the ion-fab and its children (lists and fab) programmatically with a .open() / .close() (there is a close function but no open why is that ?) function or with the activated property

Steps to reproduce:

You can also try to bind the variable to another button and it will enter in conflict with the main fab button and resulting in a wrong value for the activated variable

Related code:

https://stackblitz.com/edit/ionic-v5-bug-fab-visible-toggle

averyjohnston commented 2 years ago

Thanks for the issue! I know it's been a while, but I wanted to circle back to this with a potential fix in case you or anyone else monitoring this issue still needs it.

I noticed that the code from your Stackblitz still replicates the issue on the latest version of Ionic. However, I was able to control the fab without conflicts by directly toggling the activated property:

<ion-button (click)="toggleFab()">toggle</ion-button>
<ion-fab #fab> ... </ion-fab>
export class AppComponent {
  @ViewChild('fab') fab;

  constructor() {}

  toggleFab() {
    this.fab.activated = !this.fab.activated;
  }
}

Could you let me know if this solves your use case? Thanks!

Yohandah commented 2 years ago

I guess it fixes the issue ... but the bug is still there. The [activated] property in the template should work properly or we should have a proper open() and close() methods

Also --I'm not sure-- but is activated declared on IonFab type? Property 'activated' does not exist on type 'IonFab'.

averyjohnston commented 2 years ago

activated should be defined, yes. Here's a repo I set up to showcase both strategies on the latest Ionic version: https://github.com/amandaesmith3/sandbox-ionic-angular/tree/fab

I'll dig into the issue with toggling showFab and let you know when I have more to share there :eyes: While testing, I noticed that wrapping the switch in a raf also fixes things: requestAnimationFrame(() => this.showFab = !this.showFab);

averyjohnston commented 2 years ago

I've dug into this some more. The issue here is that ion-fab has its own click listener built in, which is also flipping the value of activated, on top of the manually added toggleShowFab function. The built-in listener fires after the manually added one.

Say you have showFab at false, and the fab closed. If you then click the ion-fab, the following happens:

  1. toggleShowFab fires. showFab is set to true.
  2. The fab opens.
  3. The fab's built-in click listener fires.
  4. The fab closes. showFab remains at true since property binding only goes one way.

This gets the two states out of sync.

Wrapping the showFab flip in requestAnimationFrame causes it to happen after the built-in listener. This means that whatever value it gets flipped to will always overwrite whatever the built-in listener did, keeping things in sync.

This is all working as intended. I would recommend toggling this.fab.activated directly instead, as shown in my previous post, as this avoids extra flips of showFab and any re-rendering they may cause.

I'm going to go ahead and close this out. If you run into related problems that aren't solved by the aforementioned strategies, please open a new issue. Thanks!

Yohandah commented 2 years ago

Not really developer friendly but okay

ionitron-bot[bot] commented 2 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.