Bad default for SequenceWidget.error_class

[dwt]

In

deform/deform/widget.py

Line 1305 in fae6644

error_class = None

the Widgets default error_class (´´´´error´´´´) is reset to None in commit 4e53d28.

However that doesn't work if you have a validator on the sequence that this sequence widget visualizes, because then if that validator fails the erroneous region cannot be highlighted as it is not marked by any class in the code.

Use case for us: A question has several answers, at least one of those answers has to be true which was modeled as a validator on the colander.SchemaNode that models the collection of answers.

It seems as if the workaround of just setting the error class for the widget to 'error' again fixes the problem, but I don't understand why it was reset to None in the commit mentioned above.

[rbu]

This is still a problem in both deform 1.x and master branches.
@mcdonc, is 4e53d28 still relevant?

[rbu]

@miohtama this is still in the code today, do you understand what the purpose of the commit is supposed to be? Bug is still present.

[miohtama]

Unfortunately I cannot speak for the matters that have happened so long in the past before I was involved. However I am open to a pull requests regarding the matter. With hopefully more comments this time why things are being done as they are done :)

[sydoluciani]

@dwt it's been a long time since you have opened this issue, but we have just updated the error_class in pull request #439 and if you provide the code we would like to test it, otherwise we consider this issue resolved.

Thanks

[dwt]

The code in question + workaround looks roughly like this:

class QuestionEditSchema(YeepaSchema):
    answers_child_schema = AnswerEditSchema
    answers = colander.SchemaNode(
        colander.Sequence(),
        answers_child_schema(
            name="answers",
            title=_(u"Answer"),
        ),
        validator=colander.All(
            colander.Length(min=2),
            colander.Function(
                lambda answers: any(map(itemgetter('is_correct'), answers)),
                _(u"At least one correct answer required."))),
        widget=SequenceWidget(min_len=2),
        title=_(u"Answers"),
    )
    # Workaround for deform bug https://github.com/Pylons/deform/issues/168
    answers.widget.error_class = 'error'

Does that help?

[sydoluciani]

@dwt Thank you.

We set error_class to "is-invalid" in Widget, MappingWidget and SequenceWidget classes on pull request #439 to comply with Bootstrap 4, and once this change marges to master, you can remove the workaround.

Still widget.error_class can be overridden as you did, but the new error class should be made available by developer, if the class is not part of Bootstrap.

[sydoluciani]

@dwt The other way of overriding the error_class would be:

def textinput(self):
        class Schema(colander.Schema):
            text = colander.SchemaNode(
                colander.String(),
                validator=colander.Length(max=100),
                widget=deform.widget.TextInputWidget(
                    error_class = 'CSS_error_class_defined_by_developer'
                ),
                description="Enter some text",
            )
            # Following line overrides error_class set in above TextInputWidget class.
            text.widget.error_class = 'some_other_css_error_class'

        schema = Schema()
        form = deform.Form(schema, buttons=("submit",))

        return self.render_form(form)

I believe we can close this issue after merging the pull request #439, but please test and let us know if you noticed any problem.

Thank you

[stevepiercy]

@sydoluciani @dwt I merged #439 to master.

Note that this PR would need to be adapted for Bootstrap 3.4.1 if we backport it to the Deform 2.0-branch. I would accept a separate PR to the 2.0-branch on both Deform and deformdemo for this bug fix.

[sydoluciani]

@stevepiercy OK, let me know when 2.0-branch is passed all the tests, then I fork from branch 2 and apply bug fixes to branch 2 some time next week.

We keep this issue open until the same applied on 2.0-branch.

Thanks

[stevepiercy]

2.0-branch on both Deform and deformdemo are ready for backports.