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/#6216 MultiSelect - SelectionLimit issue #6218

Merged
merged 7 commits into from
Apr 13, 2024

Conversation

charithAmila
Copy link
Contributor

@charithAmila charithAmila commented Mar 24, 2024

Defect Fixes

Fix: #6121 MultiSelect - SelectionLimit issue

Fix #6391

Copy link

vercel bot commented Mar 24, 2024

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

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
primereact ⬜️ Ignored (Inspect) Visit Preview Apr 13, 2024 0:13am
primereact-v9 ⬜️ Ignored (Inspect) Visit Preview Apr 13, 2024 0:13am

@charithAmila charithAmila changed the title Fix/#6216 MultiSelect - SelectionLimit not working Fix/#6216 MultiSelect - SelectionLimit issue Mar 24, 2024
@charithAmila charithAmila marked this pull request as ready for review March 24, 2024 03:49
@@ -653,6 +657,10 @@ export const MultiSelect = React.memo(
return ObjectUtils.isFunction(props.optionDisabled) ? props.optionDisabled(option) : ObjectUtils.resolveFieldData(option, props.optionDisabled);
}

if (props.selectionLimit && props.value && props.value.length >= props.selectionLimit && !isSelected(option)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this check is needed. If the user exceeds the selected limit, other options do not need to be disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mertsincan
So how should selectionLimit work? Can users select more than the selection limit?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

according to prime react JSDoc selection limit should limit the amount of selections.

This is obviously a broken prop

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I made some tweaks and thoroughly tested this. It is in line with how PrimeNG and PrimeVue handle this scenario. @mertsincan the reason we need to check if the current option is already selected is to allow them to unselect it. So in the scenario of selectionLimit={1} they need to be able to unselect the selected one

image

@mertsincan mertsincan added the Resolution: Needs Revision The pull request can't be merged. Conflicts need to be corrected or documentation and code updated. label Mar 25, 2024
@XYIAN
Copy link

XYIAN commented Apr 12, 2024

this is an issue for my team as well - when trying to add any selection limit - the component still allows us to select as many as we want. If I have a multi select of 50 and want a limit of 2, adding selectionLimit does not limit users from selecting more than 2

https://stackblitz.com/edit/vitejs-vite-qu7tmj?file=src%2FApp.tsx

@melloware melloware added Type: Bug Issue contains a defect related to a specific component. and removed Resolution: Needs Revision The pull request can't be merged. Conflicts need to be corrected or documentation and code updated. labels Apr 13, 2024
@melloware melloware merged commit 0a3d4e7 into primefaces:master Apr 13, 2024
5 checks passed
@melloware melloware added this to the 10.6.4 milestone Apr 13, 2024
@nitrogenous nitrogenous removed this from the 10.6.4 milestone Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a defect related to a specific component.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MultiSelect: selectionLimit does not limit selections MultiSelect - SelectionLimit
5 participants