-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
docs: tweak spelling, make punctuation and wording consistent #7154
Conversation
* Fixed typo Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
* Adjusted case (“wiki”—should otherwise be qualified for more clarity) * Rephrased “microformats wiki existing-rel-values page” (super-long, reads awkwardly) * Added article to make `source` element conditions more readable * Limited repetition (double “and so forth”) * Made layout/transparency note slightly more clear Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
1. Changed spelling of “bread-crumb” to “breadcrumb” (seems [more common](https://www.google.com/search?q=%22breadcrumb%22)) 2. In [pseudo-classes list](https://html.spec.whatwg.org/multipage/semantics-other.html#pseudo-classes), ended all explanations with a period to handle section consistently. 3. Used the same wording “…has the `required` attribute” for those elements that have the `required` attribute (mentioning of the concept seems useful, but seems more effective at a spot where it doesn’t break consistency) Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
@@ -70335,13 +70335,14 @@ Demos: | |||
any element falling into one of the following categories:</p> | |||
|
|||
<ul> | |||
<li><code>input</code> elements that are <i data-x="concept-input-required">required</i></li> | |||
<li><code>input</code> elements that have a <code data-x="attr-select-required">required</code> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a normative change that does not match with the current implementations.
The mere presence of the required
attribute does not make the element's required. For instance
<input type="button" required>
will not match :required
.
Maybe https://html.spec.whatwg.org/multipage/input.html#concept-input-required should call out explicitly that when an element is barred from constraint validation it can not be required, but that would be for an other issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh and I missed it, but your link was wrong too, it should have been data-x="attr-input-required"
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe https://html.spec.whatwg.org/multipage/input.html#concept-input-required should call out explicitly that when an element is barred from constraint validation it can not be required, but that would be for an other issue.
I had reviewed that section and to me it suggests “being required” and “having the required
attribute [set]” is the same thing? What do I miss—or is there something that we could make more clear in the respective section.
Oh and I missed it, but your link was wrong too, it should have been
data-x="attr-input-required"
.
What is “my link”? Also, I copied from the two list items below, for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well that's the thing of normative changes, we can't really stop at what things "suggest". Changing when a rule applies is a normative change, it needs to be done thoroughly and checked with implementers.
As I said, I agree that https://html.spec.whatwg.org/multipage/input.html#concept-input-required should specifically tell that "barred from constraint validation" does prevent "is required" (or the other way around), if that's really what does prevent it, but that requires its own issue and investigation.
What is “my link”? Also, I copied from the two list items below, for consistency.
data-x
attributes do link to definitions, you were gonna link to https://html.spec.whatwg.org/#attr-select-required which is specifically for <select>
elements, the correct link would have been https://html.spec.whatwg.org/#attr-input-required that you set through data-x="attr-input-required"
. But anyway, this change should be reverted.
@@ -70203,14 +70203,14 @@ Demos: | |||
<ul> | |||
<li><code>input</code> elements whose <code data-x="attr-input-type">type</code> attribute is in | |||
the <span data-x="attr-input-type-checkbox">Checkbox</span> state and whose <span | |||
data-x="concept-fe-checked">checkedness</span> state is true</li> | |||
data-x="concept-fe-checked">checkedness</span> state is true.</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how authoritative it is, but according to https://www.imperial.ac.uk/brand-style-guide/writing/grammar/punctuation/bullet-points/ the more "correct" way in this case would be
The :checked pseudo-class must match any element falling into one of the following categories:
- input elements whose type attribute is in the Checkbox state and whose checkedness state is true;
- input elements whose type attribute is in the Radio Button state and whose checkedness state is true;
- option elements whose selectedness is true.
That is, a semi colon between each item and a full stop after the last one.
If going this route, :hover
section above would also need an update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a fair point. For context, and as I describe in the description of the PR, all other sentences seem to use periods, so I used those for consistency.
If you and the other editors now like to use different punctuation I suggest this is handled elsewhere, that is, feel free to dismiss this consistency update to accomplish the consistency you prefer.
|
||
<li><code>textarea</code> elements that do not have a <code | ||
data-x="attr-textarea-readonly">readonly</code> attribute, and that are not <span | ||
data-x="concept-fe-disabled">disabled</span></li> | ||
data-x="concept-fe-disabled">disabled</span>.</li> | ||
|
||
<li>elements that are <span data-x="editing host">editing hosts</span> or <span>editable</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we really choose to go with a full stop after each item (see previous comment), then at least "elements" should be capitalized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(See an earlier point on consistency. This is a good point, but the whole section may need reworking if both case and punctuation are changed.)
@Kaiido, this is a (simple) consistency update. You’re right that there are more problems in this section, but to handle that I suggest to file an issue to revisit it. If these updates are not wanted to improve things in the interim, I can submit another PR just for the spelling change. |
Not sure why you closed this. Only the normative change about "concept-input-required" needs to be reverted. |
required
attribute” for those elements that have therequired
attribute (mentioning of the concept seems useful, but seems more effective at a spot where it doesn’t break consistency)💥 Error: 503 Service Unavailable 💥
PR Preview failed to build. (Last tried on Oct 5, 2021, 6:43 PM UTC).
More
PR Preview relies on a number of web services to run. There seems to be an issue with the following one:
🚨 HTML Diff Service - The HTML Diff Service is used to create HTML diffs of the spec changes suggested in a pull request.
🔗 Related URL
If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.