Closed blasferna closed 5 months ago
Thanks for the PR.
Appologies for the slow uptake here. There's a new filter that has been merged into crispy-forms "core". See https://github.com/django-crispy-forms/django-crispy-forms/pull/1127 and here it is in action https://github.com/django-crispy-forms/django-crispy-forms/pull/1155. Once that PR is merged I'll issue a release so we can use it here.
I'm hoping this filter should simplify this template significantly. Although I've not tested it would be my expecation that this style would also give us the "multiple" attribute as well.
I think it's great. I'll be waiting for the notification to see how to simplify this template with the new crispy-form filter.
@smithdc1 I implemented the new filter optgroups
. The template is simpler, but I had to add the following portion in the definition of the select
:
{% if field.field.widget.allow_multiple_selected %}multiple {% endif %}
{% if field.field.required %}required {% endif %}
Any suggestions for improvement?
Wow!, the tests failed, apparently I broke something. As I have verified, it is not so easy to include the required
attribute.
According to Select
widget definition i need a extra check:
def use_required_attribute(self, initial):
"""
Don't render 'required' if the first <option> has a value, as that's
invalid HTML.
"""
use_required_attribute = super().use_required_attribute(initial)
# 'required' is always okay for <select multiple>.
if self.allow_multiple_selected:
return use_required_attribute
first_choice = next(iter(self.choices), None)
return use_required_attribute and first_choice is not None and self._choice_has_empty_value(first_choice)
I will find a way to fix it.
How can we help get this fix for #98 merged?
Hey @rposborne -- have you reviewed this, does this fix the issue for you?
hitting this issue in my current codebase how can I pull this solution in?
Hi @GitRon,
I've been working on this pull request for a long time. It addresses some issues related to the lack of attributes in the select
. Could you please review it?
Hi @blasferna
there seem to be some merge conflicts. Could you fix them, please? Could you tell me how to best test your changes?
Thx!
Hi @GitRon,
I have decided to close this PR as I am working on a new approach that I believe will be more effective and organized. I will soon open a new PR with these changes.
Alright, looking forward to it!
Fixes #97 #98