Paul Trowbridge
f5d0f6b2f6
ViewConfig::apply_update in perspective-client applies ten fields and
neither group_by_depth nor split_by_depth is among them. A depth therefore
works when a view is created -- table.view({ group_by_depth: 1 }), which is
what the fork's own depth_test.mjs exercises -- and is silently discarded by
restore(), which is how a viewer changes its own configuration. That is why
the EXPAND buttons did nothing however the value was sent.
group_by_depth and the omission are both upstream; the fork mirrored
split_by_depth alongside it faithfully, including the omission. So the column
axis expand/collapse the fork added has the same hole, and pf_app only avoids
it by collapsing columns through a truncated split_by instead.
The patch adds an Option-aware sibling to _apply and applies both fields. It
cannot be built here -- protoc is absent, so the generated protobuf modules
are missing and the crate does not compile for unrelated reasons -- but
cargo check reports nothing against view_config.rs. It sits in ui/vendor with
the rebuild instructions, beside the tarballs it is not yet in.
Meanwhile applyDepth reads the config back after restoring and falls back to
view.set_depth() when the value did not stick, so the buttons work on the
current build. The fallback loses depth on a view rebuild, as it always did;
what is not back is the shim and listeners that used to chase that. The check
clears itself once the engine honours the field.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
46 lines
2.4 KiB
Diff
46 lines
2.4 KiB
Diff
diff --git a/rust/perspective-client/src/rust/config/view_config.rs b/rust/perspective-client/src/rust/config/view_config.rs
|
|
index fa4f36e..360b816 100644
|
|
--- a/rust/perspective-client/src/rust/config/view_config.rs
|
|
+++ b/rust/perspective-client/src/rust/config/view_config.rs
|
|
@@ -526,6 +526,23 @@ impl ViewConfig {
|
|
}
|
|
}
|
|
|
|
+ /// `_apply` for a field which is itself `Option`, where `None` in the update
|
|
+ /// means "not mentioned" rather than "clear it". `Option<Option<T>>` would be
|
|
+ /// needed to express both, and the wire format cannot carry the difference:
|
|
+ /// these fields are `skip_serializing_if = "Option::is_none"`, so an absent
|
|
+ /// field and an explicit null arrive identically. To lift a depth, send the
|
|
+ /// number of levels on that axis rather than clearing it.
|
|
+ fn _apply_optional<T: PartialEq>(field: &mut Option<T>, update: Option<T>) -> bool {
|
|
+ match update {
|
|
+ None => false,
|
|
+ Some(_) if *field == update => false,
|
|
+ Some(_) => {
|
|
+ *field = update;
|
|
+ true
|
|
+ },
|
|
+ }
|
|
+ }
|
|
+
|
|
pub fn reset(&mut self, reset_expressions: bool) {
|
|
let mut config = Self::default();
|
|
if !reset_expressions {
|
|
@@ -568,6 +585,16 @@ impl ViewConfig {
|
|
changed = Self::_apply(&mut self.windows, update.windows) || changed;
|
|
changed = Self::_apply(&mut self.group_rollup_mode, update.group_rollup_mode) || changed;
|
|
changed = Self::_apply(&mut self.split_rollup_mode, update.split_rollup_mode) || changed;
|
|
+
|
|
+ // Without these two, a depth can be set when a view is created --
|
|
+ // `table.view({ group_by_depth })` -- but never through `restore()`, which
|
|
+ // merges a `ViewConfigUpdate` onto the live config. The field arrives,
|
|
+ // deserializes, and is then dropped here, so the viewer's config and the
|
|
+ // engine never see it and nothing happens. That makes expand/collapse
|
|
+ // unreachable for anything driven by `restore()`, which is how a viewer
|
|
+ // changes its own configuration.
|
|
+ changed = Self::_apply_optional(&mut self.group_by_depth, update.group_by_depth) || changed;
|
|
+ changed = Self::_apply_optional(&mut self.split_by_depth, update.split_by_depth) || changed;
|
|
if self.group_rollup_mode == GroupRollupMode::Total && !self.group_by.is_empty() {
|
|
tracing::info!("`total` incompatible with `group_by`");
|
|
changed = true;
|