Dialog Element Implementation #3403
Conversation
lib/jsdom/living/helpers/focusing.js
Outdated
| return exports.outsideUnopenedDialog(child.parentNode); | ||
| }; | ||
|
|
||
| exports.runAutoFocus = target => { |
There was a problem hiding this comment.
Hmm. The way this is done is very unlike the spec at https://html.spec.whatwg.org/#flush-autofocus-candidates , including when you call it. Do you think it'd be feasible to implement the spec here? I'm worried that this kind of implementation will cause people to raise more bugs about autofocus not working as it does in browsers, instead of our current status of it just not being implemented...
| return; | ||
| } | ||
|
|
||
| if (runAutoFocus(this)) { |
There was a problem hiding this comment.
runAutoFocus doesn't look like a good implementation of https://html.spec.whatwg.org/#autofocus-delegate
|
Appreciate all the feedback on this! I've been looking at the focus algorithms you suggested implementing correctly (thank you for finding them, digging through the spec is a skill on its own apparently) and I think its doable, there's just a decent amount of requirements that get pulled in and life has gotten in the way of me working on it. I'm hoping to get some time to focus on it this week or at least find someone else who has some time to invest and help out. |
|
I think I've combed through all the autofocus tests and found which ones were passable and which ones were not, but probably best to double check that...I cannot get the Script blocking style sheet implementation to work correctly and I'm not sure it can due to how some of the timing works right now, so it's been left to fail for now but is partially implemented. |
|
Thank you so much for all the work done in this so far! |
lib/jsdom/living/helpers/colors.js
Outdated
| }; | ||
|
|
||
| function colorKeywordToRGB(name) { | ||
| if (name.toLowerCase() in RGB_LOOKUP) { | ||
| if (Object.prototype.hasOwnProperty.call(RGB_LOOKUP, name.toLowerCase())) { |
There was a problem hiding this comment.
Just a suggestion from a newbie here, but instead of a Object.prototype.hasOwnProperty.call, would a simple object access that gives undefined be less scarry in this case?
*I put a ternary here, but keeping the if can be more readable too.
function colorKeywordToRGB(name) {
const rgbKey = name.toLowerCase();
return RGB_LOOKUP[rgbKey] ? `rgb(${RGB_LOOKUP[rgbKey].join(", ")})` : "";
}
domenic
force-pushed
the
master
branch
2 times, most recently
from
2f07543
to
315b0ed
Compare
|
Unfortunately, I was unable to review this PR in a timely manner, since it's fairly large and I have less time for jsdom than I'd like. And now, a bunch of new web platform tests have been added, many of which fail. (Possibly due to changes in the dialog spec itself.) So we need someone to go through and figure out which ones we can pass, and update the implementation as appropriate. I'm very sorry about this. I know it's a subpar contributor experience. I'll try to help in a small way by pulling out the color-related changes into a separate PR, which I should be able to merge quickly on my own. |
Also attempts to implement more complex things like currentColor and inheritance and so on, but the test coverage for that is spotty... Extracted from #3403 and updated to more closely follow the spec. Co-authored-by: James Hoekzema <james.hoekzema@wwt.com>
Also attempts to implement more complex things like currentColor and inheritance and so on, but the test coverage for that is spotty... Extracted from #3403 and updated to more closely follow the spec. Co-authored-by: James Hoekzema <james.hoekzema@wwt.com>
Also attempts to implement more complex things like currentColor and inheritance and so on, but the test coverage for that is spotty... Extracted from #3403 and updated to more closely follow the spec. Somewhat overlaps with #3060, but that is more complete. Co-authored-by: James Hoekzema <james.hoekzema@wwt.com>
This comment was marked as spam.
This comment was marked as spam.
|
Hey, |
This implements the Dialog Element according to the spec: https://html.spec.whatwg.org/multipage/interactive-elements.html#the-dialog-element
There's a couple side effects of properly getting the web platform tests to pass:
autofocusattribute functionality.inert-does-not-match-disabled-selector.htmltest to pass.pseudo-classestests to pass.