Paul Trowbridge
41e76978f4
The class fix rendered the `+` but the node still would not open, and the
caret did not light on hover. `tbody th:empty { pointer-events: none }`
makes the padding cells left of a tree label inert, and a label cell whose
group key is null or "" is `:empty` too — its caret is a `::before`, which
does not count as content — so the cell received no pointer events at all.
Exclude the tree classes from that rule. `row-hover.css` reads `:empty` the
same way in six border selectors; those are a hover tint, left alone.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
180 lines
8.5 KiB
Markdown
180 lines
8.5 KiB
Markdown
# Vendored Perspective
|
|
|
|
pf_app runs a **patched build of Perspective**. Upstream's C++ engine has always
|
|
implemented column-axis expand/collapse — `t_ctx2::set_depth(HEADER_COLUMN, …)`
|
|
and `open`/`close(HEADER_COLUMN, idx)` are fully written — but nothing above C++
|
|
could reach it: `set_column_pivot_depth()` was never called, and
|
|
`View<t_ctx2>::expand/collapse` hardcoded `HEADER_ROW`. The patch is wiring, not
|
|
new engine logic.
|
|
|
|
It buys two things the released packages cannot do at all:
|
|
|
|
- `split_by_depth` in `ViewConfig`, the `split_by` counterpart to `group_by_depth`
|
|
- `expand_column()` / `collapse_column()`, so one column branch can fold to its
|
|
subtotal while its siblings stay expanded — the Excel behaviour
|
|
|
|
**Source:** https://github.com/fleetside72/perspective, branch
|
|
`expand-blank-tree-nodes` — the tip of the stack, carrying
|
|
`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
|
|
|
|
The feature is not released upstream. Until it is, the four packages are built
|
|
from the fork and committed here as npm tarballs. `npm install` expands them
|
|
exactly as it expands anything from the registry — no special tooling, and
|
|
`pf.sh deploy` works unchanged. A deploy machine needs node and nothing else:
|
|
no emscripten, no cmake, no protoc, no Rust.
|
|
|
|
All four move together, never a subset. Perspective couples loader, package
|
|
versions, data format and `apache-arrow`; vendoring a partial set reintroduces
|
|
exactly the drift that causes trouble.
|
|
|
|
## Changing the engine
|
|
|
|
./rebuild-perspective.sh # builds the fork, repacks, rewrites PROVENANCE.txt
|
|
cd .. && npm install ./vendor/perspective-dev-*-5.4.0.tgz # by path — see below
|
|
git add vendor package-lock.json && git commit
|
|
|
|
Push the fork first — the script warns if the source tree is dirty, because a
|
|
tarball built from uncommitted code has no recoverable source.
|
|
|
|
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
|
|
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
|
|
|
|
This is a fork, with the maintenance that implies. The exit is upstream taking
|
|
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
|
|
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.
|
|
|
|
The class was only half of it. `tbody th:empty { pointer-events: none }` in
|
|
`regular_table.css` makes the padding cells inert, and a blank label cell is
|
|
`:empty` as well — a `::before` caret is not content — so the fixed node drew
|
|
its `+`, then ignored the click and would not even take the hover. The rule now
|
|
excludes the tree classes, which is the distinction it was reaching for: a
|
|
padding cell never carries one.
|
|
|
|
`row-hover.css` uses `:empty` the same way in six border selectors. Those are a
|
|
hover tint and are left alone; a row whose first level is blank loses a border
|
|
highlight, and nothing else.
|
|
|
|
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
|
|
|
|
`0001-apply-depth-fields-on-config-update.patch` is **in** the vendored
|
|
tarballs as of the 2026-09-17 build, from branch
|
|
`apply-depth-on-config-update`. It fixes an upstream bug in
|
|
`rust/perspective-client/src/rust/config/view_config.rs`:
|
|
`ViewConfig::apply_update` applies ten fields and **neither `group_by_depth`
|
|
nor `split_by_depth` is among them**. So a depth can be set when a view is
|
|
created (`table.view({ group_by_depth: 1 })` — which is what the fork's own
|
|
`depth_test.mjs` exercises) but never through `restore()`, which is how a
|
|
viewer changes its own configuration. The field arrives, deserializes, and is
|
|
discarded before the engine sees it.
|
|
|
|
`group_by_depth` and the omission are both upstream; the fork mirrored
|
|
`split_by_depth` alongside it faithfully, including the omission.
|
|
|
|
Note the semantics, from `server.cpp`:
|
|
|
|
```cpp
|
|
ctx1->set_depth(row_pivot_depth - 1); // one-sided
|
|
ctx2->set_depth(t_header::HEADER_ROW, row_pivot_depth - 1); // two-sided
|
|
```
|
|
|
|
The config field counts **levels to show**; `view.set_depth()` counts the
|
|
boundary below them. So `group_by_depth: n` equals `set_depth(n - 1)`, and
|
|
`Forecast.jsx` sends `d + 1`.
|
|
|
|
To apply:
|
|
|
|
```bash
|
|
cd $PSP_DIR # default ~/perspective
|
|
git apply /path/to/pf_app/ui/vendor/0001-apply-depth-fields-on-config-update.patch
|
|
./ui/vendor/rebuild-perspective.sh
|
|
cd ui && npm install
|
|
```
|
|
|
|
### Host tools this tree needs
|
|
|
|
The repo pins its Rust toolchain (`nightly-2026-06-01`), Emscripten (4.0.9) and
|
|
Binaryen (132) exactly, and pins **nothing** for the host tools — no
|
|
`packageManager`, no `.nvmrc`. So `npm i -g pnpm` and `pip install cmake` both
|
|
give versions this tree was not written against. The script's preflight now
|
|
rejects them, but for the record:
|
|
|
|
- **pnpm 10.x.** 11+ rejects a repeated `--if-present`, which
|
|
`sh_perspective.mjs:189` emits once per package in scope.
|
|
- **cmake 3.x**, not 4. Both satisfy the `>= 3.29.5` floor; CMake 4 removed
|
|
support for `cmake_minimum_required < 3.5`, which the C++ dependencies still
|
|
declare, and fails partway through the Arrow build.
|
|
- **protoc >= 22** (33.2 known good).
|
|
- **`pnpm pack`, not `npm pack`.** These packages depend on each other as
|
|
`workspace:^`, and only pnpm rewrites that to a concrete version. An
|
|
`npm pack` tarball is rejected at install: `Unsupported URL Type
|
|
"workspace:"`.
|
|
|
|
Two commits on the build branch are local build concessions rather than
|
|
fixes, and should be dropped before the real patch goes anywhere upstream:
|
|
removing the `postinstall:playwright` step (it runs `playwright install
|
|
--with-deps`, which apt-installs system libraries and so needs root), and
|
|
filling in the `allowBuilds` block pnpm 12 demanded.
|