Re: [PATCH proxmox-yew-widget-toolkit 3/5] widget: grid picker: keep selected records hidden by the filter

["Nicolas Frey" <n.frey@proxmox.com>]

On Mon Nov 2, 2026 at 3:12 PM CET, Dominik Csapak wrote:
> comments inline
>
> On 10/28/26 4:15 PM, Nicolas Frey wrote:
>> the data table drops selected records that do not match the store
>> filter. in a grid picker, the filter is only a search aid, so typing a
>> query deselected all records that did not match it.
>>
>> add an opt-in flag to only drop records that are no longer in the
>> store, using a new function `DataStore::contains_key`, and enable it in
>> the grid picker.
>>
>> Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
>> ---
>>   src/state/data_store.rs             |  3 ++
>>   src/state/store.rs                  |  4 +++
>>   src/state/tree_store/mod.rs         |  4 +++
>>   src/widget/data_table/data_table.rs | 51 ++++++++++++++++++++++++-----
>>   src/widget/grid_picker.rs           |  2 ++
>>   5 files changed, 56 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/state/data_store.rs b/src/state/data_store.rs
>> index 90460c2..16b59ea 100644
>> --- a/src/state/data_store.rs
>> +++ b/src/state/data_store.rs
>> @@ -64,6 +64,9 @@ pub trait DataStore: Clone + PartialEq {
>>       fn filtered_record_pos(&self, key: &Key) -> Option<usize>;
>>       fn filtered_data_len(&self) -> usize;
>>
>> +    /// Returns whether the store contains a record with this key, regardless of the filter.
>> +    fn contains_key(&self, key: &Key) -> bool;
>> +
>
> This is a bit too narrow for my taste for such an important part of
> the interface. I'd probably like more 'unfiltered' methods
> that mirror the 'filtered' ones. with that, your contains_key
> is just a 'unfiltered_record_pos(_).is_some()'
>
> Also even when adding only the 'contains' method, i'd add the
> 'unfiltered' part, otherwise this can be misunderstood without
> looking at the docs.
>

makes sense, will adjust in a v2, thanks!

>>       #[allow(clippy::type_complexity)]
>>       fn filtered_data<'a>(
>>           &'a self,
>> diff --git a/src/state/store.rs b/src/state/store.rs
>> index 1afe0a4..89a1ad4 100644
>> --- a/src/state/store.rs
>> +++ b/src/state/store.rs
>> @@ -429,6 +429,10 @@ impl<T: Clone + PartialEq + 'static> DataStore for Store<T> {
>>           self.filtered_data_len()
>>       }
>>
>> +    fn contains_key(&self, key: &Key) -> bool {
>> +        self.read().record_pos(key).is_some()
>> +    }
>> +
>>       fn filtered_data<'a>(
>>           &'a self,
>>       ) -> Box<dyn Iterator<Item = (usize, Box<dyn DataNode<T> + 'a>)> + 'a> {
>> diff --git a/src/state/tree_store/mod.rs b/src/state/tree_store/mod.rs
>> index 5e84b8f..67f0d76 100644
>> --- a/src/state/tree_store/mod.rs
>> +++ b/src/state/tree_store/mod.rs
>> @@ -288,6 +288,10 @@ impl<T: Clone + PartialEq + 'static> DataStore for TreeStore<T> {
>>           self.filtered_data_len()
>>       }
>>
>> +    fn contains_key(&self, key: &Key) -> bool {
>> +        self.read().lookup_node(key).is_some()
>> +    }
>> +
>>       fn filtered_data<'a>(
>>           &'a self,
>>       ) -> Box<dyn Iterator<Item = (usize, Box<dyn DataNode<T> + 'a>)> + 'a> {
>> diff --git a/src/widget/data_table/data_table.rs b/src/widget/data_table/data_table.rs
>> index 90f37ea..2ee8ed2 100644
>> --- a/src/widget/data_table/data_table.rs
>> +++ b/src/widget/data_table/data_table.rs
>> @@ -237,6 +237,14 @@ pub struct DataTable<S: DataStore> {
>>       #[prop_or_default]
>>       pub multiselect_mode: MultiSelectMode,
>>
>> +    /// In multiselect mode, keep selected records that are hidden by the store filter.
>> +    ///
>> +    /// By default, the selection is reduced to the records that match the filter, so that
>> +    /// actions on the selection only affect visible rows. Enable this if the filter is only a
>> +    /// search aid, and must not change the selection.
>> +    #[prop_or_default]
>> +    pub keep_filtered_selection: bool,
>> +
>
>
> With this i asked myself: 'what is a filtered selection' ?
>
> I think something like 'keep_selection_when_filtered' would be more
> descriptive (even though it's longer).
>

ack

>
>>       /// Table scroll callback
>>       #[prop_or_default]
>>       pub on_table_scroll: Option<Callback<Event>>,
>> @@ -395,6 +403,17 @@ impl<S: DataStore> DataTable<S> {
>>           self.allow_deselect = allow_deselect.into_prop_value();
>>       }
>>
>> +    /// Builder style method to set the keep_filtered_selection flag.
>> +    pub fn keep_filtered_selection(mut self, keep: impl IntoPropValue<bool>) -> Self {
>> +        self.set_keep_filtered_selection(keep);
>> +        self
>> +    }
>> +
>> +    /// Method to set the keep_filtered_selection flag.
>> +    pub fn set_keep_filtered_selection(&mut self, keep: impl IntoPropValue<bool>) {
>> +        self.keep_filtered_selection = keep.into_prop_value();
>> +    }
>> +
>
> these could be automatically implemented by using the #[builder] macro
>

ah yeah forgot, as all the other ones are written out without the macro
I just went with this. I'll update the others with #[builder] too in a
separate commit

>>       /// Builder style method to set the show_header flag.
>>       pub fn show_header(mut self, show_header: impl IntoPropValue<bool>) -> Self {
>>           self.set_show_header(show_header);
>> @@ -765,6 +784,9 @@ impl<S: DataStore> PwtDataTable<S> {
>>           }
>>
>>           let mut keys: HashSet<Key> = HashSet::new();
>> +        if props.keep_filtered_selection {
>> +            keys.extend(selection.selected_keys());
>> +        }
>>           for pos in 0..record_count {
>>               if let Some(key) = props.store.lookup_filtered_record_key(pos) {
>>                   keys.insert(key);
>> @@ -797,7 +819,11 @@ impl<S: DataStore> PwtDataTable<S> {
>>               return;
>>           }
>>
>> -        let selection_len = selection.len();
>> +        // only count visible rows, the selection may contain records hidden by the filter
>> +        let selection_len = (0..record_count)
>> +            .filter_map(|pos| props.store.lookup_filtered_record_key(pos))
>> +            .filter(|key| selection.contains(key))
>> +            .count();
>
> since the selection is basically a hashset, we could think
> of exposing a the usual set operations too (intersect, difference, etc)
> and offer a 'filtered_get_keys' that returns a hashset of the existing
> (visible) keys
>
> nothing wrong with this code here, per se but exposing more of the
> selection might benefit us in the future too...
>

ack

>>           if record_count == selection_len {
>>               self.selection_status = RowSelectionStatus::All;
>>           } else if selection_len > 0 {
>> @@ -812,13 +838,22 @@ impl<S: DataStore> PwtDataTable<S> {
>>           if let Some(selection) = &props.selection {
>>               let mut selection = selection.write();
>>               if selection.is_multiselect() {
>> -                let mut keys: HashSet<Key> = HashSet::new();
>> -                for (_pos, node) in props.store.filtered_data() {
>> -                    let key = props.store.extract_key(&node.record());
>> -                    if selection.contains(&key) {
>> -                        keys.insert(key);
>> -                    }
>> -                }
>> +                let keys = if props.keep_filtered_selection {
>> +                    selection
>> +                        .selected_keys()
>> +                        .into_iter()
>> +                        .filter(|key| props.store.contains_key(key))
>> +                        .collect()
>> +                } else {
>> +                    props
>> +                        .store
>> +                        .filtered_data()
>> +                        .filter_map(|(_pos, node)| {
>> +                            let key = props.store.extract_key(&node.record());
>> +                            selection.contains(&key).then_some(key)
>> +                        })
>> +                        .collect()
>> +                };
>>                   selection.bulk_select(keys);
>>               } else if let Some(key) = selection.selected_key() {
>>                   if props.store.filtered_record_pos(&key).is_none() {
>> diff --git a/src/widget/grid_picker.rs b/src/widget/grid_picker.rs
>> index b40ac92..d3ac5ae 100644
>> --- a/src/widget/grid_picker.rs
>> +++ b/src/widget/grid_picker.rs
>> @@ -185,6 +185,8 @@ impl<S: DataStore + 'static> Component for PwtGridPicker<S> {
>>               .clone()
>>               .key(Key::from("picker-table"))
>>               .autoselect(false)
>> +            // the filter is only a search aid and must not change the selection
>> +            .keep_filtered_selection(true)
>>               .cell_configuration(CellConfiguration::new().class("pwt-datatable-cell pwt-pointer"))
>>               .hover(true)
>>               .header_focusable(false)