feat: add new checkbox to choose if already picked users should be skipped #19
Conversation
|
Hello @Ithanil. Thanks for the contribution, it's indeed an interesting feature! I will test it later on! |
| name="options" | ||
| value="skipPickedUsers" | ||
| /> | ||
| <span className="check-box-label">Skip Picked Users</span> |
There was a problem hiding this comment.
This name is not really meaningful, I would suggest something like:
"Allow pick same user multiple times" which will be false by default.
Also, there is the need to make necessary changes in the code afterwards.
There was a problem hiding this comment.
"A user may be picked more than once" perhaps?
There was a problem hiding this comment.
In line with #13, I would suggest "Include already picked users". What do you think?
There was a problem hiding this comment.
In line with #13, I would suggest "Include already picked users". What do you think?
That's quite good! @antobinary, what do you think? I'm on board with this one
There was a problem hiding this comment.
Done.
But for consistency #13 needs to be adressed too now. Otherwise it looks awkward to have 2x "Skip" options (enabled per default) and then one "Include" (disabled per default).
There was a problem hiding this comment.
But for consistency #13 needs to be adressed too now
Don't worry about that, I will handle it, one other thing this PR exposed is that if the list of already picked users is long enough, there is no way to pick another one, because the height of the modal grows indefinitely... I will open an issue for that. And don't worry about that either, the plugin already did that.
I think removing already picked users from the pool of users to be picked is an OK default, but not always what you want. Hence, I've added a checkbox to "Skip Picked Users", which is enabled by default. If it is unchecked, previously picked users will enter the pool for selection again.
Considered alternative: Manually clearing the list of previously picked users every time. But this is a bit of a hassle and has the side effect of clearing the history. ( Also, the reset currently just doesn't work ;-) )