From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id D0B421FF09C for ; Mon, 02 Nov 2026 15:12:20 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id A08152166F; Mon, 02 Nov 2026 15:12:20 +0100 (CET) Message-ID: <53d706ba-5154-4369-8d67-ccb009adb23a@proxmox.com> Date: Mon, 2 Nov 2026 15:12:14 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH proxmox-yew-widget-toolkit 3/5] widget: grid picker: keep selected records hidden by the filter To: Nicolas Frey , yew-devel@lists.proxmox.com References: <20261028151517.518742-1-n.frey@proxmox.com> <20261028151517.518742-4-n.frey@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: <20261028151517.518742-4-n.frey@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793628735199 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.336 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: UNZFU3LFYRNTC2X4RPSSPCODIQMFYAUG X-Message-ID-Hash: UNZFU3LFYRNTC2X4RPSSPCODIQMFYAUG X-MailFrom: d.csapak@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Yew framework devel list at Proxmox List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 > --- > 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; > 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. > #[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 DataStore for Store { > 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 + '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 DataStore for TreeStore { > 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 + '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 { > #[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). > /// Table scroll callback > #[prop_or_default] > pub on_table_scroll: Option>, > @@ -395,6 +403,17 @@ impl DataTable { > 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) -> 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) { > + self.keep_filtered_selection = keep.into_prop_value(); > + } > + these could be automatically implemented by using the #[builder] macro > /// Builder style method to set the show_header flag. > pub fn show_header(mut self, show_header: impl IntoPropValue) -> Self { > self.set_show_header(show_header); > @@ -765,6 +784,9 @@ impl PwtDataTable { > } > > let mut keys: HashSet = 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 PwtDataTable { > 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... > if record_count == selection_len { > self.selection_status = RowSelectionStatus::All; > } else if selection_len > 0 { > @@ -812,13 +838,22 @@ impl PwtDataTable { > if let Some(selection) = &props.selection { > let mut selection = selection.write(); > if selection.is_multiselect() { > - let mut keys: HashSet = 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 Component for PwtGridPicker { > .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)