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
Add for attribute in label for RadioSelect #699
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #699 +/- ##
=======================================
Coverage 95.66% 95.66%
=======================================
Files 24 24
Lines 2560 2560
Branches 223 223
=======================================
Hits 2449 2449
Misses 68 68
Partials 43 43 Continue to review full report at Codecov.
|
@@ -5,7 +5,7 @@ | |||
{% include 'bootstrap/layout/field_errors_block.html' %} | |||
|
|||
{% for choice in field.field.choices %} | |||
<label class="radio{% if inline_class %} {{ inline_class }}{% endif %}"> | |||
<label for="id_{{ field.html_name }}_{{ forloop.counter }}" class="radio{% if inline_class %} {{ inline_class }}{% endif %}"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds like this isn't necessary if the input
is within the label:
A label element can have both a for attribute and a contained control element, as long as the for attribute points to the contained control element.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but, in some cases when you need to hide the input to use after and/or before pseudo elements this tweak keeps onclick behaviour.
On the other hand, i'll try it with a contained input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to merge this then. Don't see any reasons not to.
Can you resolve the conflicts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course! 😀
Thanks!
LGTM 👍 I've made the change for Bootstrap 3 and 4 as well. As soon as the tests are done, we can merge. |
No description provided.