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: TriStateCheckbox lost margin between label and checkbox #14369

Closed
jpgoelz opened this issue Dec 14, 2023 · 8 comments · Fixed by #14371
Closed

Component: TriStateCheckbox lost margin between label and checkbox #14369

jpgoelz opened this issue Dec 14, 2023 · 8 comments · Fixed by #14371
Labels
Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible

Comments

@jpgoelz
Copy link

jpgoelz commented Dec 14, 2023

Describe the bug

There is a design bug which was introduced by first replacing p-checkbox-label with p-tristatecheckbox-label in
#13977, then just removing the class altogether in #14312 and readding it in #14360.

When checking the changes between 16.6.0 (the version I know where both clicking the label and the style were working correctly) and 17.1.0 (link), I notice that the p-tristatecheckbox-label CSS class does no longer exist and should be replaced again with p-checkbox-label to fix this.

Additionally, the on-click event should be re-added to allow clicking on the label.

image

Environment

n/a

Reproducer

https://stackblitz.com/edit/77z5q5?file=src%2Fapp%2Fdemo%2Ftri-state-checkbox-basic-demo.html

Angular version

16, 17

PrimeNG version

16.9.1, 17.0.0, 17.1.0

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

20.x

Browser(s)

No response

Steps to reproduce the behavior

No response

Expected behavior

No response

@jpgoelz jpgoelz added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Dec 14, 2023
@SoyDiego
Copy link
Contributor

Hi @jpgoelz I did a PR solving the problem.
You can check here: #14371
But the click in the checkbox it is working ok, you should add a value. Here you have a example:

solved

But I fixed also the examples in the website because were failing. If you use the attribute label you don't need to use inputId like the examples in the website. You can see more examples in my PR.

Thanks for report it!

@jpgoelz
Copy link
Author

jpgoelz commented Dec 16, 2023

Hey, thank you for the fast response!

The issue with clicking was regarding clicking the label: of the tri-state-checkbox:

tri-state-checkbox-label-click
(StackBlitz)

I guess my "minimal working example" was too minimal. 😅

@SoyDiego
Copy link
Contributor

Hey, thank you for the fast response!

The issue with clicking was regarding clicking the label: of the tri-state-checkbox:

tri-state-checkbox-label-click tri-state-checkbox-label-click (StackBlitz)

I guess my "minimal working example" was too minimal. 😅

Yeah, don't worry with my PR will fixed the problem, If PrimeNG Team approve it and it's a good solution for them, will be released.
Here you have a example working with your code:

fixed

@jpgoelz
Copy link
Author

jpgoelz commented Dec 16, 2023

Perfect, thanks!

@jpgoelz
Copy link
Author

jpgoelz commented Dec 21, 2023

@cetincakiroglu, this was released in 17.2.0 but does not appear in the Changelog.

@SoyDiego
Copy link
Contributor

@cetincakiroglu, this was released in 17.2.0 but does not appear in the Changelog.

Yes, it does @jpgoelz.

image

@jpgoelz
Copy link
Author

jpgoelz commented Dec 21, 2023

I obviously need glasses. Sorry for wasting your time! 🤦

I only looked at the changelog provided by Renovate, which apparently parses the milestones and not the changelog provided by you guys. My bad!

image

@SoyDiego
Copy link
Contributor

no problem, happy to help!

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

Successfully merging a pull request may close this issue.

2 participants