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

[in progress] Make version that doesn't rely on JS library #1380

Open
Daniel15 opened this issue Jul 21, 2013 · 17 comments
Open

[in progress] Make version that doesn't rely on JS library #1380

Daniel15 opened this issue Jul 21, 2013 · 17 comments

Comments

@Daniel15
Copy link

I'd like to use Chosen on one of my sites, but I am not using a JavaScript library (all the JS is "vanilla"). I'd like to see a version of Chosen that does not depend on jQuery or Prototype.

Edit: I've started on this, expect to hear back from me some time soon :) 👍

@Daniel15
Copy link
Author

I'm going to look into this once I get time to, unless someone else has already done it. :)

@koenpunt
Copy link
Collaborator

I was thinking about this recently, and as the AbstractChosen class is already framework independent, we only have to 'translate' the jquery or prototype version.

@kenearley
Copy link

This has been a goal for @pfiller for quite a while. A lot of the recent updates have been moving the code in that direction.

@Daniel15
Copy link
Author

Yeah what I've done so far is I've made a copy of "chosen.jquery.coffee", called it "chosen.native.coffee", and am going through removing all the jQuery-specific code. I'm using a diff tool to compare chosen.jquery.coffee and chosen.prototype.coffee and looking at all the lines that differ between the two. I'll commit what I've done once it's in a usable state, probably some time this week.

The chosen.jquery.coffee and chosen.prototype.coffee are quite big and have lots of overlapping code. I think it'd be good to pull out the framework-specific code (adding and removing CSS classes, attaching event handlers, DOM manipulation, etc.) and put it in a small facade. History.js did this pretty well and the adapters only contain the smallest amount of code required.

Depending on how much extra code the "native" version needs, the jQuery and Prototype versions may not even be required in the future. I looked through the whole file last night and the majority of it seems to be relatively straightforward and things that are easy to do with "vanilla" JavaScript (attaching events, inserting elements to the DOM)

@Daniel15
Copy link
Author

I spent a bit of time on this tonight, you can see what I've done so far at https://github.com/Daniel15/chosen/tree/vanilla-no-framework (clone that and open index.native.html). I've gotten the controls to all initialise without errors, and basic functionality of the standard selects (typing to search, keyboard navigation, clicking to select) are working properly in modern (non-IE) browsers.

Please let me know if I'm heading in the right direction. This is the first major CoffeeScript project I've worked on, so let me know if there's anything I can improve :)

FYI @koenpunt @kenearley @pfiller

@kenearley
Copy link

This is looking very promising. I've just noticed a few issues, such as opening/reopening selects. It also appears you may not have the latest changes that are now in master.

Initial Thoughts

  • Is it acceptable for us to support IE10+ for the native version?
  • Are we willing to maintain 3 different versions?
    • Ultimately, I think we would use native js going forward, jQuery for oldIE support, and drop Prototype (maybe)

@stof
Copy link
Collaborator

stof commented Jul 22, 2013

@kenearley If we keep the jQuery version, supporting IE10+ is fine IMO if we make it clear in the doc to avoid people complaining. Duplicating the work of the jQuery team to support old IE flaws seems pointless.
I vote for this solution as keeping only the native version with IE8+ support would require duplicating work.

And we can probably strip down the jQuery-specific code by overwriting methods only for the parts needing a special handling for jQuery instead of the full method. for instance, we could have as hasClass method (like done in the PR aboveso code needing to check a class could be kept in AbstractChosen and only thehasClass`` method would be in the child class.

@Daniel15
Copy link
Author

@kenearley Yeah, it's definitely not working 100% yet and it's still quite fragile, but a lot of the basic framework is there. I pulled from Github two or three days ago so I'll have to get the latest changes from master and merge them in. Feel free to point out any issues you find (or contribute bug fixes if you like).

I think support for IE 8+ is still possible with a vanilla JS version. The main issues with IE are well-known, well-defined, and don't require significant effort (like the different event handling model). IE 8 is significantly better than IE 6 and 7 and a lot of the old IE issues aren't relevant any more. That and some of the hacks for IE are also required for other browsers (eg. no ClassList support in Android 2.3 or old iOS). I've not yet tested in IE but I'll keep a note of all the issues I encounter and how easy they are to fix.

@stof I agree, the classes could be rearranged a bit so that the jQuery version is a thin facade over the native version (I've seen other controls do this) with very little functionality. Basically just wrapping the native control in a familiar API pattern. Similar lightweight wrapper classes could be added for other libraries (like AngularJS as per #1118, and YUI). I'm staying away from major refactoring of AbstractChosen at the moment, and keeping everything in a new file.

Prototype support is worth keeping as in the context of Chosen there's nothing jQuery can do that Prototype can't do. At my work place we chose Chosen over Select2 as Chosen had support for Prototype (although a native version would have been even better). We are not using it with new development due to the accessibility issues (#264) though.

@Daniel15
Copy link
Author

@harvesthq/chosen-developers My code still has a few bugs but I think it's at a point where I'd like to get code style input from the official developers and have it merged into the mainline code (perhaps in a separate branch for now, like I've done in my fork) and do more enhancements and cleanup.

There's some odd bugs with reopening selects that I haven't had a chance to track down yet.

There's a bunch of helper methods at the top of chosen.native.coffee. I wasn't too sure where to put these - They're not specific to the chosen control so it didn't feel right putting them inside the Chosen class.

IE 8 compatibility is almost there - Firing custom events doesn't work properly in IE 8 (jQuery and Prototype both have hacks to deal with this). I didn't bother too much with IE 8 testing (apart from things I already knew about), based on the comments above. IE 9 + seems fine except for some minor bugs that appear in other browsers too.

@stof
Copy link
Collaborator

stof commented Jul 24, 2013

@Daniel15 Please open a pull request. It is the best way to give you feedback when reviewing it.

@Daniel15
Copy link
Author

I was just worried about submitting a pull request for unfinished code.

@stof
Copy link
Collaborator

stof commented Jul 24, 2013

a pull request is about reviewing submissions and it is an awesome tool for that.
If it is not ready, you can add [WIP] in its title to make it clear.

@kenearley
Copy link

I was just worried about submitting a pull request for unfinished code.

We typically write "NOT READY FOR MERGE" at the top of the pull request and then create a tasklist of todos in the description.

@craigcosmo
Copy link

Is this idea still on the road map? It will be great.

@florianajir
Copy link

👍

@jshjohnson
Copy link

Shameless plug, but for anyone interested I decided to create me own chosen-style plugin that is not dependent on a framework: https://github.com/jshjohnson/Choices

@stof
Copy link
Collaborator

stof commented Oct 18, 2018

If we go this way, there is a thing to be careful about: we expect our users to trigger a chosen:updated event that we listen to. But if we listen using the native APIs, it requires dispatching the event with native APIs, not with jQuery. jQuery.trigger does not dispatch a native event. It only works for listeners registered with jQuery.. So that would either force doing a new major version, or keep registering that listener with jQuery when available.

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

No branches or pull requests

7 participants