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

Inherit select classes #917

Merged
merged 5 commits into from Dec 12, 2012
Merged

Inherit select classes #917

merged 5 commits into from Dec 12, 2012

Conversation

pfiller
Copy link
Contributor

@pfiller pfiller commented Nov 29, 2012

As requested in #209 and #418, this carries over the classes from the original select element to the chosen container div. It also cleans up the class logic a bit so it's easier to add/remove classes in the future.

2 questions

  1. Should this be enabled only if an option is set to true (something like keep_select_classes) or should it be on by default?
  2. Should it be possible to pass a list of classes to exclude from being copied over? Or should it just grab anything there?

Would appreciate reviews and opinions from @stof @jkintscher.

@stof
Copy link
Collaborator

stof commented Nov 29, 2012

This looks good to me

Conflicts:
	chosen/chosen.jquery.min.js
	chosen/chosen.proto.min.js
	coffee/chosen.proto.coffee
@pfiller
Copy link
Contributor Author

pfiller commented Dec 11, 2012

@stof I've added an option to make class inheritance a non-default option. I think there are a number of scenarios where someone would not want Chosen to have the same classes as a select and I think this is an easy safeguard against that. What do you think?

@stof
Copy link
Collaborator

stof commented Dec 11, 2012

looks good to me

@pfiller pfiller merged commit dc66716 into master Dec 12, 2012
@pfiller
Copy link
Contributor Author

pfiller commented Dec 12, 2012

Thanks, @stof

pfiller added a commit that referenced this pull request Dec 12, 2012
- #824, #940 Add an option to disable searching split words
- #917 Add an option to inherit initial select classes
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.

None yet

2 participants