Skip to content

Conversation

@SteveTheTechie
Copy link
Contributor

@SteveTheTechie SteveTheTechie commented Jan 14, 2018

The selectedMax option limits the number of selections to a maximum value. If the user attempts to check an additional selection in the multi-select, a warning message is shown in the button for 1 second.

This feature is very useful for the "select up to a maximum of x out of y available selections" use case. It implements a constraint approach.

An additional unit test has been also added for this feature.

NOTE: This PR follows PR #725 in succession.

Mostly changing to $naming scheme for jQuery objects and adding more comments.
bind() is deprecated.  Use on() instead.
Changed the default selectedText to be "# of # selected" so that developers will be aware that this form is available.  (It always has been available..)
@SteveTheTechie
Copy link
Contributor Author

Will re-open later.

@SteveTheTechie
Copy link
Contributor Author

Re-opening. Found the problem. ;-)

@SteveTheTechie
Copy link
Contributor Author

There is a question of what to do with checkAll if a selectedMax constraint is in place.
Perhaps "Check All" cannot be shown or acted on if a constraint is in place.
This also may present some challenges w/ Flip All.

Perhaps any selections beyond the selectedMax limit are cleared?

BTW, I already use selectedMax (I call it maxSelected) constraint feature in my old code version that I am using in my web app.

I am just trying to make sure I "tidy it up" here if other people are going to be using it, also.

@mlh758
Copy link
Collaborator

mlh758 commented Jan 15, 2018

Okay, I rebased your last branch to squash it down. That means the history is a little different now between these branches. Don't worry though, it's an easy fix! You might want to make a branch off this branch to test on the first time you do the rebase --onto command just in case.

git rebase --onto <target_branch, probably ehynds/version3> HEAD~2
Make sure your changes are still there and correct
git push -f <-- The force push let's you overwrite the remote branch with the new history

What this does is take 2 commits off your current branch and replays them on top of the target branch. It's essentially a simplified cherry pick since you're only pulling off the top.

@mlh758
Copy link
Collaborator

mlh758 commented Jan 17, 2018

Sorry I didn't respond to your question sooner. I would say it would be best to disable select all when the selectedMax feature is enabled. I don't think it would make sense to trim the options list down, and just selecting the first x rows options wouldn't be helpful either.

@SteveTheTechie
Copy link
Contributor Author

Ok I will tweak this a bit more in a subsequent PR to address the concerns.

@SteveTheTechie
Copy link
Contributor Author

SteveTheTechie commented Jan 17, 2018

This code update is included in #728. I am not sure if you want me to just close this and let you just merge #728?

@mlh758
Copy link
Collaborator

mlh758 commented Jan 21, 2018

Yeah if this is in 728 as well I'll just merge it all at once.

@mlh758 mlh758 closed this Jan 21, 2018
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.

2 participants