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.

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

[css-nav-1] Clarify the condition of focusable element #3395

Closed
jihyerish opened this issue Dec 4, 2018 · 5 comments
Closed

[css-nav-1] Clarify the condition of focusable element #3395

jihyerish opened this issue Dec 4, 2018 · 5 comments

Comments

@jihyerish
Copy link
Contributor

jihyerish commented Dec 4, 2018

Currently, the spec defines the focusable elements depending on the HTML Spec.

But there are some missing parts for considering what is focusable for the spatial navigation.
For example, the element specified with "visibility: hidden", is it a focusable or not?
This matters when we get the result of focusableAreas({ mode: "all" }), the result will include
the elements with visibility: hidden or display: none.

There is more detailed description about the Focusable Element.
: https://allyjs.io/data-tables/focusable.html#css-property-visibility

We need to refer it and make it clear about the condition of focusable in the spatial navigation.

@jihyerish
Copy link
Contributor Author

The additional condition needs to be considered:

  • The element which uses keydown event handler

Also, we need to follow up the discussion whatwg/html#4464 in WHATWG

@hugoholgersson
Copy link

For example, the element specified with "visibility: hidden", is it a focusable or not?

IMO, things users can't see should not be navigable.

Nodes with a key- or click handler most likely needs to be navigable.

Some web apps use one "catch all"-click handler, often put on body or a container div, to react on clicks within certain sub areas of the page. These web apps often lack tab indices. Instead, clickable/focusable elements, for example divs that look like buttons, are styled with cursor: |pointer|. To enable SpatNav on these web apps, let's add cursor: pointer to the list too? (Chrome implementation).

@bokand
Copy link
Contributor

bokand commented May 30, 2019

I don't think we need to redefine what it means to be focusable. We should instead restrict spatial navigation such that "focusable" is required but not sufficient. i.e. The element must be focusable as well as visible. I believe Chrome's implementation already does this.

aarongable pushed a commit to chromium/chromium that referenced this issue Jun 14, 2019
Background:
Some web apps use one "catch all"-click handler, often put
on <body> or a container <div>, to react on clicks within
certain areas of the page. These web pages expect clicks
(or touches) and are not designed with key[board] users in
mind (they lack tabindex).

These pages' clickables, for example <div>s that look like
buttons, are often styled with {cursor: pointer}.

Problem:
These web pages are inaccessible for keyboard-only users.

Solution:
Enable SpatNav on these web apps by adding {cursor: pointer}
sub trees' tip to the list of navigation candidates.

Spec discussion:
w3c/csswg-drafts#3395

Note:
snav-imagemap-area-not-focusable.html found a bug in
Chrome's UA CSS. We used to style *all* <area>s as
clickables, with {cursor: pointer}, even those without a
href-attribute. I fixed this in crrev.com/c/1653009.

Bug: None
Change-Id: I7baa2ba6f5f36ebc23b072d677edc66acfb2a70b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1632231
Commit-Queue: Hugo Holgersson <hholgersson@fb.com>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#669193}
pdigennaro pushed a commit to washezium/washezium that referenced this issue Jul 7, 2019
Background:
Some web apps use one "catch all"-click handler, often put
on <body> or a container <div>, to react on clicks within
certain areas of the page. These web pages expect clicks
(or touches) and are not designed with key[board] users in
mind (they lack tabindex).

These pages' clickables, for example <div>s that look like
buttons, are often styled with {cursor: pointer}.

Problem:
These web pages are inaccessible for keyboard-only users.

Solution:
Enable SpatNav on these web apps by adding {cursor: pointer}
sub trees' tip to the list of navigation candidates.

Spec discussion:
w3c/csswg-drafts#3395

Note:
snav-imagemap-area-not-focusable.html found a bug in
Chrome's UA CSS. We used to style *all* <area>s as
clickables, with {cursor: pointer}, even those without a
href-attribute. I fixed this in crrev.com/c/1653009.

(cherry picked from commit 308a300)

Bug: 961927
Change-Id: I7baa2ba6f5f36ebc23b072d677edc66acfb2a70b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1632231
Commit-Queue: Hugo Holgersson <hholgersson@fb.com>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#669193}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1662868
Cr-Commit-Position: refs/branch-heads/3809@{#375}
Cr-Branched-From: d82dec1-refs/heads/master@{#665002}
@jihyerish
Copy link
Contributor Author

jihyerish commented Aug 16, 2019

I agree that in the spatial navigation spec, we don't need to redefine the "focusables".
But I'll consider putting a note that UA can make the element with

  • keydown event handler
  • {cursor: pointer}, click event handler (especially for the page in TV)

@hugoholgersson
Copy link

But I'll consider putting a note that UA can make the element [a "SpatNav candidate"] with

  • keydown event handler
  • {cursor: pointer}

Cool. Yeah, I think that's a good to mention in the spec.

In Chrome, this is not only on "keydown" but also on "keyup", "keypress" and on "click". See https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/element.cc?type=cs&q=function:SupportsSpatialNavigationFocus&sq=package:chromium. I think we should mention all of them, in that case.

I also think we should mention that tabindex=-1 can be used to override this heuristics. I encouraged one major web app to use this trick on their topmost container <div> that had a click-handler (to stop chromium's SpatNav from going to there). They now use it in production. I'd like to add a web test for that.

jiongle1 pushed a commit to jiongle1/chromium that referenced this issue Mar 27, 2024
Background:
Some web apps use one "catch all"-click handler, often put
on <body> or a container <div>, to react on clicks within
certain areas of the page. These web pages expect clicks
(or touches) and are not designed with key[board] users in
mind (they lack tabindex).

These pages' clickables, for example <div>s that look like
buttons, are often styled with {cursor: pointer}.

Problem:
These web pages are inaccessible for keyboard-only users.

Solution:
Enable SpatNav on these web apps by adding {cursor: pointer}
sub trees' tip to the list of navigation candidates.

Spec discussion:
w3c/csswg-drafts#3395

Note:
snav-imagemap-area-not-focusable.html found a bug in
Chrome's UA CSS. We used to style *all* <area>s as
clickables, with {cursor: pointer}, even those without a
href-attribute. I fixed this in crrev.com/c/1653009.

(cherry picked from commit 308a300)

(cherry picked from commit b2f29c7)

Bug: 961927
Change-Id: I7baa2ba6f5f36ebc23b072d677edc66acfb2a70b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1632231
Commit-Queue: Hugo Holgersson <hholgersson@fb.com>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#669193}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1662868
Cr-Original-Commit-Position: refs/branch-heads/3809@{#375}
Cr-Original-Branched-From: d82dec1-refs/heads/master@{#665002}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1662837
Cr-Commit-Position: refs/branch-heads/3770@{#1032}
Cr-Branched-From: a9eee1c-refs/heads/master@{#652427}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants