Shadow DOM docs #471

marcysutton · 2017-08-03T20:10:30Z

Should close #401. Feedback welcome!

WilcoFiers

There are a few more methods I think we should add. The test utilities are important, as well as dom.getComposedParent, dom.getRootNode and maybe also dom.findUp.

WilcoFiers · 2017-08-04T09:48:25Z

doc/API.md

+
+##### Description
+
+Recursvely return an array containing the virtual DOM tree for the node specified, excluding comment nodes and shadow DOM nodes `&lt;content> and `&lt;slot>`. This method will return a composed tree containing both light and shadow DOM, if applicable.


You shouldn't use HTML entities here. Anything in backticks is escaped automatically (have a look at how Github rendered this line). Also, you're missing a backtick char.

WilcoFiers · 2017-08-04T09:52:07Z

doc/API.md

@@ -608,6 +613,120 @@ In axe-core v1 the main method for axe was `axe.a11yCheck()`. This method was re
 - .a11yCheck requires a context object, and so will not fall back to the document root.
 - .a11yCheck does not return a Promise.

+### Virtual DOM Utilities
+
+#### API Name: axe.utils.getFlattenedTree


Do we need to document this method? People shouldn't use this in their checks. The only place this is relevant is in testing, and for those scenarios we have test utilities that do this stuff for them instead. Should we document testUtils instead?

If we add a method to set and clear the axe._tree cache then we would not need to document this.

However, if we do, then we should point ot the algorithm definition in the HTML spec (see the source code comments for the link)

We need something to pass to axe.utils.getNodeFromTree, so I'd vote for documenting it and dropping axe._tree. But we could also do as Dylan suggests and create a method to set and clear the cache. I had some questions about that in a separate comment.

WilcoFiers · 2017-08-04T09:56:13Z

doc/API.md

+
+##### Returns
+
+An object containing the virtualNode:


Should this be "A virtualNode object"?

WilcoFiers · 2017-08-04T10:02:29Z

doc/developer-guide.md

+```javascript
+axe.commons.text.accessibleText = function (element, inLabelledbyContext) {
+  let virtualNode = axe.utils.getNodeFromTree(axe._tree[0]); // throws an exception on purpose if axe._tree not correct
+  return axe.commons.text.virtualAccessibleText(virtualNode, inLabelledbyContext);


This would be text.accessibleTextVirtual, although I just realized I messed up and didn't rename that one. I made a ticket for it.

WilcoFiers · 2017-08-04T10:05:36Z

doc/developer-guide.md

+}
+```
+
+`virtualAccessibleText` would never be called directly; rather, it would be called


That's not true. We always call the virtual functions if we've got a virtual node we can use. Only if we don't have one do we use the lookup function.

Wouldn't that make our rules/checks less backwards-compatible?

WilcoFiers · 2017-08-04T16:57:23Z

Nah, new rules should use them when they can, as that's more performant. We have the old ones mostly to stay backward competable.

…

On 4 Aug 2017 18:13, "Marcy Sutton" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In doc/developer-guide.md <#471 (comment)>: > +it expects to operate on a virtual DOM tree. + +Let’s look at an example: + +```javascript +axe.commons.text.accessibleText = function (element, inLabelledbyContext) { + let virtualNode = axe.utils.getNodeFromTree(axe._tree[0]); // throws an exception on purpose if axe._tree not correct + return axe.commons.text.virtualAccessibleText(virtualNode, inLabelledbyContext); +} + +axe.commons.text.virtualAccessibleText = function (element, inLabelledbyContext) { + // rest of the commons code minus the virtual tree lookup, since it’s passed in +} +``` + +`virtualAccessibleText` would never be called directly; rather, it would be called Wouldn't that make our rules/checks less backwards-compatible? — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#471 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAgY_yjlhcJaTrpUyMCDyn8Vg9H6CAWQks5sU0NBgaJpZM4Os7jw> .

dylanb · 2017-08-04T14:03:28Z

doc/API.md

+		1. [API Name: axe.utils.getFlattenedTree](#api-name-axeutilsgetflattenedtree)
+		2. [API Name: axe.utils.getNodeFromTree](#api-name-axeutilsgetnodefromtree)
+		3. [API Name: axe.utils.querySelectorAll](#api-name-axeutilsqueryselectorall)
+		4. [API Name: axe._tree](#api-name-axe_tree)


Should we add a method to set and clear this from the outside rather than documenting the internal object?

I might just remove it for now so we don't hold up the release. Because I have questions about how that would work, since it gets set internally during the audit: 1. when would you set or clear it? 2. what should happen if you try to manipulate it at the wrong time?

dylanb · 2017-08-04T14:04:27Z

doc/API.md

@@ -608,6 +613,120 @@ In axe-core v1 the main method for axe was `axe.a11yCheck()`. This method was re
 - .a11yCheck requires a context object, and so will not fall back to the document root.
 - .a11yCheck does not return a Promise.

+### Virtual DOM Utilities
+
+#### API Name: axe.utils.getFlattenedTree


If we add a method to set and clear the axe._tree cache then we would not need to document this.

dylanb · 2017-08-04T14:05:53Z

doc/API.md

+
+##### Parameters
+ - `node` – node. The current HTML node for which you want a flattened DOM tree.
+ - `shadowId` – string(optional). ID of the shadow DOM that is the closest shadow ancestor of the node


A caller should always call this with the shadowID undefined as this parameter is intended only to be used by the recursive calls

I'm not quite following. Can't we set it as undefined if it doesn't get passed in? This may also be moot if we drop documentation of this function.

nbd - I was just suggesting adding a note to say that it is not intended to be directly used by an external caller

dylanb · 2017-08-04T14:06:40Z

doc/API.md

+
+##### Description
+
+A querySelectorAll implementation that works on the virtual DOM and Shadow DOM by manually walking the tree instead of relying on DOM API methods which don't step into Shadow DOM.


It walks the flattened tree

It walks the flattened tree

Do you mean your implementation? Or document.querySelectorAll? I assume you meant the former, but I wanted to make sure.

I mean this implementation walks the flattened tree. It is like document|Element.querySelectorAll except for the flattened tree rather than the DOM (light DOM).

dylanb · 2017-08-04T15:25:59Z

doc/developer-guide.md

@@ -41,7 +41,7 @@ After execution, a Check will return `true` or `false` depending on whether or n
 Rules are defined by JSON files in the [lib/rules directory](../lib/rules).  The JSON object is used to seed the [Rule object](../lib/core/base/rule.js#L30).  A valid Rule JSON consists of the following:

 * `id` - `String` A unique name of the Rule.
-* `selector` - **optional** `String` which is a CSS selector that specifies the elements of the page on which the Rule runs.  If omitted, the rule will run against every node.
+* `selector` - **optional** `String` which is a CSS selector that specifies the elements of the page on which the Rule runs. aXe-core will look inside of *open* [Shadow DOM](https://developer.mozilla.org/en-US/docs/Web/Web_Components/Shadow_DOM) subtrees for elements matching the provided selector. If omitted, the rule will run against every node.


perhaps we should say the light DOM and and open shadow DOM trees

dylanb · 2017-08-04T15:31:32Z

doc/developer-guide.md


-Occasionally, you may want to add additional information about why a Check passed or failed into its message.  For example, the [aria-valid-attr](../lib/checks/aria/valid-attr.json) will add information about any invalid ARIA attributes to its fail message.  The message uses the [doT.js](http://olado.github.io/doT/) and is compiled to a JavaScript function at build-time.  In the Check message, you have access to the `CheckResult` as `it`.
+Occasionally, you may want to add additional information about why a Check passed, failed or returned undefined into its message.  For example, the [aria-valid-attr](../lib/checks/aria/valid-attr.json) will add information about any invalid ARIA attributes to its fail message.  The message uses the [doT.js](http://olado.github.io/doT/) and is compiled to a JavaScript function at build-time.  In the Check message, you have access to the `CheckResult` as `it`.


This seems like it could benefit from an example?

There is one, at least for incomplete messages, in my other PR. I could always expand on that later. #469

I added an example in #469.

dylanb · 2017-08-04T17:42:10Z

doc/developer-guide.md

+```
+
+A virtualNode is an object containing an HTML DOM element (`actualNode`), an
+array of child virtualNodes, and, if provided when querying the virtual tree, a


The shadowID indicates whether the node is in a shadow root and if it is, which one it is inside.

dylanb · 2017-08-04T17:43:56Z

doc/API.md

@@ -608,6 +613,120 @@ In axe-core v1 the main method for axe was `axe.a11yCheck()`. This method was re
 - .a11yCheck requires a context object, and so will not fall back to the document root.
 - .a11yCheck does not return a Promise.

+### Virtual DOM Utilities
+
+#### API Name: axe.utils.getFlattenedTree


However, if we do, then we should point ot the algorithm definition in the HTML spec (see the source code comments for the link)

marcysutton · 2017-08-04T19:54:01Z

There are a few more methods I think we should add. The test utilities are important, as well as dom.getComposedParent, dom.getRootNode and maybe also dom.findUp.

I didn't add those because they are commons functions and not utils, but it sounds like we had a request for documenting the rest of the commons. I won't go through all of those right now, but I can start with these few and the testUtils.

Closes #401

Closes #472

Also a few copy tweaks from feedback

dylanb · 2017-08-07T14:01:53Z

doc/API.md

+
+##### Description
+
+Recursvely return an array containing the virtual DOM tree for the node specified, excluding comment nodes and shadow DOM nodes `<content>` and `<slot>`. This method will return a composed tree containing both light and shadow DOM, if applicable.


composed tree is an outdated term. The new term is flattened tree.

What about axe.commons.dom.getComposedParent? Is that still good?

WilcoFiers

This is looking really excellent. Thank you Marcy! The one thing I still think needs to change is that the Virtual DOM Utilities in my opinion should be in developer-guide.md instead of in API.md. I think API.md should only contain code you need to run axe-core. The develop guide is more suited for stuff you only need to know if you start writing code for axe-core. WDYT?

marcysutton · 2017-08-08T15:45:35Z

That's fine by me, I'll move the test utilities over there too. Thanks!

marcysutton · 2017-08-08T17:37:04Z

Ok, I moved them to the developer guide and added a table of contents to it.

marcysutton requested review from dylanb and WilcoFiers August 3, 2017 20:10

marcysutton changed the title ~~Shadow docs~~ Shadow DOM docs Aug 3, 2017

WilcoFiers reviewed Aug 4, 2017

View reviewed changes

dylanb requested changes Aug 4, 2017

View reviewed changes

Marcy Sutton added 8 commits August 4, 2017 13:22

docs(API): change mentions of a11yCheck to axe.run

36a0551

docs: add Shadow DOM documentation

7331bb0

Closes #401

docs: add Shadow DOM documentation

7d859dc

Closes #401

docs: try to fix virtual dom table of contents

9f8ab45

docs(API.md): fix tabs in table of contents

38c0c50

chore: rename file as accessible-text-virtual

2014d57

feat: add missing accessibleTextVirtual method

dd12381

Closes #472

docs: remove axe._tree, make copy adjustments

278a6c6

marcysutton force-pushed the shadow-docs branch from 9e17f8e to 278a6c6 Compare August 4, 2017 21:17

Marcy Sutton added 4 commits August 4, 2017 14:18

docs: remove axe._tree from TOC

e05d0a7

docs: add shadowID info to developer guide

f1c7eb2

docs: document testUtils and a few commons

fca4cc3

docs: add line about ShadowDOM to contributing

5a4fec4

Also a few copy tweaks from feedback

dylanb reviewed Aug 7, 2017

View reviewed changes

Marcy Sutton added 2 commits August 7, 2017 16:18

docs: move Shadow APIs to "core API" section

d44ee3b

docs: fix API TOC

324ecdd

WilcoFiers reviewed Aug 8, 2017

View reviewed changes

docs: move core/test APIs to developer guide

adcd227

dylanb approved these changes Aug 8, 2017

View reviewed changes

marcysutton merged commit 57ccbbd into shadowDOM Aug 8, 2017

marcysutton deleted the shadow-docs branch August 8, 2017 21:34

marcysutton mentioned this pull request Aug 8, 2017

Consistent treatment of virtualNode #435

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Shadow DOM docs #471

Shadow DOM docs #471

marcysutton commented Aug 3, 2017

WilcoFiers left a comment

WilcoFiers Aug 4, 2017

WilcoFiers Aug 4, 2017

dylanb Aug 4, 2017

dylanb Aug 4, 2017

marcysutton Aug 4, 2017

WilcoFiers Aug 4, 2017

WilcoFiers Aug 4, 2017

WilcoFiers Aug 4, 2017

marcysutton Aug 4, 2017

WilcoFiers commented Aug 4, 2017 via email

dylanb Aug 4, 2017

marcysutton Aug 4, 2017

dylanb Aug 4, 2017

dylanb Aug 4, 2017

marcysutton Aug 4, 2017

dylanb Aug 7, 2017

dylanb Aug 4, 2017

marcysutton Aug 4, 2017

dylanb Aug 7, 2017

dylanb Aug 4, 2017

dylanb Aug 4, 2017

marcysutton Aug 4, 2017 •

edited

Loading

marcysutton Aug 7, 2017

dylanb Aug 4, 2017

dylanb Aug 4, 2017

marcysutton commented Aug 4, 2017 •

edited

Loading

dylanb Aug 7, 2017

marcysutton Aug 7, 2017

WilcoFiers left a comment

marcysutton commented Aug 8, 2017

marcysutton commented Aug 8, 2017


		##### Description

		Recursvely return an array containing the virtual DOM tree for the node specified, excluding comment nodes and shadow DOM nodes `<content> and `<slot>`. This method will return a composed tree containing both light and shadow DOM, if applicable.


		##### Description

		A querySelectorAll implementation that works on the virtual DOM and Shadow DOM by manually walking the tree instead of relying on DOM API methods which don't step into Shadow DOM.


		Occasionally, you may want to add additional information about why a Check passed or failed into its message. For example, the [aria-valid-attr](../lib/checks/aria/valid-attr.json) will add information about any invalid ARIA attributes to its fail message. The message uses the [doT.js](http://olado.github.io/doT/) and is compiled to a JavaScript function at build-time. In the Check message, you have access to the `CheckResult` as `it`.
		Occasionally, you may want to add additional information about why a Check passed, failed or returned undefined into its message. For example, the [aria-valid-attr](../lib/checks/aria/valid-attr.json) will add information about any invalid ARIA attributes to its fail message. The message uses the [doT.js](http://olado.github.io/doT/) and is compiled to a JavaScript function at build-time. In the Check message, you have access to the `CheckResult` as `it`.

Shadow DOM docs #471

Shadow DOM docs #471

Conversation

marcysutton commented Aug 3, 2017

WilcoFiers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WilcoFiers commented Aug 4, 2017 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcysutton Aug 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcysutton commented Aug 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WilcoFiers left a comment

Choose a reason for hiding this comment

marcysutton commented Aug 8, 2017

marcysutton commented Aug 8, 2017

marcysutton Aug 4, 2017 •

edited

Loading

marcysutton commented Aug 4, 2017 •

edited

Loading