vaadin / web-components

A set of high-quality standards based web components for enterprise web applications. Part of Vaadin 20+
https://vaadin.com/docs/latest/components
463 stars 83 forks source link

[grid-pro] Make it possible to select row by clicking editable cell #637

Closed finaris-cs closed 5 months ago

finaris-cs commented 3 years ago

When I single-click an editable cell in GridPro, the visual focus is updated, but registered SelectionListeners on the grid are not notified. For non-editable columns, everything works as expected.

Would be nice if this could be fixed, also any workaround could be sufficient.

TatuLund commented 3 years ago

This is somewhat related to https://github.com/vaadin/vaadin-grid/issues/1934

Or if you have e.g. ComponentRenderer in the cell, the component in it will hijack the click, and Grid underneath is not able to catch it.

vlukashov commented 3 years ago

The current behavior is by design - starting editing a cell switches the grid-pro component into the 'edit' mode where selection does not apply. Handling selection in a different way would require a different design.

rolfsmeds commented 3 years ago

Changed the title to reflect that this is an enhancement proposal.

enver-haase commented 6 months ago

The current behavior is by design - starting editing a cell switches the grid-pro component into the 'edit' mode where selection does not apply. Handling selection in a different way would require a different design.

Please can someone double-check that this makes sense? After all, a SINGLE click does not start the editing mode.

rolfsmeds commented 6 months ago

Agree – as long as edit-on-click i.e. single-click editing is disabled, as it is by default, single clicking a cell should select the row if selection is enabled. Relabelling as a bug.

web-padawan commented 6 months ago

The issue is added to the backlog priority queue for further investigation.

web-padawan commented 6 months ago

It looks like the current behavior is caused by preventing active-item-changed event on editable cell click. This logic was introduced in https://github.com/vaadin/vaadin-grid-pro/pull/84 alongside with the following unit test that covers it:

https://github.com/vaadin/web-components/blob/b3ac7f7053ea6a03b19becad32187a2249184f80/packages/grid-pro/test/edit-column.common.js#L659-L671

tomivirkki commented 6 months ago

I'd imagine it's originally been implemented this way to avoid having the selection change as a side effect when the user double-clicks a cell to edit it:

https://github.com/vaadin/web-components/assets/1222264/a6f07c35-3284-476c-a887-04591259b206

Changing the current behavior might lead to unexpected UX (unintentional selection changes) for some users:

https://github.com/vaadin/web-components/assets/1222264/959f8f16-db6f-4826-a143-4c55e1ac148a

rolfsmeds commented 6 months ago

So the challenge here is that selection should be triggered by single click, while editing should be triggered with double click.

If selection were to happen immediately on click, as it does in non-editable cells, double-clicking would also trigger selection, since the first click would select before the second click triggers editing. So we would want to distinguish between single and double click.

But to distinguish between the two, the component would need to wait to see if a second click is coming, before acting on the single click, i.e. a delay of ~500ms before selection changes.

That's a long enough delay to negatively affect the user experience, and may lead users to click again because it seems that the first click didn't register, which then would trigger editing instead, or, if they've waited just long enough, it might cause the row to be immediately de-selected instead.

While this delay would only be needed on editable cells, the difference in behavior between editable and non-editable cells may actually make things worse, as the quick response of non-editable cells conditions users to expect selection to be very responsive, which then further encourages them to click twice.

Solution suggestions are welcome.

finaris-cs commented 6 months ago

What about a Mode applicable to the grid, which determines the selection handling?

rolfsmeds commented 6 months ago

Perhaps it would help to understand the use cases for a Grid that has both inline editing and single selection.

Personally I can think of two scenarios:

  1. Grids where you need to be able to edit some cells and also select a cell e.g. to delete, duplicate or perform some other non-editing action on it. (In this case you probably don't want to select the row you're editing.)
  2. Grids where selection and editing of a cell are somehow closely related, e.g. the UI can display some additional information about the selected item that's useful while editing the cells of that item. (In this case you actually want selection and editing to go hand in hand, maybe even so that when you select the row the first cell in it goes to edit mode automatically.)

Could you @finaris-cs share some details of what your use case it (maybe even a screenshot)?

finaris-cs commented 6 months ago

Our use-case is your scenario 2.

subITCSS commented 5 months ago

And our useCase would be scenario 1.

rolfsmeds commented 5 months ago

Thanks. Obviously reconciling these two behaviors complicates matters: in the first case you want to separate selection and editing (i.e. double click to edit should not also select), while in the other you want them to go hand-in-hand.

Some thoughts (not arguments against this feature per se):

Scenario 1 – separate editing from selection:

I would probably lean towards doing this with a separate column for selection, and single-click editing. I think this would be better in terms of UX and accessibility, as users wouldn't have to figure out the two different ways to interact with a cell. (There is a feature proposal for having that built-in, but it would be fairly easy to do a custom implementation.) @subITCSS would that work for you?

Scenario 2 – combined editing and selection:

I think this could be achieved by programmatically selecting the row that's being edited: gridpro.addCellEditStartedListener(e->gp.select(e.getItem())); Maybe even switch to single-click editing. @finaris-cs would that work for you?

finaris-cs commented 5 months ago

would be fine

subITCSS commented 5 months ago

@rolfsmeds It would be a solution but i think a much more easy way for us would be having a ItemClickEvent. So when a cell in a multiselection grid wich is editable is clicked we want to get an event (a new click event would also be work for us). So we just need the ability to listen on a cell click when the user is in multiselection.

rolfsmeds commented 5 months ago

@subITCSS that would be significantly less problematic than automatically toggling selection on click – then your app can handle the situation as it sees best.

rolfsmeds commented 5 months ago

According to @tomivirkki, the click event used to fire on edit cells as well, at least back in V14, and we're not aware of any explicit decision to disable that, so it may even be a regression.

tomivirkki commented 5 months ago

The regression mentioned above has been fixed by https://github.com/vaadin/web-components/pull/7453

GridPro still doesn't automatically change selection on edit column cell clicks, but the selection can now be changed programmatically using an item click listener as suggested here:

grid.addItemClickListener(e -> grid.select(e.getItem()));

The fix will be shipped with Vaadin 24.4. Please let us know if it's required for earlier versions also.