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

Add an example for for #2376

Merged
merged 21 commits into from
Aug 2, 2023
Merged

Add an example for for #2376

merged 21 commits into from
Aug 2, 2023

Conversation

MrBrain295
Copy link
Contributor

@NiedziolkaMichal
Copy link
Member

Currently, this example doesn't show how attribute for is useful. You have also set its content to the value of attribute id, while name should be used instead. I think that the value of property height is currently invalid for this amount of content.

@NiedziolkaMichal NiedziolkaMichal mentioned this pull request Jan 6, 2023
94 tasks
@MrBrain295
Copy link
Contributor Author

@NiedziolkaMichal I have made the requested changes.

@NiedziolkaMichal
Copy link
Member

@MrBrain295 Sorry but I don't think that you have. The value of attribute for is still pointing to name instead of id and the example doesn't show how this attribute is useful. You should also get rid of form element since we don't use it for similar examples.

@MrBrain295
Copy link
Contributor Author

I’ve applied the feedback except for demonstrating it’s usefulness which I don’t know how I should do.

@NiedziolkaMichal
Copy link
Member

It would be great if your CSS would have an immediate visual impact. Additionally, the attribute selector might be confusing to the user when the name of the attribute is for.
How about something like that?
image
When the reader will click on the labels, he/she might notice that label without the attribute for is not setting the focus state to the control field. Additionally, usage its usage in the output element is shown. This example should also have JS code though because output is supposed to be an interactive element.

@github-actions github-actions bot added the idle Issues and pull requests with no activity for three months. label Feb 18, 2023
@estelle
Copy link
Member

estelle commented Jul 26, 2023

i added your suggestions. Now we just need to add the JS file.

@github-actions github-actions bot removed the idle Issues and pull requests with no activity for three months. label Jul 27, 2023
@Josh-Cena Josh-Cena added the Content:HTML issues related to HTML examples. label Jul 31, 2023
@github-actions
Copy link

github-actions bot commented Aug 2, 2023

This pull request has merge conflicts that must be resolved before it can be merged.

@queengooborg queengooborg requested a review from a team as a code owner August 2, 2023 15:02
@queengooborg queengooborg requested review from estelle and removed request for a team August 2, 2023 15:02
@queengooborg
Copy link
Collaborator

Thanks everyone -- because this has been sitting around for a while, I've decided to go ahead and make the requested changes. This is now ready to go!

@queengooborg queengooborg merged commit 755b4ba into mdn:main Aug 2, 2023
5 checks passed
@MrBrain295 MrBrain295 deleted the patch-3 branch August 2, 2023 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:HTML issues related to HTML examples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants