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

Select2 options #16

Closed
wants to merge 5 commits into from
Closed

Select2 options #16

wants to merge 5 commits into from

Conversation

stephane
Copy link
Contributor

This branch fixes issue #15

@applegrew
Copy link
Owner

Thanks again @stephane

However, I do have two comments:-

  • render_select2_options_code is needed for clarity. I feel self.render_select2_options_code(options, id_) is clearer than convert_dict_to_js_map(options, id_). The util function used to render the options is an implementation detail internal to render_select2_options_code().
  • There is a reason for not allowing the users to set random Select2 options. Light Select2Multiple widgets use <select> boxes with multiple attribute set. In this case if you try to pass multiple option to Select2 then it throws error. However, for Heavy Select2Multiple widgets multiple option needs to be passed since there we use <input> boxes. Like wise there are other implementation details internal to Django Select2 which require suppressing some options. The current white-listing approach works well. However, if we use black-list approach then can allow users to set any option while supporting the above requirement.

I have have manually picked the changes from this pull request, and they are now merged into master. Please let me know if see the need to put some changes.

@applegrew applegrew closed this Nov 27, 2012
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