html: add radio buttons

[fscherf]

This PR adds initial support for HTML radio buttons (#229)

[codecov-commenter]

Codecov Report

Patch coverage: 89.09% and project coverage change: +0.13% 🎉

Comparison is base (50f2c78) 72.63% compared to head (c974645) 72.77%.
Report is 8 commits behind head on master.

❗ Current head c974645 differs from pull request most recent head bd7638e. Consider uploading reports for the commit bd7638e to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #435      +/-   ##
==========================================
+ Coverage   72.63%   72.77%   +0.13%     
==========================================
  Files          84       85       +1     
  Lines        5760     5866     +106     
  Branches     1238      959     -279     
==========================================
+ Hits         4184     4269      +85     
- Misses       1311     1329      +18     
- Partials      265      268       +3     

Files Changed	Coverage Δ
lona/html/nodes/forms/radio_button.py	88.34% <88.34%> (ø)
lona/html/__init__.py	100.00% <100.00%> (ø)
lona/html/attribute_dict.py	71.17% <100.00%> (+1.07%)	⬆️

... and 13 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[SmithChart]

Nice to see some progress here :)

I like that even the RadioGroup feels like an HTML element in this context (even if HTML does not specify such node).

Do you see some room for a (specific) high-level interface like the one suggested here in Lona? Or should such interfaces live somewhere else? It feels wrong to have a custom widget in every project.

[SmithChart]

I guess returning None if no RadioButton is checked is a good way to handle this state.
So in the same way setting value = None creates a list where no RadioButton is checked.

But: If I try to set a value that is not part of a RadioGroup I would suggest to raise an ValueError instead of silently not checking any RadioButton.

[SmithChart]

The HTML5 specification explicitly does not allow nesting of <form>. Even if it feels like there are not many use-cases where one want's to use actual HTML forms in Lona. Why make using them extra hard?

Is there any gain from using form here instead of an element like div that is intended for nesting?

[fscherf]

Nice to see some progress here :)

Yeah, sorry :D

Do you see some room for a (specific) high-level interface like the one suggested here in Lona? Or should such interfaces live somewhere else? It feels wrong to have a custom widget in every project.

I was thinking about something like lona.html.RadioGroup.add_button() too, but I am not sure which HTML layout such a method should produce. In picocss the default layout seems to be this:

<label>
    My Label
    <input type="radio" value="my-value">
</label>

This seems to work fine in all cases I encountered but the "more HTML5" layout would be like this:

<label for="my-radio-button>My Label</label>
<input type="radio" id="my-radio-button" value="my-value">

[...] If I try to set a value that is not part of a RadioGroup I would suggest to raise an ValueError instead of silently not checking any RadioButton.

I agree. lona.html.Select2 implements exactly this behavior so this should be consistent anyway.

The HTML5 specification explicitly does not allow nesting of <form>. Even if it feels like there are not many use-cases where one want's to use actual HTML forms in Lona. Why make using them extra hard?

Is there any gain from using form here instead of an element like div that is intended for nesting?

Unfortunately, radio groups have no dedicated HTML tag but are identified by multiple radio buttons using the same name. When using a div as radio group, your code would have to look like this:

RadioGroup(
    RadioButton(value='foo', name='my-group'),
    RadioButton(value='bar', name='my-group'),
    RadioButton(value='baz', name='my-group', checkd=True),
)```

If possible I don't want a mandatory name field that is not used by the application code since the `RadioGroup` object is the handle to the checked value. When using a `<form>` `RadioButton.name` can be generic, when using `<div>` not.

[SmithChart]

I was thinking about something like lona.html.RadioGroup.add_button() too, but I am not sure which HTML layout such a method should produce. In picocss the default layout seems to be this:

<label>
    My Label
    <input type="radio" value="my-value">
</label>

This seems to work fine in all cases I encountered but the "more HTML5" layout would be like this:

<label for="my-radio-button>My Label</label>
<input type="radio" id="my-radio-button" value="my-value">

So, if both work, why care which one to use? (But I personally would prefer to stick closer to what HTML5 suggests.)
In my use-cases the choices for a RadioGroup often come from a (filtered) Django DB choice. Having add_button() would reduce boilerplate to:

choice = (("v1", "Display Value V2"), ("v2", "Display Value V2")) # already in the DB model

rg = RadioGroup()
[rg.add_button(v, k) for k,v in l]

Unfortunately, radio groups have no dedicated HTML tag but are identified by multiple radio buttons using the same name. When using a div as radio group, your code would have to look like this:

RadioGroup(
    RadioButton(value='foo', name='my-group'),
    RadioButton(value='bar', name='my-group'),
    RadioButton(value='baz', name='my-group', checkd=True),
)```

If possible I don't want a mandatory name field that is not used by the application code since the `RadioGroup` object is the handle to the checked value. When using a `<form>` `RadioButton.name` can be generic, when using `<div>` not.

