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 how sptialNavigationSearch() works #3743

Closed
jihyerish opened this issue Mar 19, 2019 · 4 comments · Fixed by #4377
Closed

[css-nav-1] Clarify how sptialNavigationSearch() works #3743

jihyerish opened this issue Mar 19, 2019 · 4 comments · Fixed by #4377

Comments

@jihyerish
Copy link
Contributor

jihyerish commented Mar 19, 2019

There is ambiguity in the behavior of spatialNavigationSearch(),
if the element which triggers the API is the spatial navigation container.

The reason for this ambiguity is the direction of spatial navigation means not only moving the focus in 4-ways but also moving the focus inside or outside of the container element.
It isn't clear what will be the scope of candidates in this case.

For example,
image
(B and C are focusable elements inside the container2)

What will be the result of
container2.spatialNavigationSearch({dir: 'right', container: document}); between B and D?
I guess both results would be expected depending on the use case.

[NOTE]
I assumed if the candidate option is omitted, the candidates are decided by the value of container option.

For example,
container2.spatialNavigationSearch({dir: 'right', container: document}); is equal to
container2.spatialNavigationSearch({dir: 'right', candidate: document.focusAreas(), container: document});

Therefore, I suggest the option outsideOnly for spatialNavigationSearch API which will decide where to find the best candidate first - inside or outside of the element.

The detail of how it works is:

  • Let container be the value of container option.
  • If the value of container option is null, container is the result of element.getSpatialNavigationContainer()
  • If the value of outsideOnly option is true, then find the best candidate outside of the element and inside the container.
    • If there is the best candidate, then return it.
    • Else, return null.
  • Else if the value of outsideOnly option is false, find the best candidate inside the element.
    • If there isn't any candidate inside the element, then find the best candidate outside of the element and inside the container.
      • If there is the best candidate, then return it.
      • Else, return null.
    • Else, return the best candidate.

IDL may change as following:

dictionary SpatialNavigationSearchOptions {
    sequence<Node>? candidates;
    Node? container;
    boolean outsideOnly = false;
}; 
 
Node? spatialNavigationSearch(SpatialNavigationDirection dir, 
                              optional SpatialNavigationSearchOptions arg);

More results of API with suggesting option will as below:

Code Result
container3.spatialNavigationSearch('right'); (same with container3.spatialNavigationSearch('right',{candidates: container3.focusableAreas(), container: container3, outsideOnly: false});) null
container3.spatialNavigationSearch('right',{candidates: container2.focusableAreas(), container: container2, outsideOnly: true}); C
container2.spatialNavigationSearch('right'); (same with container2.spatialNavigationSearch('right',{candidates: container2.focusableAreas(), container: container2, outsideOnly : false});) B
container2.spatialNavigationSearch('right',{outsideOnly: true}); (same with container2.spatialNavigationSearch('right',{candidates: container2.focusableAreas(), container: container2, outsideOnly: true});) null
container2.spatialNavigationSearch('right',{container: document, outsideOnly: true}); (same with container2.spatialNavigationSearch('right',{candidates: document.focusableAreas(), container: document, outsideOnly: true});) D
@jihyerish
Copy link
Contributor Author

While considering this issue, I think the result of getSpatialNavigationContainer() needs to be changed.

In the current spec, getSpatialNavigationContainer() returns the nearest spatial navigation container of the element.
If the element is a spatial navigation container, it returns the element itself.
But this may be uncomfortable and change to return the nearest ancestor spatial navigation container (a.k.a. spatnav container) of the element always for several reasons.

  1. It's tricky to know the nearest ancestor spatnav container of the element (A) which is a spatnav container.

    We should implement like:

let spatnavContainer = A.parentElement.getSpatialNavigationContainer();
  1. The result of getSpatialNavigationContainer() seems inconsistant and will confuse the developer who will use this API.

    This API returns the element itself or the nearest ancestor spatnav container depending on the condition of the element.
    It looks like this API has two features:
    (1) Knowing whether the element is a spatnav container or not
    (2) Knowing the nearest ancestor spatnav container of the element
    I think separating those two features and making this API just return the nearest ancestor spatnav container consistantly for will make clear about the feature of this API.

  2. Knowing whether the element is a spatnav container or not isn't hard.
    We need to check
    (1) Whether the element is a document or not
    (2) Whether the element is a scroll container
    (3) Whether the element is specified with spatial-navigation-container: contain.

    These conditions aren't hard to check as the API in JS lib.
    It would be nice if we have the new API in the spec such as isSpatialNavigationContainer(),
    but I don't feel this strongly.

@jihyerish
Copy link
Contributor Author

The behavior of getSpatialNavigationContainer() had been changed. It returns the nearest parent node, which is the spatial navigation container, even if the target element is a container(precisely, it's spatial navigation container, but I'll write it shortly in this thread).
This change affects the initial proposal, as below:

  • API searches the best candidate outside of the target element within the container of the target element. (even if the target element itself is the container)
    • This means that if the target element itself is the container, the API cannot find the best candidate inside the target element because when there isn't any candidate the API returns null.
    • Therefore, the outsideOnly in initially proposed IDL would be changed to inside
  • API returns the best candidate which meets all the conditions specified in SpatialNavigationSearchOptions.

I suggest to change the IDL of spatialNavigationSearch() :

dictionary SpatialNavigationSearchOptions {
    sequence<Node>? candidates;
    Node? container;
    boolean inside = false;
}; 

Node? spatialNavigationSearch(SpatialNavigationDirection dir, 
                                   optional SpatialNavigationSearchOptions arg);

The inside attribute indicates whether candidates should be selected inside of the target element or not.

  • It's false by default, but if it's true,
    • the API searches the best candidate inside the target element first.
    • If the best candidate isn't found inside, then it searches the best candidate outside the target element within the container.

The naming of attribute inside in SpatialNavigationSearchOptions isn't straightforward to me.
So it would be very helpful for me to know other suggestions instead of it.

The example below shows how the API with this IDL works:

image

Example Code Result
container2.spatialNavigationSearch(“right"); button2
container2.spatialNavigationSearch(“left"); null
container2.spatialNavigationSearch(“right“, {inside: true}); button1
container2.spatialNavigationSearch(“left“, {inside: true}); container3
container1.spatialNavigationSearch("right”); null
container1.spatialNavigationSearch(“left”); null
container1.spatialNavigationSearch("right”, {inside: true}); container2
container1.spatialNavigationSearch(“left”, {inside: true}); button2
container3.spatialNavigationSearch(“right“); null
container3.spatialNavigationSearch(“left"); button1
container3.spatialNavigationSearch(“right“, {inside: true}); null
container3.spatialNavigationSearch(“left“, {inside: true}); button1

FYI, the markup of the image would be:

<div id="container1" style="overflow-x: scroll; border: 1px solid red;">
  <div id="container2" style="overflow-x: scroll; border: 1px solid blue;">
    <button id="button1" style="background-color: green;"></button>
    <div id="container3" style="overflow-x: scroll; border: 1px solid yellow;"></div>  
  </div>  
  <button id="button2" style="background-color: purple;"></button>
</div>

@frivoal Would this change be reasonable?

@frivoal
Copy link
Collaborator

frivoal commented Jun 8, 2019

I'd suggest making two changes:

  • add an isContainer() (or some similar name) method to element
  • inside spatialNavigationSearch(options)'s definition change point 2.else.1.else to

else, let container be the element if it is a spatial navigation container, or its nearest spatial navigation container ancestor otherwise.

@jihyerish
Copy link
Contributor Author

  • add an isContainer() (or some similar name) method to element

I think this new API would be useful because it would be frequently used.
But isSpatialNavigationContainer() seems better name.

  • inside spatialNavigationSearch(options)'s definition change point 2.else.1.else to

else, let container be the element if it is a spatial navigation container, or its nearest spatial navigation container ancestor otherwise.

I think spatialNavigationSearch(options)'s default container needs to be set as the nearest ancestor spatial navigation container always.
Because this behavior should be the same as the behavior of getSpatialNavigationContainer().
getSpatialNavigationContainer() always returns the nearest ancestor spatial navigation container of the target element.
If those aren't the same, it will confuse the author because of the inconsistent methodology.

Also, with this change, if the author wants to find the best candidate outside of the target element ,
a line of code will be:
elm.spatialNavigationSearch("right", {container: elm.getSpatialNavigationContainer()});
Calling the function inside another is sometimes inconvenient, especially when this type of code is used frequently.
This would be much simpler in the suggestion, like:
elm.spatialNavigationSearch(“right");

jihyerish pushed a commit to WICG/spatial-navigation that referenced this issue Sep 26, 2019
For reducing the confusion when using spatialNavigationSearch API.

Related: w3c/csswg-drafts#3743
jihyerish pushed a commit that referenced this issue Sep 27, 2019
jihyerish pushed a commit to jihyerish/csswg-drafts that referenced this issue Sep 27, 2019
…ibute

* Add the step for using `inside` attribute of `option` argument
* Modify that the `candidates` are assigned after assigning the `container`.

Related: w3c#3743
Yeti-or pushed a commit to salute-developers/spatial-navigation that referenced this issue Apr 26, 2022
For reducing the confusion when using spatialNavigationSearch API.

Related: w3c/csswg-drafts#3743
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants