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

Fix ambigious clearing behaviour on SelectButton #4107

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

Coretteket
Copy link
Contributor

In #3610, the meaning of the unselectable property on SelectButton was reversed from "able to unselect" to "unable to unselect". Because the documentation was not updated accordingly, the property currently behaves exactly opposite to how it's described.

In #3708 and #3973, there was significant disagreement as to what the property should mean. I would agree with those arguing that the textbook meaning of "unselectable" would be that you are able to unselect. However, the name clearly causes some confusion.

In this PR, I've therefore deprecated the unselectable property in favour of the allowEmpty property, to avoid further breaking changes. This naming is much less ambigious, and matches the one already found on InputNumber. I've also updated the documentation for unselectable to match current behaviour.

Please let me know if I've missed anything, or if further clarification is needed!

Fixes #3973.

@vercel
Copy link

vercel bot commented Jul 4, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
primevue ⬜️ Ignored (Inspect) Jul 4, 2023 2:44pm

@Coretteket
Copy link
Contributor Author

Hey – just wondering if there's anything I can do to help move this PR along?

@mertsincan
Copy link
Member

Thanks a lot for your contribution!

Best Regards,

@SamuelWei
Copy link

@Coretteket and @mertsincan Can you have a look at #4731 I think this attribute has an unwanted sideeffect on the multiple prop

@jnoordsij
Copy link

@Coretteket and @mertsincan Can you have a look at #4731 I think this attribute has an unwanted sideeffect on the multiple prop

By the looks of things, the old unselectable and multiple already exhibited this behavior. This MR has not introduced any change of behavior, but rather created a new name for it. Therefore I think this MR is not relevant to the issue as described in #4731.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add allowEmpty property to SelectButton
5 participants