Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Disabled Grid allows focusing cells using keyboard #1831

Closed
4 tasks done
anezthes opened this issue Jun 22, 2021 · 8 comments
Closed
4 tasks done

Disabled Grid allows focusing cells using keyboard #1831

anezthes opened this issue Jun 22, 2021 · 8 comments

Comments

@anezthes
Copy link
Contributor

anezthes commented Jun 22, 2021

Description

A disabled Grid doesn't allow selecting items using pointing devices, but it is possible to select an item (at least visually) using keyboard.

Expected outcome

Selection should be fully disabled.

Actual outcome

Selection is possible using keyboard.

Minimal reproducible example

Grid<Item> grid = new Grid();
grid.addColumn(Item::getName).setHeader("Name");
grid.setEnabled(false);

ArrayList<Item> items = new ArrayList();
for (int i = 0; i < 10; i++) {
  items.add(new Item("Item " + i));
}
grid.setItems(items);

...

private class Item {

  String name;

  Item(String name) {
    this.name = name;
  }

  public String getName() {
    return name;
  }
}

Environment

  • Vaadin 14.6.3 (Flow)
  • macOS 11.2.3

Browsers Affected

  • Chrome
  • Firefox
  • Safari
  • Edge
@tomivirkki tomivirkki transferred this issue from vaadin/web-components Jun 22, 2021
@vlukashov
Copy link

NOTE: the vaadin-grid web-component does not have a disabled mode at all.

@rolfsmeds rolfsmeds changed the title Disabled Grid allows selection using keyboard Disabled Grid allows focusing cells using keyboard Jun 29, 2021
@rolfsmeds rolfsmeds removed Impact: High Severity: Major bug Something isn't working labels Jun 29, 2021
@rolfsmeds
Copy link
Contributor

Changed the title to refer to cell focus rather than selection, as selection is correctly disabled, but cell focus works normally (both with mouse and keyboard, although only the latter has a visual indication).

Question is: Should cell focus, or indeed all focus, be disabled when the Grid is disabled?

@rolfsmeds
Copy link
Contributor

Related: col sort arrows are togglable in disabled grid (sorting doesn't happen, but the arrow states change) #1555

@rolfsmeds
Copy link
Contributor

Ping @jouni @tomivirkki for comments.

@tomivirkki
Copy link
Member

Grid shouldn't have setEnabled API in the first place, because <vaadin-grid> doesn't have disabled mode/feature either (as stated above).

I'd be in favor of first designing how <vaadin-grid disabled> should work in general and then implementing it in the Web Component as a feature. Once the feature is in place, the disabled mode should be taken into account whenever a new feature gets added to <vaadin-grid> to avoid inconsistent behavior, like the one in question, between the Flow and Web components.

In case disabled doesn't make sense for <vaadin-grid>, the erroneous setEnabled API could just be removed from the Flow Grid.

@DiegoCardoso
Copy link
Contributor

In case disabled doesn't make sense for , the erroneous setEnabled API could just be removed from the Flow Grid.

I guess disabled makes sense to Flow Grid because of the Editor:

grid-disabled.mp4

@tomivirkki
Copy link
Member

If it's only required for the editor (another Flow-only feature), there could be a dedicated API for disabling/enabling the editor.

@rolfsmeds
Copy link
Contributor

Created a separate ticket for implementing a proper disabled state in vaadin-grid. Closing this in favor of that.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

5 participants