iTwin / iTwinUI

A design system for building beautiful and well-working web interfaces.
https://itwin.github.io/iTwinUI/
MIT License
104 stars 37 forks source link

Table with not editable cells can't be click to select the row in single selection mode #2130

Closed maxime-lavergne closed 1 week ago

maxime-lavergne commented 1 week ago

Describe the bug (current behavior)

When using the Table component with single selection, rows that contain cells with text (not editable) don't detect a click event, so the row isn't selected.

For example, if a user clicks on any red boxes in this picture, the row won't be selected, even if the user can't change the text: image

Expected Behavior

If the table is not editable, the row should be selected OR if a user click on a component that isn't editable, the row should be selected.

Link to minimal repro

https://itwin.github.io/iTwinUI/react/?story=table--selectable-single

Steps To Reproduce

  1. Open a page with a Table with single selection
  2. Click on a cell where there's no editable content
  3. Notice the row isn't selected

Anything else?

It was working in previous itwinui version: https://itwin.github.io/iTwinUI-react/?path=/story/core-table--selectable-single

mayank99 commented 1 week ago

Hey, thanks for opening this issue. In short, this behavior is intentional and you should add a <button> instead of relying on the static text being clickable. Here's a sandbox showing how you can do that: https://stackblitz.com/edit/github-efehsf?file=src%2FApp.tsx

Longer explanation:

In a normal multi-selectable Table, there are checkboxes which can be clicked to select/deselect the rows and also indicate whether a row is selected. This is the primary means of performing the selection. After this, there is usually another action that the user performs on the selected set of rows (e.g. "delete", "copy"); this is usually done by clicking a button outside the Table. (Note: There might also be additional actions available within the cells, but those should generally use stopPropagation to avoid unintentionally selecting the row).

However, in a single-selection Table, there are no checkboxes because checkboxes don't make sense when multiple rows cannot be selected. This means there must be another primary means of performing the selection. Since single-selection Tables usually also perform the action right away upon selection (e.g. viewing something on the canvas, or opening a panel showing more information), it makes sense to provide a button specifically intended for such actions.

You can think of clicking this button as the primary way to select rows. Clicking anywhere on the row should be an additional (not primary) way to do the same.

So while you are right in bringing up the issue that "only clicking outside the text selects row", the proper fix isn't to select the rows when clicking text, but to instead add buttons that are obviously meant to be clickable. (We should probably update our examples to demonstrate this). Static text should never be expected to perform an action upon click; the default behavior of clicking static text is text selection, which is often useful if the user wants to copy something to their clipboard.

Furthermore, by using buttons, you make your interface more accessible. Below you can see that without buttons, a keyboard user cannot select the rows at all; the focus skips past the Table entirely. Whereas when using buttons, the user can press the Tab key to move focus between the buttons and press Enter to "click"/select it.

Without <button>:

https://github.com/iTwin/iTwinUI/assets/9084735/0b54b497-56ee-47a9-ad10-3638965bf902

With <button>:

https://github.com/iTwin/iTwinUI/assets/9084735/2cc3cc5d-c872-4811-aa18-ef0a2283cbcd

Hope that makes sense!