Let a tree node with a blank group key expand
`partgroup` is null on 93k rows of `gs.osm_skinny` and empty on another 111k, and the pivot showed those groups with no `+`: no way to drill into them at all, while every named sibling opened. The cause is upstream, in the datagrid's `cell_style_row_header`. A tree row's header is the label cell padded on its left by one blank cell per level above it, and it told padding from label by testing the cell's own text — so a node whose group key is blank read as padding, got no `psp-tree-label`, and therefore no caret. The click handler gates on the same class, so the cell was not clickable either. The node was in the traversal and its children existed; only the affordance was missing. Tell them apart structurally instead: the label is the last defined entry in `row_header`, which is how `is_collapse` two lines below already reads the following row. Flat rollup has no padding to distinguish and keeps the text test. Rebuilt the vendored packages from fork branch `expand-blank-tree-nodes`, which stacks this on the depth patch already carried. Note that `npm install` on its own will not pick a rebuilt tarball up — the version in the filename never changes and the lock pins an integrity hash, so npm serves the old one from its cache and nothing reports a problem. The README and the rebuild script now say to re-resolve by path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
aa8ce87560
commit
a95d663076
6
ui/package-lock.json
generated
6
ui/package-lock.json
generated
@ -578,7 +578,7 @@
|
|||||||
"node_modules/@perspective-dev/client": {
|
"node_modules/@perspective-dev/client": {
|
||||||
"version": "5.4.0",
|
"version": "5.4.0",
|
||||||
"resolved": "file:vendor/perspective-dev-client-5.4.0.tgz",
|
"resolved": "file:vendor/perspective-dev-client-5.4.0.tgz",
|
||||||
"integrity": "sha512-DOk5TM4+SpDW29w6wJoHtH6oZVSon9YZjkMfKap4Lmf8SLp2PYwec/LouetkMfg4CvveGJemG4domsl8XeNafA==",
|
"integrity": "sha512-e1rkiCyIlEwSFcEmw/KuNOnrX0k6UiMfnvQB8nwQfkKtoW5d1hkveQ8lAGsj8UZskJuuHrxpWpnKnFm2bwBV7A==",
|
||||||
"license": "Apache-2.0",
|
"license": "Apache-2.0",
|
||||||
"dependencies": {
|
"dependencies": {
|
||||||
"@perspective-dev/server": "^5.4.0",
|
"@perspective-dev/server": "^5.4.0",
|
||||||
@ -596,7 +596,7 @@
|
|||||||
"node_modules/@perspective-dev/viewer": {
|
"node_modules/@perspective-dev/viewer": {
|
||||||
"version": "5.4.0",
|
"version": "5.4.0",
|
||||||
"resolved": "file:vendor/perspective-dev-viewer-5.4.0.tgz",
|
"resolved": "file:vendor/perspective-dev-viewer-5.4.0.tgz",
|
||||||
"integrity": "sha512-2y2xRb5k9BedpnRXcaweRYC2LiP/mblNqbFaRtpqNhnVmp+c3pwO6kx57oRuLhyzkcmNZUAzlqZvs1MJNDn50g==",
|
"integrity": "sha512-dioznGFDoR2fx+0JBdg4SP/WWUxcCeZ9XrKuUiYOgJaXYadegESS0yr1rhMbMNJlG9IQzBCi/WFOvKKzcxkJJA==",
|
||||||
"license": "Apache-2.0",
|
"license": "Apache-2.0",
|
||||||
"dependencies": {
|
"dependencies": {
|
||||||
"@perspective-dev/client": "^5.4.0",
|
"@perspective-dev/client": "^5.4.0",
|
||||||
@ -607,7 +607,7 @@
|
|||||||
"node_modules/@perspective-dev/viewer-datagrid": {
|
"node_modules/@perspective-dev/viewer-datagrid": {
|
||||||
"version": "5.4.0",
|
"version": "5.4.0",
|
||||||
"resolved": "file:vendor/perspective-dev-viewer-datagrid-5.4.0.tgz",
|
"resolved": "file:vendor/perspective-dev-viewer-datagrid-5.4.0.tgz",
|
||||||
"integrity": "sha512-ccJOioKVPAGp/lv7RGSP2rtYj0PfwkJrNZWFMRJkmdz+eAPiHViDZhIHudOuA/2DOmcz6LbD617XaPasTU0Uog==",
|
"integrity": "sha512-J5UL8GNqi5rrmeiOpyGanosSTxRi2bwHNxEHode/DMRP55FEMt+gMVqkC/LfuINi1b0TKVtJQCP+UN8MlnGgpw==",
|
||||||
"license": "Apache-2.0",
|
"license": "Apache-2.0",
|
||||||
"dependencies": {
|
"dependencies": {
|
||||||
"@perspective-dev/client": "^5.4.0",
|
"@perspective-dev/client": "^5.4.0",
|
||||||
|
|||||||
6
ui/vendor/PROVENANCE.txt
vendored
6
ui/vendor/PROVENANCE.txt
vendored
@ -1,8 +1,8 @@
|
|||||||
Built from https://github.com/fleetside72/perspective
|
Built from https://github.com/fleetside72/perspective
|
||||||
branch apply-depth-on-config-update
|
branch expand-blank-tree-nodes
|
||||||
commit 71d1a7508aa86d7e7224ad91e247afe4c201313d
|
commit 40266dc6ec4d83895483124386f0c79938d639ca
|
||||||
based on unknown
|
based on unknown
|
||||||
built 2026-09-17T14:34:21Z on usmidsap02
|
built 2026-09-22T04:06:28Z on usmidsap02
|
||||||
dirty 0 uncommitted file(s) in the source tree at build time
|
dirty 0 uncommitted file(s) in the source tree at build time
|
||||||
|
|
||||||
Regenerate with ui/vendor/rebuild-perspective.sh
|
Regenerate with ui/vendor/rebuild-perspective.sh
|
||||||
|
|||||||
75
ui/vendor/README.md
vendored
75
ui/vendor/README.md
vendored
@ -14,8 +14,9 @@ It buys two things the released packages cannot do at all:
|
|||||||
subtotal while its siblings stay expanded — the Excel behaviour
|
subtotal while its siblings stay expanded — the Excel behaviour
|
||||||
|
|
||||||
**Source:** https://github.com/fleetside72/perspective, branch
|
**Source:** https://github.com/fleetside72/perspective, branch
|
||||||
`column-axis-expand-collapse`. See `PROVENANCE.txt` for the exact commit these
|
`expand-blank-tree-nodes` — the tip of the stack, carrying
|
||||||
tarballs were built from.
|
`column-axis-expand-collapse` and `apply-depth-on-config-update` beneath it.
|
||||||
|
See `PROVENANCE.txt` for the exact commit these tarballs were built from.
|
||||||
|
|
||||||
## Why tarballs and not npm
|
## Why tarballs and not npm
|
||||||
|
|
||||||
@ -32,8 +33,8 @@ exactly the drift that causes trouble.
|
|||||||
## Changing the engine
|
## Changing the engine
|
||||||
|
|
||||||
./rebuild-perspective.sh # builds the fork, repacks, rewrites PROVENANCE.txt
|
./rebuild-perspective.sh # builds the fork, repacks, rewrites PROVENANCE.txt
|
||||||
cd .. && npm install
|
cd .. && npm install ./vendor/perspective-dev-*-5.4.0.tgz # by path — see below
|
||||||
git add vendor && git commit
|
git add vendor package-lock.json && git commit
|
||||||
|
|
||||||
Push the fork first — the script warns if the source tree is dirty, because a
|
Push the fork first — the script warns if the source tree is dirty, because a
|
||||||
tarball built from uncommitted code has no recoverable source.
|
tarball built from uncommitted code has no recoverable source.
|
||||||
@ -42,6 +43,37 @@ The build itself needs cmake >= 3.29.5, protoc >= 22 (its version silently
|
|||||||
selects which protobuf source tree gets cloned), pnpm, and the Rust nightly the
|
selects which protobuf source tree gets cloned), pnpm, and the Rust nightly the
|
||||||
repo pins. Roughly 40 minutes cold. Only ever on a machine changing the engine.
|
repo pins. Roughly 40 minutes cold. Only ever on a machine changing the engine.
|
||||||
|
|
||||||
|
### `npm install` alone does not pick up a rebuilt tarball
|
||||||
|
|
||||||
|
The version in the filename never changes, and `package-lock.json` pins an
|
||||||
|
`integrity` hash per package — so npm satisfies the dependency from its own
|
||||||
|
cache and the freshly built tarball on disk is never read. Nothing fails; the
|
||||||
|
old code is simply still there. Re-resolve by path instead, which recomputes
|
||||||
|
the hashes and rewrites the lock:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd ui
|
||||||
|
rm -rf node_modules/@perspective-dev
|
||||||
|
npm install ./vendor/perspective-dev-{client,server,viewer,viewer-datagrid}-5.4.0.tgz
|
||||||
|
```
|
||||||
|
|
||||||
|
Check the install rather than trusting it — the packages ship their TypeScript
|
||||||
|
sources, so the change is readable without unminifying anything:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
grep -n is_label node_modules/@perspective-dev/viewer-datagrid/src/ts/style_handlers/table_cell/row_header.ts
|
||||||
|
```
|
||||||
|
|
||||||
|
`cargo check` on this patch was clean — the 125 errors it reports without
|
||||||
|
`protoc` installed are unresolved generated protobuf modules, none of them in
|
||||||
|
`view_config.rs`.
|
||||||
|
|
||||||
|
`applyDepth()` in `Forecast.jsx` reads the config back after restoring it and
|
||||||
|
falls back to the imperative `view.set_depth()` when the value did not stick.
|
||||||
|
With the patched engine installed that fallback should no longer run; it is kept
|
||||||
|
for now so an unpatched engine still works, and can be removed once the
|
||||||
|
declarative path is confirmed.
|
||||||
|
|
||||||
## Getting rid of this
|
## Getting rid of this
|
||||||
|
|
||||||
This is a fork, with the maintenance that implies. The exit is upstream taking
|
This is a fork, with the maintenance that implies. The exit is upstream taking
|
||||||
@ -49,6 +81,31 @@ the change — the patch is small and additive, and the engine work is already
|
|||||||
theirs. When a release ships it, delete this directory and put normal version
|
theirs. When a release ships it, delete this directory and put normal version
|
||||||
ranges back in `ui/package.json`.
|
ranges back in `ui/package.json`.
|
||||||
|
|
||||||
|
## Applied: expand a tree node whose group key is blank
|
||||||
|
|
||||||
|
In the vendored tarballs as of the 2026-09-22 build, from branch
|
||||||
|
`expand-blank-tree-nodes` (which is `apply-depth-on-config-update` plus this).
|
||||||
|
Upstream bug, in `packages/viewer-datagrid/src/ts/style_handlers/table_cell/row_header.ts`.
|
||||||
|
|
||||||
|
A tree row's header is the label cell padded on its left by one blank cell per
|
||||||
|
level above it, and `cell_style_row_header` told padding from label by testing
|
||||||
|
the cell's own **text**. A node whose group key is null or `''` therefore read
|
||||||
|
as padding: no `psp-tree-label` class, so no `+` caret — and because
|
||||||
|
`createClickListener` in `event_handlers/header_click.ts` gates expand/collapse
|
||||||
|
on that same class, no way to open the node at all. The node was in the
|
||||||
|
traversal and its children existed; only the affordance was missing. On
|
||||||
|
`gs.osm_skinny` that is `partgroup`, which has 93k null and 111k empty rows.
|
||||||
|
|
||||||
|
The fix tells them apart structurally — the label is the last defined entry in
|
||||||
|
`row_header` — which is how `is_collapse` two lines below already reads the
|
||||||
|
following row. Flat rollup (`group_rollup_mode: 'flat'`) writes each level into
|
||||||
|
its own cell with no padding, so there is nothing structural to test there and
|
||||||
|
it keeps the text test.
|
||||||
|
|
||||||
|
Separate from the data question: null and `''` group as two distinct rows that
|
||||||
|
look identical, so a `COALESCE(NULLIF(col,''), '(none)')` at load time is still
|
||||||
|
worth doing on its own merits.
|
||||||
|
|
||||||
## Applied: depth fields on a config update
|
## Applied: depth fields on a config update
|
||||||
|
|
||||||
`0001-apply-depth-fields-on-config-update.patch` is **in** the vendored
|
`0001-apply-depth-fields-on-config-update.patch` is **in** the vendored
|
||||||
@ -85,16 +142,6 @@ git apply /path/to/pf_app/ui/vendor/0001-apply-depth-fields-on-config-update.pat
|
|||||||
cd ui && npm install
|
cd ui && npm install
|
||||||
```
|
```
|
||||||
|
|
||||||
`cargo check` on this patch was clean — the 125 errors it reports without
|
|
||||||
`protoc` installed are unresolved generated protobuf modules, none of them in
|
|
||||||
`view_config.rs`.
|
|
||||||
|
|
||||||
`applyDepth()` in `Forecast.jsx` reads the config back after restoring it and
|
|
||||||
falls back to the imperative `view.set_depth()` when the value did not stick.
|
|
||||||
With the patched engine installed that fallback should no longer run; it is kept
|
|
||||||
for now so an unpatched engine still works, and can be removed once the
|
|
||||||
declarative path is confirmed.
|
|
||||||
|
|
||||||
### Host tools this tree needs
|
### Host tools this tree needs
|
||||||
|
|
||||||
The repo pins its Rust toolchain (`nightly-2026-06-01`), Emscripten (4.0.9) and
|
The repo pins its Rust toolchain (`nightly-2026-06-01`), Emscripten (4.0.9) and
|
||||||
|
|||||||
BIN
ui/vendor/perspective-dev-client-5.4.0.tgz
vendored
BIN
ui/vendor/perspective-dev-client-5.4.0.tgz
vendored
Binary file not shown.
BIN
ui/vendor/perspective-dev-viewer-5.4.0.tgz
vendored
BIN
ui/vendor/perspective-dev-viewer-5.4.0.tgz
vendored
Binary file not shown.
BIN
ui/vendor/perspective-dev-viewer-datagrid-5.4.0.tgz
vendored
BIN
ui/vendor/perspective-dev-viewer-datagrid-5.4.0.tgz
vendored
Binary file not shown.
11
ui/vendor/rebuild-perspective.sh
vendored
11
ui/vendor/rebuild-perspective.sh
vendored
@ -10,7 +10,7 @@ set -euo pipefail
|
|||||||
# built packages are vendored into this directory as npm tarballs.
|
# built packages are vendored into this directory as npm tarballs.
|
||||||
#
|
#
|
||||||
# Source of truth: https://github.com/fleetside72/perspective
|
# Source of truth: https://github.com/fleetside72/perspective
|
||||||
# branch column-axis-expand-collapse
|
# branch expand-blank-tree-nodes (tip of the patch stack)
|
||||||
#
|
#
|
||||||
# This script exists because vendored binaries are opaque: once the .tgz files
|
# This script exists because vendored binaries are opaque: once the .tgz files
|
||||||
# are committed, nothing in the repo records how to regenerate them. Run this
|
# are committed, nothing in the repo records how to regenerate them. Run this
|
||||||
@ -22,7 +22,7 @@ set -euo pipefail
|
|||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
PSP="${PSP_DIR:-$HOME/perspective}"
|
PSP="${PSP_DIR:-$HOME/perspective}"
|
||||||
BRANCH="${PSP_BRANCH:-column-axis-expand-collapse}"
|
BRANCH="${PSP_BRANCH:-expand-blank-tree-nodes}"
|
||||||
VENDOR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
VENDOR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||||
|
|
||||||
# The four packages that must move together. Perspective's own docs are
|
# The four packages that must move together. Perspective's own docs are
|
||||||
@ -122,6 +122,11 @@ EOF
|
|||||||
echo
|
echo
|
||||||
ls -la "$VENDOR"/*.tgz | awk '{printf " %-52s %5.1f MB\n", $NF, $5/1048576}'
|
ls -la "$VENDOR"/*.tgz | awk '{printf " %-52s %5.1f MB\n", $NF, $5/1048576}'
|
||||||
echo
|
echo
|
||||||
ok "Done. Now: cd ui && npm install && git add vendor && git commit"
|
ok "Done. Now, from ui/ — note the paths: npm resolves these from its cache
|
||||||
|
otherwise, since the version in the filename never changes.
|
||||||
|
|
||||||
|
rm -rf node_modules/@perspective-dev
|
||||||
|
npm install ./vendor/perspective-dev-{client,server,viewer,viewer-datagrid}-5.4.0.tgz
|
||||||
|
git add vendor package-lock.json && git commit"
|
||||||
[[ $dirty -gt 0 ]] && echo -e "\033[1;33m !\033[0m source tree had uncommitted changes — push them to the fork first"
|
[[ $dirty -gt 0 ]] && echo -e "\033[1;33m !\033[0m source tree had uncommitted changes — push them to the fork first"
|
||||||
exit 0
|
exit 0
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user