Skip to content

[TwigBridge] [Bootstrap 4] Fix validation error design for expanded choiceType #24837

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

Merged
merged 1 commit into from
Nov 9, 2017

Conversation

ostrolucky
Copy link
Contributor

@ostrolucky ostrolucky commented Nov 6, 2017

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Unfortunately I didn't test #24802 with expanded option set to true, sorry about that. Without this fix, it applies form error style twice.

Before:
screenshot from 2017-11-06 10 31 55

After:
screenshot from 2017-11-06 10 32 40

@ostrolucky
Copy link
Contributor Author

ostrolucky commented Nov 6, 2017

Note that this could break user space choice widgets which rely on is valid :/ But hey, this is BS4 only and they can still use errors|length. Still, better ideas welcome.

@@ -105,6 +105,7 @@
{%- else -%}
{%- if not valid -%}
{%- set attr = attr|merge({class: (attr.class|default('') ~ ' form-control is-invalid')|trim}) %}
{% set valid = true %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about instead just passing true in the form_widget() call below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that works too. Thanks

@nicolas-grekas nicolas-grekas changed the title [TwigBridge] [Bootstrap 4] Fix validation error design for expanded c… [TwigBridge] [Bootstrap 4] Fix validation error design for expanded choiceType Nov 7, 2017
@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Nov 7, 2017
@fabpot
Copy link
Member

fabpot commented Nov 9, 2017

Thank you @ostrolucky.

@fabpot fabpot merged commit 39083a2 into symfony:3.4 Nov 9, 2017
fabpot added a commit that referenced this pull request Nov 9, 2017
… expanded choiceType (ostrolucky)

This PR was merged into the 3.4 branch.

Discussion
----------

[TwigBridge] [Bootstrap 4] Fix validation error design for expanded choiceType

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Unfortunately I didn't test #24802 with expanded option set to true, sorry about that. Without this fix, it applies form error style twice.

Before:
![screenshot from 2017-11-06 10 31 55](https://user-images.githubusercontent.com/496233/32434235-c13c02f8-c2dd-11e7-97da-6bfa312c5825.png)

After:
![screenshot from 2017-11-06 10 32 40](https://user-images.githubusercontent.com/496233/32434252-d30c2224-c2dd-11e7-86d6-fd06af3ef753.png)

Commits
-------

39083a2 [TwigBridge] [Bootstrap 4] Fix validation error design for expanded choiceType
This was referenced Nov 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants