-
Notifications
You must be signed in to change notification settings - Fork 22.4k
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
fix: flaws in web/html batch 3 #3510
Conversation
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.
Thanks again @nschonni ! There are a few references to HTMLElement
that I think should stay, as they will be made correct in openwebdocs/project#23.
@@ -14,7 +14,7 @@ | |||
|
|||
<div class="hidden">The source for this interactive example is stored in a GitHub repository. If you'd like to contribute to the interactive examples project, please clone <a href="https://github.com/mdn/interactive-examples">https://github.com/mdn/interactive-examples </a> and send us a pull request.</div> | |||
|
|||
<p>All such custom data are available via the {{domxref("HTMLElement")}} interface of the element the attribute is set on. The {{domxref("HTMLElement.dataset")}} property gives access to them.<br> | |||
<p>All such custom data are available via the {{domxref("HTMLElement")}} interface of the element the attribute is set on. The {{domxref("HTMLOrForeignElement.dataset")}} property gives access to them.<br> |
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 one and the other two in this file should stay HTMLElement
. HTMLOrForeignElement
is a mixin that will be demixed in openwebdocs/project#23.
@@ -20,7 +20,7 @@ | |||
<li>A <em>negative value</em> (usually <code>tabindex="-1"</code>) means that the element is not reachable via sequential keyboard navigation, but could be focused with JavaScript or visually by clicking with the mouse. It's mostly useful to create accessible widgets with JavaScript. | |||
|
|||
<div class="note"> | |||
<p>A negative value is useful when you have off-screen content that appears on a specific event. The user won't be able to focus any element with a negative <code>tabindex</code> using the keyboard, but a script can do so by calling the <code>focus()</code> <a href="/en-US/docs/Web/API/HTMLElement/focus">method</a>.</p> | |||
<p>A negative value is useful when you have off-screen content that appears on a specific event. The user won't be able to focus any element with a negative <code>tabindex</code> using the keyboard, but a script can do so by calling the <code>focus()</code> <a href="/en-US/docs/Web/API/HTMLOrForeignElement/focus">method</a>.</p> |
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 one should stay HTMLElement
. HTMLOrForeignElement
is a mixin that will be demixed in openwebdocs/project#23.
@@ -68,6 +68,6 @@ <h2 id="See_also">See also</h2> | |||
|
|||
<ul> | |||
<li>All <a href="/en-US/docs/Web/HTML/Global_attributes">global attributes</a></li> | |||
<li>{{domxref("HTMLElement.tabIndex")}} that reflects this attribute</li> | |||
<li>{{domxref("HTMLOrForeignElement.tabIndex")}} that reflects this attribute</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.
Again here: this should stay HTMLElement
. HTMLOrForeignElement
is a mixin that will be demixed in openwebdocs/project#23.
d87ed1d
to
c644a0e
Compare
@nschonni , are you expecting to address the review comments I had for this PR (and some of the other flaw-fixing PRs)? |
@wbamberg I'll be happy when the de-mixing happens, but these changes are what the flaw fixing does today, and if I undo it while we wait, they'll just continue to be flagged till movement is made on that external work |
The de-mixing work is ongoing at the moment, but even without that, in the cases I highlighted, the changes make the pages less good, not more. That's also the opinion of Florian in, for example, #3243 (comment). In a case like #3243 (comment), the new content doesn't really make sense any more:
Even without fixing the flaw it would be better to keep the same link text. But if comments like this are out of scope, so review isn't supposed to check that the changes improve the content, just that they fix the flaws, then it would be great to know that beforehand! |
Nope, you're fine to block the PR if you don't think the changes are an improvement. I'd suggest a Yari issue instead of the OpenWebDocs one, if you think the flaw fixing is wrong |
c644a0e
to
412acf6
Compare
412acf6
to
55c6ea0
Compare
No description provided.