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.

Sign up for GitHub

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

Jump to bottom

Fix #556, make focus navigation work when shadow host's tabindex is -1 #557

Open
wants to merge 1 commit into
base: gh-pages
Choose a base branch
from
Open

Fix #556, make focus navigation work when shadow host's tabindex is -1 #557

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 10 additions & 4 deletions spec/shadow/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1153,7 +1153,7 @@ <h3>Focus</h3>
<li>When <var>HOST</var> is focused by <code>focus()</code> method or <code>autofocus</code> attribute: The first <a>focusable area</a> in focus navigation order of <var>HOST</var>'s <a>shadow root</a>'s <a>focus navigation scope</a> gets focus. See the next section for the formal definition of the ordering.</li>
<li>When mouse is clicked on a node in <var>HOST</var>'s <a>shadow tree</a> and the node is not a <a>focusable area</a>: The first <a>focusable area</a> in focus navigation order of <var>HOST</var>'s <a>shadow root</a>'s <a>focus navigation scope</a> gets focus. See the next section for the formal definition of the ordering.</li>
<li>When any element in <var>HOST</var>'s <a>shadow tree</a> has focus, <code>:focus</code> pseudo-class applies to <var>HOST</var> in addition to the focused element itself.</li>
<li>If <code>:focus</code> pseudo-class applies to <var>HOST</var>, and <var>HOST</var> is in a <a>shadow root</a> of another <a>shadow host</a> <var>HOST2</var> which also <a>delegates focus</a>, <code>:focus</code> pseudo-class applies to <var>HOST2</var> as well.</li>
<li>If <code>:focus</code> pseudo-class applies to <var>HOST</var>, and <var>HOST</var> is in a <a>shadow root</a> of another <a>shadow host</a> <var>HOST2</var> which also <a>delegates focus</a>, <code>:focus</code> pseudo-class applies to <var>HOST2</var> as well.</li>
</ol>
</p>
</section>
Expand Down Expand Up @@ -1260,9 +1260,15 @@ <h3>Sequential Focus Navigation</h3>
<li>Let <var>NAVIGATION-ORDER</var> be an empty list.</li>
<li>For each element <var>ELEMENT</var> in a <a>focus navigation scope</a> <var>SCOPE</var>,
<ol>
<li>if <var>ELEMENT</var> is focusable, a <a>shadow host</a>, or a <a>slot element</a>, append <var>ELEMENT</var> to <var>NAVIGATION-ORDER</var>.</li>
<li>If <var>ELEMENT</var> is a <a>shadow host</a>:
<ol>
<li>If its <a>tabindex</a> value is not negative and its <a>shadow root</a>'s delegatesFocus flag is set, append <var>ELEMENT</var> to <var>NAVIGATION-ORDER</var>.</li>
<li>Otherwise, append <var>ELEMENT</var> to <var>NAVIGATION-ORDER</var>.</li>
Copy link
Contributor

@hayatoito hayatoito Aug 30, 2016 •
edited

Choose a reason for hiding this comment

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

It looks a shadow host is always appended to NAVIGATION-ORDER. :(

Copy link
Member Author

Choose a reason for hiding this comment

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

This is intentional. Unless delegatesFocus flag is set, put the shadow host in the NAVIGATION-ORDER regardless of the tabindex value, to mark the point into which its shadow scope will be merged at the next step.

Copy link
Contributor

@hayatoito hayatoito Sep 1, 2016 •
edited

Choose a reason for hiding this comment

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

No. My point is:

If (ELEMENT is a shadow host) {
  If (... some condition...) {
    Add appendELEMENT to NAVIGATION-ORDER.
  } else {
    Add appendELEMENT to NAVIGATION-ORDER.
  }
}

This is equivalent to:

If (ELEMENT is a shadow host) {
  Add appendELEMENT toNAVIGATION-ORDER.
}

Please re-check the condition and wording.

</ol>
</li>
<li>Otherwise if <var>ELEMENT</var> is focusable, or a <a>slot element</a>, and its <a>tabindex</a> value is not negative, append <var>ELEMENT</var> to <var>NAVIGATION-ORDER</var>.</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

The "and", "or" relationship are confusing. Could you make this explicit?

</ol>
<li>Reorder <var>NAVIGATION-ORDER</var> according to the <a>tabindex</a> value attached to each node. In this step, an element whose <a>tabindex</a> value is negative <strong>must</strong> not be appended to <var>NAVIGATION-ORDER</var>.</li>
<li>Reorder <var>NAVIGATION-ORDER</var> according to the <a>tabindex</a> value attached to each node. In this step, a shadow host with negative <a>tabindex</a> value must be treated as if its <a>tabindex</a> value were 0.</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you have a link which explains how "Reorder" sort elements?

In the current definition, it looks that elements would be sorted by only "tabindex". No tie-breaker.
We should have a link to explain how these elements are sorted by tabindex and tree-order.

</ol>
</li>
</ol>
Expand All @@ -1284,7 +1290,7 @@ <h3>Sequential Focus Navigation</h3>
<ol>
<li>If <var>ELEMENT</var> is a shadow host:
<ol>
<li>Unless its shadow root’s delegatesFocus flag is set, append<var> ELEMENT</var> to <var>MERGED-NAVIGATION-ORDER</var>.</li>
<li>If it is focusable, its <a>tabindex</a> value is not negative, and its <a>shadow root</a>’s delegatesFocus flag is not set, append <var>ELEMENT</var> to <var>MERGED-NAVIGATION-ORDER</var>.</li>
Copy link
Contributor

@hayatoito hayatoito Aug 30, 2016 •
edited

Choose a reason for hiding this comment

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

This condition in the sentence might be confusing. Could you say ELEMENT has to satisfy every conditions explicitly?

<li>Apply the <a>focus navigation order merging algorithm</a> with the focus navigation order owned by its <a>shadow root</a> as input, then append the result to <var>MERGED-NAVIGATION-ORDER</var>.</li>
</ol>
</li>
Expand Down