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

Fix territory form label clicks not toggling correct input #784

Merged
merged 2 commits into from
Feb 4, 2024

Conversation

mzivil
Copy link
Contributor

@mzivil mzivil commented Feb 3, 2024

On the territory form, clicking on the labels of the "post types" or "billing" options don't toggle the correct input:

Feb-03-2024.16-49-26.mp4

All the labels within each group were being assigned the same html "id" since we were falling back to the "name" prop. We need to keep the "name" prop the same because that's how the form associates the value with the correct form field. Fix is to assign explicit "id" prop to each checkbox so that the label toggles the correct input.

Feb-03-2024.16-55-58.mp4

Currently, all the post types checkbox are being assigned the
same "postTypes" id, so clicking on any of the post type labels
always toggles the first one. If you inspect the HTML, all the
post type labels have 'for="postTypes"' which is how the HTML
knows which checkbox to toggle.

We have to keep the "name" attribute the same because that's how
the checkbox values are linked to the postTypes field.

To fix, we explicitly assign the id prop for each checkbox so
that the <label>'s "for" attribute is tied to the correct
checkbox input.
Currently, all the billing types radios are being assigned the
same "billingType" id, so clicking on any of the labels
always selects the monthly one. If you inspect the HTML, all the
billing type labels have 'for="billingType"' which is how the HTML
knows which input to select.

We have to keep the "name" attribute the same because that's how
the input values are linked to the billingType form field.

To fix, we explicitly assign the "id" prop for each radio so
that the <label>'s "for" attribute is tied to the correct
radio input.
@mzivil mzivil marked this pull request as ready for review February 3, 2024 22:00
@huumn
Copy link
Member

huumn commented Feb 3, 2024

Dang how did no one report that!

@huumn huumn merged commit 42e491b into stackernews:master Feb 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants