fix(results): vertical scroll + reliable ⌘A in the raw output pane#2
Merged
Merged
Conversation
Two bugs in the TSV/JSON output pane: - Vertical scroll didn't work (only horizontal). .raw-text-view/.json-view used `flex: 1`, but their parent .res-body is a relatively-positioned block, not a flex container — so flex had no effect and the pane grew to content height with no scroll (the parent just clipped it). Use `height: 100%` + overflow:auto like .res-table-wrap, so both axes scroll. - ⌘A didn't select the pane on macOS. Detection required the pane to be focused (e.target.closest), but macOS WebKit doesn't focus a tabindex <div> on click, so e.target stayed <body>. Re-key off "not editing + a raw pane is on screen" (document.querySelector) instead of focus; a focused editor/input still gets the native select-all (whole query). Verified in-browser against the built bundle: tall pane scrolls vertically; ⌘A fired from <body> selects the entire pane text. 333 tests pass; shortcuts.js + results.js at 100%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QennTvGKAtJZrv9EpQagef
Root cause of the "16 rows don't scroll, 2000 do" report. `html { zoom: 1.2 }`
combined with `#root { height: 100vh }`: `vh` units ignore `zoom`, so 100vh
(= viewport px) is then scaled ×1.2 and #root renders ~120vh tall. The whole
shell overflowed the window by ~20% (measured: 970px tall in an 808px viewport),
pushing the results pane's lower portion below the fold — and html is
overflow:hidden, so the page can't scroll to reach it. With a tall result the
inner pane's own scrollbar bridged the gap (so 2000 rows "worked"); with a short
result the off-screen rows were simply unreachable.
Use `height: 100%` (cascades html→body→#root, and % scales with zoom) so the
shell is bounded to the viewport. Verified in-browser: #root/main-row/res-body
all bottom-out exactly at the viewport; the 16-row table sits fully on screen
and its pane scrolls. 333 tests pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QennTvGKAtJZrv9EpQagef
- Header: rename "Sign out" → "Log Out" and add a GitHub-logo link (new Icon.github, opens the repo in a new tab, rel=noopener). svgFilled gains optional viewBox args so the 24×24 octocat path renders at 15px. - History panel: remove the "Clear history" row (looked out of place); per-row delete is enough. clearHistory() stays as a tested state op, just unused in UI. Verified in-browser: header shows "Log Out" + GitHub icon; history lists rows with per-row delete, no clear row. 334 tests pass; icons/saved-history at 100%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QennTvGKAtJZrv9EpQagef
BorisTyshkevich
added a commit
that referenced
this pull request
Jun 23, 2026
…t, qualified fallback, doc-cache, single tokenize (#26, #27) Five findings from a follow-up manual review; verified live on otel. Correctness: - Hover docs now resolve the hovered word from the string/comment-masked text (host.maskedValue()), so hovering a function/keyword inside a string or comment no longer pops a phantom doc card — consistent with signature help (#1). - Signature help strips the optional-param brackets ClickHouse uses (`name(a, b[, c])`) before splitting on commas, so args render cleanly and the active-arg highlight aligns (was showing `offset[` / `length]` for substring) (#2). - completionContext only flags `qualified` when a real identifier precedes the dot; a bare dot (`.col`, `count().c`) now falls back to normal completion instead of an empty dropdown (#4). - loadEntityDoc returns null on a query FAILURE vs '' for genuinely-no-doc, and entityDoc caches the latter but drops the former — a transient error no longer permanently suppresses a function's hover/footer doc for the session (#8). Performance: - The keystroke path tokenizes the buffer ONCE and shares the token list between the syntax highlighter and the literal mask, instead of two full passes per keystroke. maskFromTokens()/renderTokensInto() consume a token list; the editor memoizes it by (text, refData) so it re-tokenizes when server keyword/func sets arrive after connect (#5). Tests cover each: hover-in-literal, bracketed-param signature split, bare-dot fallback, failed-doc retry, and re-highlight-on-refData-change (the shared-token cache invalidation). All gated layers stay 100/100/100/100 (ui glue within its functions>=95 / branches>=90 floor). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGBS74oUsXarGkCRQKEFLu
BorisTyshkevich
added a commit
that referenced
this pull request
Jun 25, 2026
- run(): a typed EXPLAIN now honors exactly what was typed — a plain EXPLAIN always opens the verbatim Explain view instead of inheriting a stale rich view from a previous run/tab. Drop the unused app.state.explainView. (review #1) - dot.parseDot: blank quoted label strings before edge scanning and only keep edges between declared processors, so a `->` inside a label (e.g. a lambda `x -> x + 1`) no longer fabricates phantom nodes/edges. (review #2) - panzoom.zoomBox: guard a degenerate zero-size viewBox (defensive). - overlay: Esc stops propagation so it doesn't also reach the app key handler. - remove now-dead isExplain from core/format.js (superseded by parseExplain). - document the ontime DOT fixture's capture provenance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGBS74oUsXarGkCRQKEFLu
BorisTyshkevich
added a commit
that referenced
this pull request
Jun 25, 2026
…line/Estimate) + Explain button (#37) * feat(explain): EXPLAIN result views (Explain/Indexes/Projections/Pipeline/Estimate) + Explain button Replace the one-dimensional raw EXPLAIN dump with five views in the data pane: - Explain (default) runs the user's EXPLAIN verbatim as clean TabSeparatedRaw, so arbitrary/complex parameters are honored. - Indexes / Projections derive `indexes=1` / `projections=1` from the inner query. - Pipeline derives `EXPLAIN PIPELINE graph=1` and renders the Graphviz DOT as a zero-dep SVG layered graph (pure parse+layout in src/core/dot.js). - Estimate derives `EXPLAIN ESTIMATE` and renders structured rows as a table. On Run we auto-select a rich view only when the typed statement is exactly that canonical form; anything else stays on the verbatim Explain tab. Switching a view re-runs the derived query and never edits the editor SQL. New "Explain" toolbar button (between Format and Save) explains any query without editing it. No new runtime dependency. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGBS74oUsXarGkCRQKEFLu * docs(explain): README EXPLAIN-views section; neutral empty-Estimate message Reword the empty-Estimate placeholder (a trivial count() is answered from metadata, so ESTIMATE returns no part rows even on a MergeTree table). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGBS74oUsXarGkCRQKEFLu * feat(explain): vertical pipeline graph, drop EXPLAIN header stats, e2e graph test - Pipeline graph now lays out top→bottom: sequential stages stack vertically, parallel processors in a stage sit side-by-side horizontally. Edges flow bottom→top-centre; self-loops are filtered. - EXPLAIN views drop the ms/rows/bytes header stats (not meaningful for a plan) to give the five tabs room. - Add a Playwright e2e test that renders a real `EXPLAIN PIPELINE graph = 1` capture from the antalya ontime dataset (fact/dim join + aggregated subquery join, 37 processors / 40 edges) and asserts the vertical-with-parallel-lanes SVG. Fixture: tests/e2e/fixtures/ontime-pipeline-graph.dot. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGBS74oUsXarGkCRQKEFLu * feat(explain): fullscreen pan/zoom overlay for the pipeline graph Add an Expand button (results header, Pipeline view) that opens the graph in a fullscreen overlay with wheel-zoom around the cursor, drag-pan, +/- and Fit-to-screen controls, and Esc/✕/backdrop close. Makes complex pipelines (which the current layout draws wide) navigable; better automatic layout is deferred to a follow-up PR. - src/core/panzoom.js: pure viewBox algebra (fit/zoom/pan), 100% covered. - src/ui/explain-graph.js: extract buildPipelineSvg; openPipelineFullscreen. - src/ui/results.js: Expand button; icons.js: expand + minus glyphs. - e2e: pipeline.html exposes __openFullscreen; spec drives real wheel/drag/Esc. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGBS74oUsXarGkCRQKEFLu * fix(explain): address code-review findings - run(): a typed EXPLAIN now honors exactly what was typed — a plain EXPLAIN always opens the verbatim Explain view instead of inheriting a stale rich view from a previous run/tab. Drop the unused app.state.explainView. (review #1) - dot.parseDot: blank quoted label strings before edge scanning and only keep edges between declared processors, so a `->` inside a label (e.g. a lambda `x -> x + 1`) no longer fabricates phantom nodes/edges. (review #2) - panzoom.zoomBox: guard a degenerate zero-size viewBox (defensive). - overlay: Esc stops propagation so it doesn't also reach the app key handler. - remove now-dead isExplain from core/format.js (superseded by parseExplain). - document the ontime DOT fixture's capture provenance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QGBS74oUsXarGkCRQKEFLu --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two bugs reported in the data output pane (TSV/JSON raw view), both fixed and verified in-browser against the built bundle.
1. Vertical scroll didn't work (only horizontal)
.raw-text-view/.json-viewusedflex: 1, but their parent.res-bodyis a relatively-positioned block, not a flex container — soflexhad no effect, the pane grew to its content height, and.res-body'soverflow: hiddenjust clipped it. Horizontal worked because width was bounded. Fix:height: 100%+overflow: auto(same as.res-table-wrap), so both axes scroll.2. ⌘A didn't select the pane on macOS
Detection required the pane to be focused (
e.target.closest(...)), but macOS WebKit doesn't focus atabindex<div>on click — soe.targetstays<body>and the handler bailed. Re-keyed off "not editing + a raw pane is on screen" (document.querySelector) rather than focus. A focused editor/input still falls through to the native select-all (whole query), so ⌘A in the SQL editor is unchanged.Verification (real browser, bundled code)
overflow-y: auto,scrollTopset and took → vertical scroll works; horizontal still works.KeyboardEventfrom<body>(pane not focused): selection equals the full pane text (exact match).npm test→ 333 pass;shortcuts.jsandresults.jsat 100% coverage.🤖 Generated with Claude Code
https://claude.ai/code/session_01QennTvGKAtJZrv9EpQagef