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

Passing on validators to select subfields seems wrong #715

Open
ThiefMaster opened this issue Dec 2, 2021 · 4 comments
Open

Passing on validators to select subfields seems wrong #715

ThiefMaster opened this issue Dec 2, 2021 · 4 comments

Comments

@ThiefMaster
Copy link

This was added in #615 but it breaks custom select field subclasses that use e.g. a list of checkboxes as they now all get the required attribute.

Here's a failing test case (since both inputs get the required flag):

def test_required_flag_custom():
    class CustomWidget:
        def __call__(self, field, **kwargs):
            html = []
            for subfield in field:
                html.append(f'{subfield.label}: {subfield()}')
            return ''.join(html)

    class CustomSelectMultipleField(SelectMultipleField):
        widget = CustomWidget()
        option_widget = CheckboxInput()

    F = make_form(
        c=CustomSelectMultipleField(
            choices=[("a", "hello"), ("b", "bye")],
            validators=[validators.DataRequired()],
        )
    )
    form = F(DummyPostData(c=["a"]))
    assert form.c() == (
        '<label for="c-0">hello</label>: <input checked id="c-0" name="c" type="checkbox" value="a">'
        '<label for="c-1">bye</label>: <input id="c-1" name="c" type="checkbox" value="b">'
    )

I have the feeling that passing validators to subfields isn't the correct thing to do - only <input type="radio"> handles the required flag correctly, but of course for multi-select you'll never have radio buttons...

@homeworkprod
Copy link

Yes, please.

Is there a workaround?

@homeworkprod
Copy link

For the record, I had success fixing this issue for myself by using the patch done by @ThiefMaster in indico/indico#5185.

Would be great, though, to have this fixed in WTForms itself somehow.

@azmeuk
Copy link
Member

azmeuk commented Jan 6, 2024

Reproducible with:

>>> import wtforms
>>> import wtforms.widgets
>>> from werkzeug.datastructures import ImmutableMultiDict
...
>>> class CustomWidget:
...     def __call__(self, field, **kwargs):
...         html = []
...         for subfield in field:
...             html.append(f'{subfield.label}: {subfield()}')
...         return ''.join(html)
...
>>> class CustomSelectMultipleField(wtforms.SelectMultipleField):
...     widget = CustomWidget()
...     option_widget = wtforms.widgets.CheckboxInput()
...
>>> class F(wtforms.Form):
...     c=CustomSelectMultipleField(
...         choices=[("a", "hello"), ("b", "bye")],
...         validators=[wtforms.validators.DataRequired()],
...     )
...
>>> form = F(ImmutableMultiDict({"c":["a"]}))
>>> assert form.c() == (
...     '<label for="c-0">hello</label>: <input checked id="c-0" name="c" type="checkbox" value="a">'
...     '<label for="c-1">bye</label>: <input id="c-1" name="c" type="checkbox" value="b">'
... ), form.c()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AssertionError: <label for="c-0">hello</label>: <input checked id="c-0" name="c" required type="checkbox" value="a"><label for="c-1">bye</label>: <input id="c-1" name="c" required type="checkbox" value="b">

@azmeuk
Copy link
Member

azmeuk commented Jan 7, 2024

I am not sure how to do with this issue. render_kw being passed to sub-fields comes from #688
The idea was to the required flag could be passed to radio inputs:
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/radio#required
However, indeed it seems awkward for select fields, and since #739 subfield render_kw can be set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants