Do not merge: Updated the chosen plugin to the latest stable release #2891

Closed
wants to merge 2 commits into
from

6 participants

@micmania1
SilverStripe Ltd. member
@micmania1
SilverStripe Ltd. member

Just noticed that some custom functions had been added to the old version. I'll try re-implementing these without altering the plugin so we can easily upgrade in the future.

@tractorcow

Doesn't the new version of this plugin rename some of the CSS classes from chzn -> chosen? If so you might need to check that all scss / css files have been updated. (correct me if I'm wrong on this point).

If doing so, this might be considered an API change and likely won't make it into 3.1.3, if that's what you were hoping to aim for. :)

@chillu
SilverStripe Ltd. member

Thanks micmania! Have you cross browser checked this? IE8/9/10, FF, Chrome?

@micmania1
SilverStripe Ltd. member

@tractorcow I'll double check the CSS when I get back to this.

@chillu I'll make sure that's done before I push back.

Regarding the custom functionality, do you want this as a separate commit? It might make it easier for the reviewer.

@tractorcow

@micmania1 I don't think it would hurt to leave your commits unsquashed while we review it. We'll get you to suash it prior to merge.

Are you going to do the custom function updates in coffeescript?

@micmania1
SilverStripe Ltd. member

@tractorcow I was hoping not to touch the actual plugin this time but i'll not know until I delve in deeper.

I'm hoping I can use one of the plugins events to perform the same actions (chosen:ready or chosen:showing_dropdown).

@micmania1
SilverStripe Ltd. member

I was looking at this last night. I'm not sure I can hook in at the right place.

Does anybody have experience using select2? Would a change to this be an option if I can hook into it without changing the core?

http://ivaynberg.github.io/select2/ - mostly fishing for opinion here.

Update:
Just realised that this supports the rise_up functionality by default so I wouldn't even have to hook into it. Try opening a dropdown near the bottom of the page.

@micmania1
SilverStripe Ltd. member

I've swapped out chosen for select2 to see how it functions and it pretty much 'just works'.

I've made it function as chosen did, however I plan on adding some config options to the DropdownField class which will allow for greater control over the the plugin including pagination to deal with large amounts of data.

I understand that the select2/chosen stuff is only implemented in the backend so consideration will be taken when implementing the changes.

Can one of the core devs check over this and give me their thoughts before I continue as this is turning into quite a time consuming job.

@wilr
SilverStripe Ltd. member

I'm happy for the core library to change since this is in master (not 3.1), editing Chosen would seem like a bad idea :) Note that you could pass plugin config using the existing API and data attributes (which has been handy with the bootstrap stuff) = (new DropdownField('Awesome'))->setAttribute('data-select2-pagination', true); rather than a setPagination API.

@micmania1
SilverStripe Ltd. member

@wilr Thanks, I'll take a look at that.

@simonwelsh simonwelsh added this to the 3.2 milestone Mar 15, 2014
@halkyon
SilverStripe Ltd. member

I'm closing this, we shouldn't be doing "do not merge" pull requests.

@halkyon halkyon closed this Sep 25, 2014
@micmania1 micmania1 referenced this pull request in silverstripe/silverstripe-blog Apr 8, 2015
Closed

Added tag field #174

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