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

feat: add JavaScript to code snippet #2536

Merged
merged 9 commits into from
Aug 2, 2023
Merged

feat: add JavaScript to code snippet #2536

merged 9 commits into from
Aug 2, 2023

Conversation

CBID2
Copy link
Contributor

@CBID2 CBID2 commented Jun 19, 2023

Description

This PR adds JavaScript to the code snippet in the HTML input example.

Motivation

David Cook mentioned that the demo doesn't demonstrate the browser validation for required or pattern, because it is not inside a <form>. By adding JavaScript to it, the example will work.

Additional details

Example where the issue takes place

Related issues and pull requests

Fixes #2535

@github-actions
Copy link

It looks like this is your first pull request. 🎉 Thank you for your contribution! One of the project maintainers will triage and assign the pull request for review. We appreciate your patience. To safeguard the health of the project, please take a moment to read our code of conduct.

Copy link
Member

@estelle estelle left a comment

Choose a reason for hiding this comment

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

The EmbedInteractiveExample macro works in the /mdn/content/ directory.
This directory contains the examples that are embedded using the EmbedInteractiveExample within MDN. In other words, this repo contains HTML, CSS, and JS, which is can then be pulled into MDN pages using the macro.

To solve the issue raised, you would want to update the code to include a <form>. The form with no action will submit to itself, reloading the example in place.

@CBID2
Copy link
Contributor Author

CBID2 commented Jul 26, 2023

The EmbedInteractiveExample macro works in the /mdn/content/ directory. This directory contains the examples that are embedded using the EmbedInteractiveExample within MDN. In other words, this repo contains HTML, CSS, and JS, which is can then be pulled into MDN pages using the macro.

To solve the issue raised, you would want to update the code to include a <form>. The form with no action will submit to itself, reloading the example in place.

Ok @estelle, I added the form element.

@CBID2 CBID2 requested a review from estelle July 26, 2023 17:55
@estelle
Copy link
Member

estelle commented Jul 27, 2023

The EmbedInteractiveExample macro works in the /mdn/content/ directory. This directory contains the examples that are embedded using the EmbedInteractiveExample within MDN. In other words, this repo contains HTML, CSS, and JS, which is can then be pulled into MDN pages using the macro.

This part needs to be addressed.

To solve the issue raised, you would want to update the code to include a <form>. The form with no action will submit to itself, reloading the example in place.

Ok @estelle, I added the form element.

while a label works anywhere in the page, visually the label with the form control it's labelling is vital. Best practices is to put the label next to the input, before the input in most cases, but after a checkbox or radio button. In other words, the label should be in the form.

so, the EmbedInteractiveExample needs to be removed, the label needs to be moved to be in the form, and no js was added, but not sure if it needs any

@estelle estelle self-assigned this Jul 27, 2023
@CBID2
Copy link
Contributor Author

CBID2 commented Jul 30, 2023

The EmbedInteractiveExample macro works in the /mdn/content/ directory. This directory contains the examples that are embedded using the EmbedInteractiveExample within MDN. In other words, this repo contains HTML, CSS, and JS, which is can then be pulled into MDN pages using the macro.

This part needs to be addressed.

To solve the issue raised, you would want to update the code to include a <form>. The form with no action will submit to itself, reloading the example in place.

Ok @estelle, I added the form element.

while a label works anywhere in the page, visually the label with the form control it's labelling is vital. Best practices is to put the label next to the input, before the input in most cases, but after a checkbox or radio button. In other words, the label should be in the form.

so, the EmbedInteractiveExample needs to be removed, the label needs to be moved to be in the form, and no js was added, but not sure if it needs any

Hi @estelle! :) I made the changes.

@Josh-Cena Josh-Cena added the Content:HTML issues related to HTML examples. label Jul 31, 2023
Copy link
Member

@estelle estelle left a comment

Choose a reason for hiding this comment

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

Thanks! 🎉

@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.

1 similar comment
@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.

@bsmth bsmth merged commit cf8314f into mdn:main Aug 2, 2023
5 checks passed
@github-actions
Copy link

github-actions bot commented Aug 2, 2023

Congratulations on your first merged pull request. 🎉 Thank you for your contribution! Did you know we have a project board with high-impact contribution opportunities? We look forward to your next contribution.

@dacook
Copy link

dacook commented Aug 3, 2023

Thanks @CBID2 and @estelle for working through this one, it works perfectly now!

Sorry for causing confusion with the mention of the input-url.html file; it was all I could figure out at the time.

@CBID2 CBID2 deleted the CBID2-patch-1 branch August 3, 2023 23:30
@CBID2
Copy link
Contributor Author

CBID2 commented Aug 3, 2023

Thanks @CBID2 and @estelle for working through this one, it works perfectly now!

Sorry for causing confusion with the mention of the input-url.html file; it was all I could figure out at the time.

No problem @dacook! :) We're glad that this makes the documentation clearer now! :)

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.

Demo for <input type=url> does not demonstrate validation
5 participants