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

Added remove single for remove plugin #848

Merged
merged 1 commit into from
Jan 21, 2016
Merged

Conversation

ChoppyThing
Copy link
Contributor

Hi, i am reopening this pull request for the single close plugin.

Hope its fine now

@mattdennewitz
Copy link

a loving +1 on this. it'd be a great addition.

@sman591
Copy link

sman591 commented Jul 15, 2015

+1, this would be fantastic

@sidoruk-sv
Copy link

Any progress?

@ChoppyThing
Copy link
Contributor Author

Still waiting for now

@flaviooliveri
Copy link

+1

@joallard
Copy link
Member

Original issue in #665

joallard added a commit that referenced this pull request Jan 21, 2016
Plugin removeButton: Make it work with single option mode
@joallard joallard merged commit 6a174ea into selectize:master Jan 21, 2016
@michaelperrin
Copy link

Hi @joallard , thanks for the merge of this PR!

Will you make a new release with the updated dist files to make this feature easily integrated?

@joallard
Copy link
Member

joallard commented Apr 1, 2016

Good point, I'll discuss releasing with @brianreavis

@michaelperrin
Copy link

Thanks @joallard !

@Pictor13
Copy link
Contributor

Pictor13 commented Sep 1, 2016

I am having problems with this.

While with a multi Selectize the plug-in works correctly, it doesn't when having a single Selectize (in my specific case is a multiple + maxItems:1).
I tried to debug the problem but I was not able to find the bug yet cause of my limited knowledge of the library internals.

Any suggestion? Am I maybe missing something?

@thiagomorato
Copy link

@Pictor13 did you have any luck with this issue? I'm experiencing the same thing.

@Pictor13
Copy link
Contributor

@thiagomorato No actually I am still looking into it (but don't know if I'll have soon the time to properly fix it).

Anyway, the issue resides in the render method that always returns html[0], while in our case the markup is formed by two elements (html[0] as a div and html[1] as a a). See line 3210.
So we never get the button returned in the item markup. I wonder how

Another thing that I noted is that, when setting maxItems:1 then the selectize mode changes to "single", if not explicitly specified with settings.mode (see line 1194).
Specifying "multi" as mode made the remove_button magically appear; but unfortunately it doesn't work in removing the option, failing with a jQuery error:

Error: Syntax error, unrecognized expression: .

No idea what is happening here; even less how it has been thought and how it is supposed to work. I don't want to change stuff if I am not sure about the side-effects.
So for now I still struggle with this and couldn't find a working solution.
I'll update when I'll find how to fix.

Did you find something more about it?

@Pictor13
Copy link
Contributor

Oook, I just found a fix that works for my case:

First, just copy paste the same append function from multiClose into singleClose. This fixes the not appearing "remove_button", making it render inside the "item" element, instead that as adjacent element!
I wonder what was the purpose of making that different from the append in multiClose.

Additionally, I changed the line 3716 in:

options.className = options.className || 'remove-single';

because I want the possibility to pass my custom classname, instead of being forced to the "remove-single" (in my case I actually don't care of the difference between 'multi' and 'single', when styling the remove button).

I also found that there was another pull-request with a more clear code:
1c62573
but actually I am not so good with GitHub to really understand what was the sequence of events, what was integrated and what wasn't, and why.

My changes fix the issue, but am not really sure why a different append function was being used in the first place, so I can't assume that my changes are correct.

Eventually I can create a pull request, but it should be reviewed to avoid it to break something else.

@GTCrais
Copy link

GTCrais commented Dec 2, 2016

Remove button wasn't rendering for me in single mode. I can confirm copy-pasting append function from multiClose into singleClose which @Pictor13 suggested works. The CSS then needs to be adjusted. I even prefer this solution since it allows me to position the remove button relative to the item container, i.e. I can position it to the left of the item.

@bradleybensmith
Copy link

I just ran into this myself. Thank you @Pictor13 for the workaround.

Any reason why this hasn't been fixed?

@Pictor13
Copy link
Contributor

Pictor13 commented Jul 24, 2017

Because I made a mess with my commits, and I didn't take the time to tidy things up 😛
Hopefully this new pull-request #1311 will be merged soon:

Fixes to remove button plugin (#1311)

Please @bram1028 let me know if that fixes correctly also for you. I reviewed my code changes again, but a confirmation always feels better 😁

@jsruok
Copy link

jsruok commented Aug 7, 2017

I was missing the single mode remove button as well, but copy-pasting the append function fixed it. After corresponding CSS adjustments all is well everything looks ok.

Edit 1: However, I can only remove the selection without the dropdown being open. When dropdown is visible a click on the remove button only closes the dropdown.


Edit 2: When dropdown is open, an onMouseDown event is triggered by a click on the single mode remove button, but an onClick event is not. At this point I chose to work around this by changing the hook from click to mousedown i.e. from

thisRef.$control.on('click', '.' + options.className, function(e) {

to

thisRef.$control.on('mousedown', '.' + options.className, function(e) {

in the singleClose function. Not very elegant, but works for me.

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

Successfully merging this pull request may close these issues.