zopefoundation / z3c.form

An advanced form and widget framework for Zope 3
Other
8 stars 39 forks source link

try to fix #89 #90

Closed d-maurer closed 4 years ago

d-maurer commented 4 years ago

This PR tries to fix #89. It changes the template checkbox_hidden.pt to produce the expected markup (compatible to the one produced by checkbox_input.pt) and adapts the tests in checkbox.rst accordingly.

But I have no confidence in the tests and in the implementation of checkbox.SingleCheckboxWidget.

The basic CheckboxWidget is designed as a multiple option widget. As such, it is controlled by a sequence of terms (the possible options) and its value is naturally a sequence of the selected options. Accordingly, its isChecked method is defined as:

    def isChecked(self, term):
        return term.token in self.value

However, the tests (in checkbox.rst) always assign strings to widget.value (instead of a sequence of strings). This works accidentally, because in can accept both a sequence as well as a string as right argument. But, applied to strings, it can wrongly return True.

We can expect (from the name) that SingleCheckboxWidget is designed for a single value. Thus, its value may be a string. However, it should then get its own IsChecked method definition reflecting this type.

SingleCheckboxWidget can be expected to be used as a widget for boolean fields. By default, it has a single term selected. I am not convinced that this is the most natural representation of True and therefore, I am not sure that a field value True is really mapped to the widget value 'selected'(which is necessary to make it work correctly).