Make sure that field errors are always visible

[xi]

Fixes #763 and potentially #749

I also fixed the .input-group related issue I mentioned in #763 (comment).

I implemented this as an alternative to #766 because I thought that just overwriting bootstrap's display: none is more robust than adding dummy elements.

[coveralls]

Pull Request Test Coverage Report for Build 18627842736

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 97.917%

---

Totals
Change from base Build 18627808431:	0.0%
Covered Lines:	1433
Relevant Lines:	1454

---

💛 - Coveralls

[ppfeufer]

This would be the correct HTML:

{% if field_errors %}
    <div id="{{ field.auto_id }}_error" class="invalid-feedback">
        {% for text in field_errors %}
            <div>{{ text }}</div>
        {% endfor %}
    </div>
{% endif %}

As Bootstrap targets the directly followed element of .is-invalid via CSS via the ~ selector, here .is-invalid~.invalid-feedback:

.is-invalid~.invalid-feedback, .is-invalid~.invalid-tooltip, .was-validated :invalid~.invalid-feedback, .was-validated :invalid~.invalid-tooltip {
    display: block;
}

No need to "manually" add the display or width classes.

Credit goes to @christianwgd in #763 (comment)

[xi]

That would not fix #749, would it?

[ppfeufer]

It should, since it's the same markup required and as long as the template is injected at the right place.
I don't have a form with these kinds of fields, but I don't see a reason why it shouldn't fix this.

[ppfeufer]

If it has worked before, then it should work now.
The only difference is that - now - the wrapper needs the class, not the error message div itself.

[ppfeufer]

It definitely works with multiple fields …

[ppfeufer]

You just merged the wrong markup.
The .invalid-feedback class is on the wrong div here, see comments above and the discussion here and following …

Also, the extra classes (.w-100 and .d-block) are unnecessary and will make it harder for devs using this lib to override the default Bootstrap behaviour if needed. Divs are already block elements and are naturally 100% width and display block.

[dyve]

Should we revert #767?

[ppfeufer]

Should we revert #767?

And replace with the proper markup - #767 (comment)

{% if field_errors %}
    <div id="{{ field.auto_id }}_error" class="invalid-feedback">
        {% for text in field_errors %}
            <div>{{ text }}</div>
        {% endfor %}
    </div>
{% endif %}

[xi]

Should we revert #767?

I think I explained the reasons why I prefer this solution over the one pushed by @ppfeufer pretty well in #763. It is up to you to decide which one you think works best.