primeng icon indicating copy to clipboard operation
primeng copied to clipboard

p-button: Duplicate icons shown when using directive and [loading]

Open psarno opened this issue 2 years ago • 14 comments

Describe the bug

When we use <button pButton pRipple> and bind its [loading] proprety to a boolean, we see two icons on the button when the boolean is set back to false.

image

It includes both the loading spinner icon as well as the icon set in the icon attribute. Defined as:

<button
  [loading]="isLoading"
  icon="pi pi-check"
  label="Save"
  pButton
  pRipple
  (click)="clickSearch()"
></button>

To reproduce:

  clickSearch() {
    this.isLoading = true;
    window.setInterval(() => this.isLoading = false, 200);
}

Note that setInterval in the reproducer is just to simulate some work being done.

If we replace this <button> with a <p-button>, it works as expected.

Environment

Angular 16.2.2 PrimeNG 16.3.1

Reproducer

No response

Angular version

16.2.2

PrimeNG version

16.3.1

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

18.16.0

Browser(s)

No response

Steps to reproduce the behavior

  1. Add a <button pButton pRipple> element with [loading]="isLoading"
  2. When clicked, change the value of the isLoading boolean to true
  3. Set isLoading back to false, simulating the loading is complete

Button is re-enabled, but shows two icons

Expected behavior

Button should not still show the loading spinner icon when the loading property is set back to false.

psarno avatar Sep 22 '23 22:09 psarno

Describe the bug

When we use <button pButton pRipple> and bind its [loading] proprety to a boolean, we see two icons on the button when the boolean is set back to false.

image

It includes both the loading spinner icon as well as the icon set in the icon attribute. Defined as:

<button
  [loading]="isLoading"
  icon="pi pi-check"
  label="Save"
  pButton
  pRipple
  type="submit"
></button>

If we replace this <button> with a <p-button>, it works as expected.

This is not an isolated incident, it has occured across our site and forced us to go into each affected component and replace native <button> elements with <p-button> elements to fix.

I can not produce a repro because I can't get the latest v16 PrimeNG working under StackBlitz. We really need an updated template, which is still on v14.

Environment

Angular 16.2.2 PrimeNG 16.3.1

Reproducer

No response

Angular version

16.2.2

PrimeNG version

16.3.1

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

18.16.0

Browser(s)

No response

Steps to reproduce the behavior

1. Add a `<button pButton pRipple>` element with `[loading]="isLoading"`

2. When clicked, change the value of the `isLoading` boolean to true

3. Set `isLoading` back to false, simulating the loading is complete

Button is re-enabled, but shows two icons

Expected behavior

Button should not still show the loading spinner icon when the loading property is set back to false.

Again this problem? I did a PR a few weeks ago solving that things: https://github.com/primefaces/primeng/pull/13363 I don't know what happened now :(

SoyDiego avatar Sep 23 '23 08:09 SoyDiego

@SoyDiego I found a minimal reproducible scenario. To repro:

Template:

<button
  pButton
  pRipple
  icon="pi pi-search"
  label="Search"
  (click)="clickSearch()"
  [loading]="searching"></button>

TypeScript:

clickSearch() {

   this.searching = true;

   // Do any work here before setting back to false - this just simulates it
   window.setInterval(() => this.searching = false, 200);

}

psarno avatar Sep 23 '23 12:09 psarno

Probably, it's not being released. I tried with the latest master branch and couldn't reproduce the issue.

dalenguyen avatar Sep 23 '23 15:09 dalenguyen

@SoyDiego I found a minimal reproducible scenario. To repro:

Template:

<button
  pButton
  pRipple
  icon="pi pi-search"
  label="Search"
  (click)="clickSearch()"
  [loading]="searching"></button>

TypeScript:

clickSearch() {

   this.searching = true;

   // Do any work here before setting back to false - this just simulates it
   window.setInterval(() => this.searching = false, 200);

}

I don't know why your last two issues (this issue and https://github.com/primefaces/primeng/issues/13636), I couldn't reproduce it. For me it's working as expected. I tried your code and also I added p-button and works perfectly.

My code

image

Testing

button works

SoyDiego avatar Sep 24 '23 11:09 SoyDiego

I am not sure, I see the issue was merged and released as part of 16.2.0 and we're on 16.3.1.

Version 117.0.5938.92 (Official Build) (64-bit)

I am not sure why the behavior would be different. This is what we are seeing:

primeng-spinner

Here's what it looks like in the DOM when I inspect:

image

Template is:

image

psarno avatar Sep 24 '23 12:09 psarno

Share a stackblitz with the versions exactly that you have in your project. I don't know what more can you try. Are you sure before the fix merged on release 16.2.0 you didn't create a custom fix or something like that? Maybe I'm wrong but I'm trying to reduce possibilities. I don't know how can I help you, I don't have more ideas. It's weird.

If this problem existed, I'm sure more people would report it as has happened before. Buttons are a very important part of a website and yet no one wrote any issue except you. I think there is some problem in your particular project because as I said before, I couldn't reproduce another issue of yours either.

SoyDiego avatar Sep 24 '23 13:09 SoyDiego

I can't get StackBlitz working with PrimeNG 16.3.1. I wish I could. If anyone has success setting an empty example up that uses PrimeNG 16.3.1 and not v14 I'll happily use it.

Here's another strange thing I just noticed.

If I set loadingIcon="pi pi-check" on both buttons, the results are:

  1. The <button pButton> element does not duplicate icons when re-enabled now, but the check icon is not animated.
  2. The <p-button> does animate the check icon

While loading:

image

As you can see, the <p-button> icon is rotating while the other is fixed.

After loading they both look correct:

image

I have looked into the PrimeNG source code for <button pButton> and <p-button>but I am not sure exactly what would explain this behavior.

I have inspected the DOM in these cases and we have no custom CSS or anything else trying to override behavior on these.

Switching <button> to <p-button> resolves the issue so for now we can just go through the project and use the component and not the directive. I am unsure what the difference there is.

psarno avatar Sep 24 '23 14:09 psarno

@psarno maybe you can write your problem about stackblitz in the new forums https://github.com/orgs/primefaces/discussions/categories/primeng

SoyDiego avatar Sep 24 '23 14:09 SoyDiego

I have same issue.

primeng 16.3.1 angular 16.2.1

mountpoint avatar Sep 26 '23 05:09 mountpoint

I have same issue.

primeng 16.3.1 angular 16.2.1

Sorry I cannot replicate. I tried and I sent a gif above. @cetincakiroglu

SoyDiego avatar Sep 26 '23 05:09 SoyDiego

I have same issue. primeng 16.3.1 angular 16.2.1

Sorry I cannot replicate. I tried and I sent a gif above. @cetincakiroglu

I have change detection onpush in my component. maybe this will help you reproduce the problem

mountpoint avatar Sep 26 '23 05:09 mountpoint

@cetincakiroglu the problem is still here. primeng 17.18.1 angular 18.0.3

mountpoint avatar Jun 19 '24 15:06 mountpoint

@cetincakiroglu the problem is still here. primeng 17.18.1 angular 18.0.3

Fixed in PrimeNG 17.18.2

rosenthalj avatar Jul 01 '24 08:07 rosenthalj

@cetincakiroglu I think you can close the issue

mountpoint avatar Oct 15 '24 19:10 mountpoint

Hi,

So sorry for the delayed response! Improvements have been made to many components recently, both in terms of performance and enhancement. Therefore, this improvement may have been developed in another issue ticket without realizing it. You can check this in the documentation and try the latest PrimeNG version(v19). If there is no improvement on this, can you open a new issue so we can include it in our roadmap?

Thanks a lot for your understanding! Best Regards,

mertsincan avatar Dec 25 '24 13:12 mertsincan