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

Stop use of nested label tags in HTML #738

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

consideRatio
Copy link
Member

@consideRatio consideRatio commented May 30, 2023

There is a slight adjustment in the alignment as part of changing the nested <label> tag into a <span> tag, but I think that is preferred. In the gif below we see the difference switched back and forth. With this PR it becomes as it is when the label is put lower compared to what it was before, aligning better with the text in the dropdown lists.

alignment

@minrk
Copy link
Member

minrk commented May 31, 2023

I don't think this is quite the right solution. We do have two form elements here (the profile and the image), so each should have its label. What I think is incorrect is that the image control is inside the profile selection label.

Instead, I think the right thing to do is to make sure the profile item label does not enclose the image form element and label.

@consideRatio
Copy link
Member Author

@minrk the background is observing that nesting <label> tags isn't a well defined HTML it seems.

For example we had a label for each radio button, and when click on a label with an associated radio button, they are supposed to be selected. But, what if the label has a label inside it - then what?

In https://stackoverflow.com/a/20842000/2220152, a person leads with:

As you know, it is not allowed to nest label elements,

And another post sais:

The html5 spec explicitly says:

The label element must not contain any nested label elements.

I have not found something in the html5 specification or similar saying this explicitly, but I've seen a validator warning about it and these stackoverflow responses.

@minrk
Copy link
Member

minrk commented May 31, 2023

Thanks! I do understand the background, and that there shouldn't be a label inside another. What I mean is that the second label, which is for a real control, shouldn't be inside the first. Not that the second label shouldn't be a label at all. There also shouldn't be any interactive elements inside a label. That ought to mean the image select element to which the inner label applies also shouldn't be inside the outer label.

@@ -18,8 +18,8 @@ <h3>{{ profile.display_name }}</h3>
<div>
Copy link
Member

Choose a reason for hiding this comment

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

I think moving the </label> on L34 to before opening this <div> for the properties should be the right fix. Then the label for the profile radio button ends before the labels and controls for the profile options.

@yuvipanda
Copy link
Collaborator

/cc @batpad as well

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.

We have nested label tags in form.html and it works, but its undefined behavior
3 participants