Skip to content
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

Form template renders radio field inputs with same id #156

Open
calebsyring opened this issue Sep 13, 2021 · 3 comments · May be fixed by #157
Open

Form template renders radio field inputs with same id #156

calebsyring opened this issue Sep 13, 2021 · 3 comments · May be fixed by #157
Assignees

Comments

@calebsyring
Copy link
Contributor

calebsyring commented Sep 13, 2021

The default radio field renders with multiple inputs with same id.

{% for value, label, checked in field.iter_choices() %}
<div class="radio">
<label>
<input type="radio"
name="{{ field.id }}"
id="{{ field.id }}"

@bchopson
Copy link
Member

perhaps id="{{ field.id }}-{{ field.value }}"

@calebsyring calebsyring self-assigned this Sep 13, 2021
@calebsyring calebsyring linked a pull request Sep 13, 2021 that will close this issue
@rsyring
Copy link
Member

rsyring commented Sep 15, 2021

Is there a reason we are even setting id on these inputs? You should be able to target them in tests based on index position (since you control the data you know what index you want. In JS, it would be just as easy to target based on name or value attributes. Actually, you can do the same with PyQuery.

FWIW, I try to avoid using IDs for anything unless really needed due to the issue of accidental duplication.

I don't like just using lower b/c you could have characters in a value that aren't valid in the id attribute. I'd suggest using blazeutils simplify instead as the filter.

My preferences would be:

  1. Remove id attribute and you no longer need to worry about it being accidentally duplicated
  2. Use the loop value
  3. Use simplify on the value instead of lower.

/cc @matthiaswh, @guruofgentoo, @bchopson to weigh in.

@bchopson
Copy link
Member

I believe the id for inputs is so that a <label> can reference that input via its for attribute. Setting for explicitly is helpful for screen readers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants