vaadin / flow-components

Java counterpart of Vaadin Web Components
101 stars 66 forks source link

grid.setTabIndex(-1) has no effect [1 day] #4280

Open provi-dominik opened 1 year ago

provi-dominik commented 1 year ago

Description

If I call setTabIndex(-1) on a grid component, the grid component isn't dropped from the tab focus cycle. The table element in the DOM tree has still the tab index 0: Bild1

Expected outcome

The grid component is dropped from the tab focus cycle.

Minimal reproducible example

@Route("/grid") @PageTitle("Grid-Example") public class ExampleGrid extends Div {

private static final long serialVersionUID = 202110120935L;

private List<Employee> employees;
private Grid<Employee> grid;

public ExampleGrid() {
    super();
    this.initData();
    this.initView();
}

private void initData() {
    this.employees = new ArrayList<>();
    this.employees.add(new Employee("Mayhem", "Miles"));
    this.employees.add(new Employee("Mayhem", "Maximus"));
}

private void initView() {
    this.grid = new Grid<>(Employee.class);
    this.grid.setTabIndex(-1);

    this.grid.setWidthFull();
    this.grid.setItems(this.employees);
    this.add(this.grid);
}

public class Employee
        implements Serializable {
    private String surname;
    private String givenName;
    private String dateOfBirth;

    public Employee(String surname, String givenName) {
        this.surname = surname;
        this.givenName = givenName;
        this.dateOfBirth = "12.12.1912";
    }

    public String getSurname() {
        return this.surname;
    }

    public void setSurname(String name) {
        this.surname = name;
    }

    public String getGivenName() {
        return this.givenName;
    }

    public void setGivenName(String vorname) {
        this.givenName = vorname;
    }

    /**
     * Antwortet das Attribut {@link #dateOfBirth}.
     * @return das Attribut {@link #dateOfBirth}
     */
    public String getDateOfBirth() {
        return this.dateOfBirth;
    }

    public int getAge() {
        return 41;
    }

    /**
     * Setzt das Attribut {@link #dateOfBirth}.
     * @param dateOfBirth
     */
    public void setDateOfBirth(String dateOfBirth) {
        this.dateOfBirth = dateOfBirth;
    }

}

}

Steps to reproduce

  1. Tab through the application
  2. See that the grid is still focused

Environment

Vaadin version(s): 14.9.2 OS: Windows 10 Enterprise (Build 20H2, OS build 19042.1889)

Browsers

Chrome

rolfsmeds commented 1 year ago

Indeed, Grid's focusability is only partially implemented at all: programmatically focusing the Grid is similarly non-functional: https://github.com/vaadin/flow-components/issues/2180.

I think both issues should be fixed together.

One important detail is that the Grid as a whole is not focusable at all. Instead, focus is always on one of the cells.

Some open questions about the desired behavior:

provi-dominik commented 1 year ago

I would desire the following behavior:

rolfsmeds commented 1 year ago

Should setTabindex(-1) disable focusing focusable elements in cells? <- No, only the keyboard-traversal through focusable elements in cells should be disabled

Should setTabindex(-1) disable focusing cells in edit mode in Grid Pro? <- No, only the keyboard-traversal through cells in edit mode should be disabled

Should setTabindex(-1) disable focusing content in item details panels? <- No, only the keyboard-traversal through content in item details should be disabled

Should setTabIndex(-1) disable focusing content in an edit-mode row in Flow Grid's inline edit mode? <- No, only the keyboard-traversal through content in an edit-mode row should be disabled

Does this mean that these focusable elements would still be present in the tab-order, so that, tabbing in from the "above" the Grid, keyboard focus would land straight into the first focusable element / cell editor, essentially bypassing the rest of the Grid?

provi-dominik commented 1 year ago

No, I mean that the mentioned elements should not be present in the tab-order (that is what I mean with "disabling the keyboard-traversal"). The elements should be focusable as described in the Javadoc of the Focusable interface: "A negative value (usually tabindex = -1) means that the component should be focusable, but should not be reachable via sequential keyboard navigation."

rolfsmeds commented 1 year ago

Right, ok, sorry, I was a bit unclear in my questions: I meant keyboard-focusability in each one.

So, TL;DR: no keyboard-focusability on any element in the Grid, only focusable by mouse?

provi-dominik commented 1 year ago

Yes, this is the behavior I would expect.

web-padawan commented 1 year ago

The thing is, in HTML setting tabindex on the element to -1 excludes it from sequential navigation, but not its children. So if you set tabindex="-1" on e.g. vaadin-scroller, elements inside it would be still keyboard focusable.

While some might argue that vaadin-grid is a single element, it has several internal elements that participate in the Tab order (that in HTML Standard terminology is referred to as being "sequentially focusable") for accessibility reasons.

So I would prefer to not change the current behavior of setTabindex method. Instead, a new API would make sense:

  1. Add no-keyboard-focus attribute to the vaadin-grid web component,
  2. Add setKeyboardFocusable(false) method to the Flow counterpart.

These would need to be properly documented as not recommended for usage because of accessibility concerns.

rolfsmeds commented 1 year ago

I feel inclined to agree with that assessment. I'm also curious to hear the use cases for excluding the Grid tab stop.

web-padawan commented 1 year ago

Actually, setting tabindex="-1" on the web component does seem to exclude the grid from keyboard navigation (so we might need to do a research on why Flow API doesn't work). But I still think it would be good to have a dedicated API.

TatuLund commented 1 year ago

I found out a curious thing. When I am testing Vaadin 23.3.0 using grid.setTabIndex(-1)seems to work fine.

rolfsmeds commented 1 year ago

(Added a few more colorful labels because it's Christmas!)

If I understand the situation correctly:

So we need to decide a few things:

web-padawan commented 1 year ago

Just to clarify: setting tabindex="-1" on the vaadin-grid excludes it from keyboard navigation sequence when pressing Tab or Shift + Tab (which makes sense if you want the user to just "skip" grid and its content).

But as soon as user clicks a cell - for example an editable cell in vaadin-grid-pro - they can still navigate cell content with Tab again. Not sure if this is something that people would expect, but it's how the grid is supposed to work.

You can test this in web component by using the following HTML snippet (focus the first <input> and press Tab):

<input placeholder="Before" />

<vaadin-grid-pro tabindex="-1">
  <vaadin-grid-pro-edit-column path="name.first"></vaadin-grid-pro-edit-column>
  <vaadin-grid-pro-edit-column path="name.last"></vaadin-grid-pro-edit-column>
</vaadin-grid-pro>

<input placeholder="After" />

<script type="module">
  import '@vaadin/grid-pro';
  import '@vaadin/grid-pro/vaadin-grid-pro-edit-column.js';

  const grid = document.querySelector('vaadin-grid-pro');
  grid.items = [
    {
      name: {
        first: 'John',
        last: 'Doe',
      },
    },
    {
      name: {
        first: 'Jane',
        last: 'Doe',
      },
    },
  ];
</script>
rolfsmeds commented 1 year ago

I would still like to hear from @provi-dominik what the use case for disabling keyboard focusing on the Grid is.

provi-dominik commented 1 year ago

@rolfsmeds: This is one use case in our application: We show a dialog to display error messages. In this dialog the user has to interact with some components dependening on these error messages. For example the user has to select an entry in a Select component and confirm the choice. So that the user can process the dialog quickly and is not confused by an unexpected focus in the table, the table should not be in the keyboard navigation sequence. The table is only intended to represent the messages.