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

Closes #129, allows duplicate values #324

Closed
wants to merge 2 commits into from

Conversation

jbrooksuk
Copy link
Contributor

I broke down and had to add this in! For duplicates to work, you must have the mode set to multi, otherwise it wouldn't make sense.

Documentation changes to follow.

I've included the compiled files too. Since I only added the duplicates option, I don't know why there are so many other changes? If you can figure this out, I can change the commits.

@ohaibbq
Copy link

ohaibbq commented Mar 11, 2014

Hey @jbrooksuk, thanks for this!

We coincidentally tried updating our selectize earlier today. We noticed that the port of the solution from #129 no longer worked.

Here was the case that was breaking:

<select>
  <option value="1" selected="selected">Option</option>
  <option value="1" selected="selected">Option</option>
</select>
$('select').selectize({
  mode: 'multi',
  duplicates: true
});

The logic for handling optgroups is short circuiting L2450, causing only distinct items to be added to selectize.

I believe that we should only return if duplicates are not being allowed, however I'm not entirely sure of all the cases surrounding this logic.

@jbrooksuk
Copy link
Contributor Author

Oh damn. I completely forgot about optgroups. If you make the change that you suggested, does it fix it?

@ohaibbq
Copy link

ohaibbq commented Mar 11, 2014

It seems to work for both selects with no optgroups and those with. I haven't had a chance to dig any deeper in to the logic around all the cases around L2450. I should have some free time tomorrow, though.

@jbrooksuk
Copy link
Contributor Author

@ohaibbq ok, since I don't use optgroups, I don't really have any test cases. If you could provide me with some examples I'm happy to take a look. I'd rather provide @brianreavis with a full PR to add this feature.

@jbrooksuk
Copy link
Contributor Author

@brianreavis have you had a chance to take a look yet?

@Scottmitch
Copy link
Contributor

To ease the process of merging the pull request the owner has requested you exclude the generated files in pull requests in the readme.

@msangel
Copy link

msangel commented Sep 17, 2014

ok, o try this code and it do not work until..... until i do not change Selectize.prototype.search. in fact, if we allow same items few times, we also should show used elements in list available options

before:

// filter out selected items
if (settings.hideSelected) {

after:

var duplicates = self.settings.duplicates;              
// filter out selected items
if (settings.hideSelected && !duplicates ) {

@arbw
Copy link

arbw commented Sep 18, 2014

Hello,
I wanted exactly this functionality. Thanks guys. If you press the backspace though, it removes all instances of a selection. I will look into it, but thought I should comment.

1 similar comment
@arbw
Copy link

arbw commented Sep 18, 2014

Hello,
I wanted exactly this functionality. Thanks guys. If you press the backspace though, it removes all instances of a selection. I will look into it, but thought I should comment.

@jbrooksuk
Copy link
Contributor Author

Closing due to conflicts and no time to make it work.

@jbrooksuk jbrooksuk closed this Jan 22, 2015
@jbrooksuk jbrooksuk deleted the allow-duplicates branch October 6, 2017 11:10
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.

5 participants