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

Component: Button styleClass #13963

Closed
nunutu29 opened this issue Oct 26, 2023 · 18 comments · Fixed by #13971
Closed

Component: Button styleClass #13963

nunutu29 opened this issue Oct 26, 2023 · 18 comments · Fixed by #13971
Labels
Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Milestone

Comments

@nunutu29
Copy link

Describe the bug

Using version 16.4.1 the following code adds the class p-button-sm to the inner <button> element:

<p-button styleClass="p-button-sm" icon="pi pi-pencil"></p-button>

Result 16.4.1:
image

After updating to version 16.5.1 this doesn't work/happen anymore:
Result 16.5.1:
image

I can't find any breaking changes docs except this link: #13851

It also seems that it is relative to primeng classes, because if i add some custom class it works:

<p-button styleClass="my-test-class" icon="pi pi-pencil"></p-button>

Result 16.5.1:
image

Environment

Angular 16.2.0 + Windows 10 + Chrome (latest)

Reproducer

No response

Angular version

16.2.0

PrimeNG version

16.5.1

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

18.18.1

Browser(s)

Chrome (latest)

Steps to reproduce the behavior

No response

Expected behavior

No response

@nunutu29 nunutu29 added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Oct 26, 2023
@nunutu29 nunutu29 changed the title Component: Button Component: Button styleClass Oct 26, 2023
@maruthumj
Copy link
Contributor

@nunutu29 Can you please add [size]='small' and check?

@nunutu29
Copy link
Author

@maruthumj Adding the code size="small" to the <p-button> indeed works: it adds correctly the class p-button-sm to the <button> element but the project I am working at is way to big to apply this code everywhere.

I guess the problem affects all those classes:

  • p-button-raised
  • p-button-rounded
  • p-button-text
  • p-button-plain
  • p-button-severity
  • p-button-outlined
  • p-button-link
  • p-button-size

@maruthumj
Copy link
Contributor

maruthumj commented Oct 27, 2023

@maruthumj Adding the code size="small" to the <p-button> indeed works: it adds correctly the class p-button-sm to the <button> element but the project I am working at is way to big to apply this code everywhere.

I guess the problem affects all those classes:

  • p-button-raised
  • p-button-rounded
  • p-button-text
  • p-button-plain
  • p-button-severity
  • p-button-outlined
  • p-button-link
  • p-button-size

@nunutu29 got it. I have raised PR to fix this issue.

@cetincakiroglu
Copy link
Contributor

cetincakiroglu commented Nov 8, 2023

Hi,

I've just created an example to reproduce but it seems styleClass works as usual, could you guide me on what the exact problem is to identify clearly?

See the example: https://stackblitz.com/edit/he58hx?file=src%2Fapp%2Fdemo%2Fbutton-sizes-demo.html

@nunutu29
Copy link
Author

nunutu29 commented Nov 8, 2023

Hi @cetincakiroglu,

Using this code, the class p-button-sm doesn't render.

<p-button styleClass="p-button-sm" icon="pi pi-pencil"></p-button>

See: https://stackblitz.com/edit/he58hx-uvmmqb?file=src%2Fapp%2Fdemo%2Fbutton-sizes-demo.html

@nunutu29
Copy link
Author

Hi @cetincakiroglu,
Any news for this ? We would like to upgrade but this is blocking us to do so.

Thank you

@penihel
Copy link

penihel commented Dec 5, 2023

Any news for this ? We would like to upgrade but this is blocking us to do so.

@cetincakiroglu cetincakiroglu added Type: Bug Issue contains a bug related to a specific component. Something about the component is not working and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Dec 6, 2023
@cetincakiroglu cetincakiroglu added this to the 17.0.0 milestone Dec 6, 2023
cetincakiroglu added a commit that referenced this issue Dec 6, 2023
Bug fix: #13963 Component: Button styleClass is not being added to the <button>
@STotev
Copy link

STotev commented Jan 17, 2024

Correct me if I am wrong, but wasn't this issues supposed to be fixed and restore the previous behaviour of the p-button component?

I am upgrading to the latest version and the p-button-text and p-button-sm passed to styleClass are still not working, so I have to use size and text attributes.
Can at least the breaking changes be mentioned in the release notes?

I went through all breaking changes from 14 until 17 and now I wonder how many other things are there with breaking changes that aren't mentioned.

@STotev
Copy link

STotev commented Jan 17, 2024

It looks like the styleClass is passing the classes but if you pass any classes that are already conditionally defined in the object in the buttonClass() and if they are already false, then they for some reason get stripped out from the styleClass too.

I checked the button tests and I couldn't see tests verifying the behaviour of the classes. I think it would be nice to have such test.

@cetincakiroglu @maruthumj any thoughts and should I open a separate issue?

@cetincakiroglu
Copy link
Contributor

cetincakiroglu commented Jan 17, 2024

@STotev

Could you please provide a stackblitz example so we can identify the issue? There are 3 major versions and lots of minors between 14 and 17, did you check the changelogs?

We've planned in our roadmap to rework tests after the passthrough and unstyled implementation.

@STotev
Copy link

STotev commented Jan 17, 2024

@cetincakiroglu sure, will do as soon as possible.

I went through the changelogs between those versions but I mainly focused on the breaking changes sections. I will make another, more thorough, go with the bugfix sections, too.

I guess the forementioned issue was not mentioned as a breaking change as it wasn't thought as one after adding the styleClass in the buttonClass().

I will get back to you as soon as I create a stackblitz example.

Thank you for your quick response. Appreciate it.

@STotev
Copy link

STotev commented Jan 17, 2024

@cetincakiroglu please find the examples here https://stackblitz.com/edit/stackblitz-starters-tqwku2?file=src%2Fmain.ts

I added multiple examples with what's expected and what's the outcome in multiple scenarios. I hope it helps.

I could've written unit tests but didn't want to spend too much time setting this up, so you'll need to inspect the buttons to see.

@nunutu29
Copy link
Author

nunutu29 commented Jan 17, 2024

@STotev I can confirm your issue.

It might be something with the focus/blur/click event ?
if you inspect other element, and then go to yours by exploding on the DOM you will se the classes rendered.

image

THE SAME ELEMENT "TOUCHED":
image

@STotev
Copy link

STotev commented Jan 17, 2024

@nunutu29 nice spot. I was wondering why sometimes my buttons appeared correctly and then break if I, sometimes, hover and sometimes click.

@cetincakiroglu
Copy link
Contributor

@cetincakiroglu please find the examples here https://stackblitz.com/edit/stackblitz-starters-tqwku2?file=src%2Fmain.ts

I added multiple examples with what's expected and what's the outcome in multiple scenarios. I hope it helps.

I could've written unit tests but didn't want to spend too much time setting this up, so you'll need to inspect the buttons to see.

Thanks a lot for the detailed example, I've addressed this one in #14583
Fix will available in today's release (17.3.3)

Could you please check after the 17.3.3 release?

@STotev
Copy link

STotev commented Jan 18, 2024

Thanks a lot for the detailed example, I've addressed this one in #14583 Fix will available in today's release (17.3.3)

Could you please check after the 17.3.3 release?

Thank you for the quick fix. Will test it after it comes out and will update you.

@STotev
Copy link

STotev commented Jan 18, 2024

@cetincakiroglu tested in my stackblitz above with 17.3.3 and now all buttons behave as expected.

Great work, thank you.

@forestmaxime
Copy link

Is it possible that this error is present in 16.9.12-lts, if so could this be backported to the next 16.9.x LTS version? 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants