API for refreshing items without clearing selection/value

yuriy-fix commented 9 months ago

Description

Introducing a new API for selection components, such as MultiSelectListbox, Grid, or Checkbox Group. This API offers configuration options for item refreshing using DateProvider, allowing user to choose from three modes:

Refreshing without clearing the current selection/value.
Refreshing with clearing non-existent selection/value.
Refreshing with clearing the current selection/value.

Use cases

As a java developer I want to refresh the items shown in a selection component (e.g. Checkbox Group) without clearing the selection So that I can update the list of items without the need to reapply my selections manually Example: https://github.com/vaadin/flow-components/issues/5385

As a java developer I want to refresh the items shown in a component, that allows items selection (e.g. Grid), while filtering the current selection So that I can:

apply a filter to the items and keep the selection only for those items that satisfy the given filter
clear the selection for filtered-out items, so they won't be selected after filter reset.

Example: https://github.com/vaadin/flow-components/issues/4592

As a java developer I want to maintain the existing behaviour and refresh the items displayed in a selection component while clearing the current selection. So that I can update the list and keep all items.

Acceptance criteria

[ ] New API at the component level should be introduced (The suggested draft name for this API is setSelectionOnDataChange)
[ ] New API accepts three modes:
- [ ] PRESERVE_ALL
- [ ] PRESERVE_EXISTENT (It should throw an exception when component uses lazy loading of items)
- [ ] DISCARD

General criteria

[ ] APIs reviewed
[ ] Design
[ ] Performance
[ ] UX/DX tests in Alpha
[ ] Documentation:
[ ] How to test?
[ ] Limitations:

mstahv commented 5 months ago

I got today "rather irritated" by this bug, or techically bugs related to this enhancement. Not the first time. Clearing the value when setting items belongs to this same group of desing flaws. Please get these things fixed!

In my app today my form edits a name field in my dto. That happens to be used in the "itemlabelgenerator" in the select, so I would like to refresh that item. In an other part of the app, these dtos are added/removed, there I'd like those to appear/be removed from the select (that selects the entity edited in the form). These bugs (or unexpected design decisions 🤪), make it very hard to develop this correctly. Obsolete null checks in several places, and non-consisten behaviour amont different "select implementations"

Summary of issues/oddities I found today

With Select, the component I have chosen so that its value cannot be unset (simplifies my code as I in theory wouldn't need null checks in my listeners), clears its value if I call getGenericDataView().refreshAll(). Same with setItems. As a developer, I wouldn't expect that and I most often wouldn't want that. If I would want the value to be cleared, I would want to clear it, I would do that by myself using clear() method, mut more likely I would like to keep the current value or call setValue with a possible new value I would like to assing. Note, this is not just two extra lines of code where I first store the old value and then reset it after refreshAll()/setItems(), I need to make sure other listeners don't throw NPEs or do something sane when the Select component desides to be "intelligent".

When fiddling with this I figured out that that ComboBox seems to work "the right way". This "intelligent resetting of the value" just doesn't work with lazy loading, and this is why it probably is disabled there (might be my "fault" from the days when I was PM for flow and we re-factored the lazy loading APIs). Same for Grid. But, 1) none of the candidates (refreshAll/refreshItem/setItems) refresh the caption of the currently selected item, only the item in the dropdown 2) setItems seem to clear the value, unless a lazy data binding is in use.

Suggestion

Do not add any API. That is not needed and there is no problemn in the API itself. Only remove the weird behaviour, that complicates our component implementations and simply irritates users. This current "helpful logic" that goes and messes the UI state when I want to update a label of an option or add/remove options, can be easily implemented by framework users if they need it. Keep the value of the select components separate of the available items if possible.

I would only consider adding some API temporarily to v24 series if we could get rid of this insanity there in backwards compatible manner.

And fix ComboBox not updating "itemLabel" of selected component when refreshItem is called. Not sure if the issue exists, there was too many related issues to search for a Friday night.

yuriy-fix commented 5 months ago

In version 24, altering the behavior or defaults is not feasible. The API was discussed and designed to meet the diverse requirements outlined in the AC. Should we have the opportunity to design the entire logic from the scratch now, our approach would differ. Therefore, we will consider a change in the defaults in the upcoming major release.

vaadin / platform