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

remove superflous and wrong selectors #36744

Merged
merged 2 commits into from
Dec 23, 2024
Merged

Conversation

DavidStaus
Copy link
Contributor

Description

  • textarea selector was used multiple times, even though there is no <textarea> in the HTML.

div + div selector was changed to p + p, as all input and label elements are nested inside p elements, and not divs in the example.

Motivation

The extra selectors confused me as I was trying to copy the styling, and I thought there was something I had missed.

Additional details

Related issues and pull requests

@DavidStaus DavidStaus requested a review from a team as a code owner November 11, 2024 17:44
@DavidStaus DavidStaus requested review from hamishwillee and removed request for a team November 11, 2024 17:44
@github-actions github-actions bot added Content:Learn:Forms Learning area Forms docs size/s [PR only] 6-50 LoC changed labels Nov 11, 2024
Copy link
Contributor

github-actions bot commented Nov 11, 2024

Preview URLs

Flaws (251)

URL: /en-US/docs/Learn_web_development/Extensions/Forms/How_to_structure_a_web_form/Example
Title: Example
Flaw count: 251

  • macros:
    • Wrong xref macro used (consider changing which macro you use). Error processing path /en-US/docs/Learn/Getting_started_with_the_web
    • Wrong xref macro used (consider changing which macro you use). Error processing path /en-US/docs/Learn/Getting_started_with_the_web/Installing_basic_software
    • Wrong xref macro used (consider changing which macro you use). Error processing path /en-US/docs/Learn/Getting_started_with_the_web/What_will_your_website_look_like
    • Wrong xref macro used (consider changing which macro you use). Error processing path /en-US/docs/Learn/Getting_started_with_the_web/Dealing_with_files
    • Wrong xref macro used (consider changing which macro you use). Error processing path /en-US/docs/Learn/Getting_started_with_the_web/HTML_basics
    • and 246 more flaws omitted

(comment last updated: 2024-12-23 00:12:34)

@@ -125,7 +125,7 @@ form {
border-radius: 1em;
}

div + div {
p + p {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering if we should delete this selector altogether. There reference in https://developer.mozilla.org/en-US/docs/Learn/Forms/How_to_structure_a_web_form is that this is "extra styles you can try", but this makes very little difference visually unless you make it much better.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Done.

Copy link
Collaborator

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

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

Fixes are good, Just a few comments/questions

@github-actions github-actions bot added the merge conflicts 🚧 [PR only] label Dec 19, 2024
Copy link
Contributor

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

@github-actions github-actions bot removed merge conflicts 🚧 [PR only] Content:Learn:Forms Learning area Forms docs labels Dec 20, 2024
* textarea selector was used multiple times, even though there is no <textarea> in the HTML.

div + div selector was changed to p + p, as all input and label elements are nested inside p elements, and not divs in the example.
Copy link
Collaborator

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

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

Thanks for this @DavidStaus .

Note, you didn't respond to my review comments, so I have made those changes as I thought best.

@hamishwillee hamishwillee merged commit fc2dda9 into mdn:main Dec 23, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/s [PR only] 6-50 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants