Skip to content

Expose Chosen to the window object. #929

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

Merged
merged 1 commit into from
Dec 5, 2012
Merged

Conversation

pfiller
Copy link
Contributor

@pfiller pfiller commented Dec 5, 2012

Fixes #924

The Prototype version exposes the Chosen class because that's the only way to use it. The jQuery version doesn't typically need to do that because usage goes through jQuery itself. This means that if you want to extend the class or something, you'd have to do it in the Chosen source.

@stof @jkintscher This just mirrors the Prototype version and should be of minimal consequence. It exposes another global variable, but that's about the extent of its impact.

@pfiller pfiller mentioned this pull request Dec 5, 2012
@stof
Copy link
Contributor

stof commented Dec 5, 2012

good for me

@stof
Copy link
Contributor

stof commented Dec 5, 2012

btw, does the SelectParser and AbstractChosen really need to be exposed as global variables ? Couldn't we do the scoping of Chosen around the whole compiled file instead of doing it separately for each coffee file ?

@jkintscher
Copy link
Member

You could even make it this: class root.Chosen extends AbstractChosen.

@pfiller
Copy link
Contributor Author

pfiller commented Dec 5, 2012

I agree that we should not expose those methods. When we start switching the build method (to Grunt), we should make that change as well.

pfiller added a commit that referenced this pull request Dec 5, 2012
@pfiller pfiller merged commit 979091a into master Dec 5, 2012
pfiller added a commit that referenced this pull request Dec 5, 2012
Includes these Pull Requests:

- #902 Fiexd Firefox mousedown drag issue
- #914 Handle select title
- #916 Fixed the handling of ctrl key for multiple select
- #920 Fixed multiple select placeholder clipping bug
- #929 Expose Chosen to the window object

Refer to these pull requests for details and diffs.
@angelyordanov
Copy link

Hey @pfiller what do you think about adding AMD support so that chosen is not polluting the window or jQuery objects. I have created a pull request for that but just for the jQuery version. I can change that for the prototype as well.

It does require a build change though.

@pfiller
Copy link
Contributor Author

pfiller commented Dec 6, 2012

Hi @angelyordanov - I saw your pull and plan to give it a proper look-over before offering an opinion. I'm not super familiar with how AMD works, so I'll need to take some time and familiarize myself. Chosen has a ton of open issues and we're very slowly digging ourselves out of the hole. Thanks for being patient.

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.

Expose Chosen Object
4 participants