OK, and when using a form instead of a div the default name is enough to group the RadioButtons, I see.
In this case using form is better than having some hidden magic in RadioGroup that takes care of the renaming.

[fscherf]

So, if both work, why care which one to use? (But I personally would prefer to stick closer to what HTML5 suggests.)
In my use-cases the choices for a RadioGroup often come from a (filtered) Django DB choice. Having add_button() would reduce boilerplate to: [...]

Fair enough. If we decide on a RadioGroup.add_button() I would want to use the second layout, the more HTML5-ish, too. I have a second concern though: We have lona.html.Select and lona.html.Select2. In Lona 2, lona.html.Select will be fully replaced by lona.html.Select2 so I am only concerned with this implementation.

lona.html.Select used this API to define options:

from lona.html import Select

Select(
    values=[
        # value, label, is_selected
        ('foo', 'Foo', True),
        ('bar', 'Bar', False),
    ],
)

The comment pretty much marks the problem: It is easy to get the order of the options wrong, and there are cryptic flags needed to set properties on the resulting lona.html.Option objects. This works ok-ish for a few options and arguments but becomes a problem if you add more arguments. Also adding HTML attributes, or style attributes is a problem.

Therefore lona.html.Select2.add_option() takes whole lona.html.Option2 objects, so code like this is possible:

s = Select2()

s.add_option(Option2('Option 1', value='option-1', selected=True))
s.add_option(Option2(Span('Option 2'), value='option-2', style={'color': 'red'}))

It's more verbose, but also more flexible.

To be consistent with this API lona.html.RadioGroup.add_button() would have to look like this:

r = RadioGroup()

r.add_button(
    Label('Option 1', style={'color': 'red'}),
    RadioButton(value='option-1'),
)

r.add_button(
    Div(  # using a div to make the listing go top to bottom instead left to right
        Label('Option 1', style={'color': 'red'}),
        RadioButton(value='option-1'),
    ),
)

The add_button then could search for any given labels and set the for attribute, so clicking in the browser works as expected.

[fscherf]

@SmithChart: I added an implementation for RadioGroup.add_button(). The method takes a variable list of nodes, searches for Label and RadioButton objects, and links them together, using the for attribute. So all of these calls work:

radio_group.add_button(Label('foo'), RadioButton(value='foo'))
radio_group.add_button(Div(Label('foo'), RadioButton(value='foo')))
radio_group.add_button(Label('foo', RadioButton(value='foo')))

For convenience, I added a special case that makes code like this work too:

radio_group.add_button('Foo', 'foo')

I am not sure if this is too much magic, but it seems to be useful. What do you think?

[SmithChart]

@SmithChart: I added an implementation for RadioGroup.add_button(). The method takes a variable list of nodes, searches for Label and RadioButton objects, and links them together, using the for attribute. So all of these calls work:

radio_group.add_button(Label('foo'), RadioButton(value='foo'))
radio_group.add_button(Div(Label('foo'), RadioButton(value='foo')))
radio_group.add_button(Label('foo', RadioButton(value='foo')))

For convenience, I added a special case that makes code like this work too:

radio_group.add_button('Foo', 'foo')

I am not sure if this is too much magic, but it seems to be useful. What do you think?

I get the point on magic arguments and flags you made for the Select.
For a RadioButton I think two magic arguments are still acceptable. This should be easily discoverable. And the long form from above gives full flexibility.

So: I think I like this API.

[fscherf]

@SmithChart: Great! Thanks for your opinion on that. I will finish this PR, update the docs, and ping you for the final review

[SmithChart]

@fscherf I've reviewed your changes and gave it a try on a project I had open.

[SmithChart]

Should this be part of this PR?

[fscherf]

Not necessarily, but it isn't a problem either because it is a separate patch. While implementing this, I ran into some issues and wanted to use the new exception pretty printing of Python 3.11.

[SmithChart]

checked=True is an argument for the RadioButton, not the Label.

[fscherf]

Good catch! Fixed.

[SmithChart]

Initializing a RadioGroup via the constructor is not implemented! Using it this way Labels and RadioButtons are not linked together with the for= attribute.
Using radio_group.add_button() works however.

[fscherf]

That is due to the weird implementation of radio buttons in HTML. Correlating which Label is linked to which RadioButton is tricky when you get a whole tree of both. That's why, in this example, the RadioButton objects are inside of the Label objects. That way they are linked together by the browser implicitly. In add_button() that's not a problem, because you are supposed to provide only one label and one input with each call.

The RadioGroup.value is initialized correctly in this example because value is a property that searches for a RadioButton that is checked on every call. It is not a static value.

[SmithChart]

OK. Good point. Having the nodes in this way makes the result look funny. But add_button() works as expected. So no further action needed here.

[fscherf]

@SmithChart: This PR should be ready to merge now I think.

[SmithChart]

LGTM

@fscherf feel free to close the resolved discussions. I can't do that from here.

[fscherf]

@SmithChart: Great! Thanks a lot for your help on this topic, and for your patience. I know it took a long time.