Rename findSpatNavCandidates() to findFocusableAreas() about spatial-navigation HOT 4 CLOSED

I think there are two parts in your proposal:

1. Omitting the direction means the same thing as specifying direction inside: look for all visible focusable areas in the container, regardless of direction.

   I'm OK with that. It is less verbose, and the semantics are intuitive. It works regardless of whether we take suggestion 2 as well or not (although I guess it is most compelling if we do take suggestion 2 as well). @jihyerish, what do you think?

2. Between the container and the start element, swap with is the "this" parameter and which is the optional method argument.

   I'm not necessarily opposed to this, but I am a bit less sure: one the one hand, used out of context, maybe this is indeed a more intuitive way of phrasing it. On the other hand, if you're calling it from an "navbeforefocus" navigation event (regardless of whether it is merged and/or renamed as discussed in #32) — for example because you're trying to polyfill nav-rule — you have a handle on the start element, but you don't have one on the container. In that context, making the container mandatory and the start element optional is not super nice. Finding the container isn't super hard (it's just a walk up the tree), but if the browser could do it for you, that's even nicer.

   @jihyerish has suggested we may add a method for finding the container starting from an arbitrary element. I agree we could do that, and that would make using your suggested "flipped perspective" syntax easier to use

   something.addEventListener("navbeforefocus",funciton(e) {
     var container = e.target.getSpatnavContainer();
     var candidates = container.findFocusableAreas(e.dir, e.target);
     /* ... */
   });

   The reason I am not a little skeptical is that if the vast majority of invocations of this function end up looking like that, we might as well leave the container be an optional argument as currently designed. If on the other hand people frequently start from the container, then your syntax if more friendly.

   All in all, I'm not opposed, but I think "override some part of the spatnav logic from event handlers to write a polyfilll or extend the logic" is a realistic use case, which supports the current syntax. Do we have use cases that support the idea that starting with the container rather than the element may be a reasonably common thing to want?

from spatial-navigation.

for example because you're trying to polyfill nav-rule — you have a handle on the start element, but you don't have one on the container.

We could include the used spatnav-container in an container-attribute of NavigationEvent. That might be something we wanna do regardless? See #33.

I think we better not let the container-argument be optional. That helps authors think about containers before using the API. Still, lazy developers can just do body.findFocusableAreas().

When a second, not so lazy, developer comes along, he/she could otherwise wonder "Was this really the intended search container?" "Did we forgot to specify a container here"?.

from spatial-navigation.

@hugoholgersson

div.findFocusableAreas() // Returns all visible focusables within the div.
div.findFocusableAreas("left") // Visible focusables within the div that are left of document.activeElement().
div.findFocusableAreas("left", startelement) // Visible focusables in the div that are left of startelement.
Question: What if startelement isn't visible?

Though startelement is invisible, findFocusableAreas() returns visible focusables (candidates).
I think this API would work in the same way as pressing the arrow keys to move focus.
For example, if the startelement in the scrollable container becomes invisible after scrolling and

1. there is any visible focusable within the container, navbeforefocus event occurs.
2. there isn't any visible focusable within the container, but the container can be scrolled, navbeforescroll event can occur.
3. there isn't any visible focusable within the container and the container cannot be scrolled, navnotarget event can occur.

At this point, I have another question. What if the container isn't visible?
I think in this case the API would return null or all focusables which are not only visible but also invisible.

We could include the used spatnav-container in an container-attribute of NavigationEvent. That might be something we wanna do regardless? See #33.

This is a good idea!
But I assume this suggestion can't cover all the cases in this issue because findSpatNavCandidates() (or findFocusableAreas()) doesn't always trigger NavigationEvent.
If there is any visible focusable within the container, the API doesn't trigger navbeforefocus, but selectSpatNavBestCandidate() does.

@florian

I'm OK with that. It is less verbose, and the semantics are intuitive. It works regardless of whether we take suggestion 2 as well or not (although I guess it is most compelling if we do take suggestion 2 as well). @jihyerish, what do you think?

I'm inclined to let the startelement mandatory and container optional.

Unless there is an API for finding the container, making the container mandatory seems less intuitive.

for example because you're trying to polyfill nav-rule

To polyfill nav-rule means to replace selectSpatNavBestCandidate() . Do I understand right?

from spatial-navigation.

findFocusableAreas() doesn't always trigger NavigationEvent ... but selectSpatNavBestCandidate() does.

My understanding is that none of the JS methods triggers NavigationEvent. They are only used to query information (but not change state). Authors will still move focus with element.focus().

The JS API will typically be used in a navigation-handler to override the browser's built-in focus movements.

To polyfill nav-rule means to replace selectSpatNavBestCandidate() . Do I understand right?

Hmm... Maybe not. To polyfill nav-rule, I think we only really need to provide a cancelable navigate-event. findFocusableAreas() would help devs find the visible focusables to consider, but I think it could be done without findFocusableAreas() as well.

Unless there is an API for finding the container,

Finding the parent scroller could be done manually [1] [2]. One could do if (myscroller.contains(e.relatedTarget)) to hande certain scrollers specifically. I'd guess typical web apps have <= 3 scrollers to handle so I think the resulting JS won't be too messy.

... making the container mandatory seems less intuitive.

I like it because it resembles querySelectorAll's syntax.

from spatial-navigation.