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

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

Open
psarno opened this issue Sep 22, 2023 · 14 comments
Open

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

psarno opened this issue Sep 22, 2023 · 14 comments
Labels
Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible

Comments

@psarno
Copy link

psarno commented Sep 22, 2023

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 psarno added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Sep 22, 2023
@psarno psarno changed the title p-button: Duplicate icons shown when using directive p-button: Duplicate icons shown when using directive and [loading] Sep 22, 2023
@SoyDiego
Copy link
Contributor

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: #13363
I don't know what happened now :(

@psarno
Copy link
Author

psarno commented Sep 23, 2023

@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);

}

@dalenguyen
Copy link
Contributor

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

@SoyDiego
Copy link
Contributor

@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 #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

@psarno
Copy link
Author

psarno commented Sep 24, 2023

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

@SoyDiego
Copy link
Contributor

SoyDiego commented Sep 24, 2023

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.

@psarno
Copy link
Author

psarno commented Sep 24, 2023

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.

@SoyDiego
Copy link
Contributor

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

@mountpoint
Copy link

I have same issue.

primeng 16.3.1
angular 16.2.1

@SoyDiego
Copy link
Contributor

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

@mountpoint
Copy link

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
Copy link

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

@rosenthalj
Copy link
Contributor

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

Fixed in PrimeNG 17.18.2

@mountpoint
Copy link

@cetincakiroglu I think you can close the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible
Projects
None yet
Development

No branches or pull requests

5 participants