Skip to content

fix: keep query editor toolbar visible with large queries (#173)#176

Merged
eugene-mobile merged 1 commit into
mainfrom
fix/big-query-toolbar-scroll
Jun 13, 2026
Merged

fix: keep query editor toolbar visible with large queries (#173)#176
eugene-mobile merged 1 commit into
mainfrom
fix/big-query-toolbar-scroll

Conversation

@eugene-mobile

Copy link
Copy Markdown
Contributor

Problem

Closes #173.

When a query is tall, the editor toolbar (Run / Explain / Format buttons) scrolls out of view, so users can't trigger actions while at the bottom of a large query.

issue

Root cause

QueryEditor's root container is a flex column but had no height constraint. Its parent in App.tsx is a fixed flex: 0 0 240px box with overflow: hidden. Because the root defaulted to flex: 0 1 auto (and flex items have min-height: auto, which prevents shrinking below content), a tall query grew the whole editor past 240px. Focusing the textarea near the bottom then made the browser scroll the overflow:hidden parent to reveal the caret — dragging the pinned toolbar off the top of the viewport. The textarea's own overflowY: auto never engaged because the textarea was never height-bounded.

Fix

  • Add flex: 1; min-height: 0 to the editor root so it fills its fixed-height parent. The bounded editorWrap now constrains the textarea, whose existing overflowY: auto scrolls the query internally while the toolbar (flex-shrink: 0) stays pinned and always reachable.
  • Sync the line-number gutter's scrollTop to the textarea on scroll. Previously the gutter and text scrolled together as one unit via the parent; now that the textarea scrolls on its own, this keeps line numbers aligned with the text.

Testing

  • tsc --noEmit passes; no new lint errors in the edited file; all 76 unit tests pass.
  • Manual: paste a long query, scroll to the bottom — the Run/Explain toolbar stays fixed at the top and line numbers remain aligned.

🤖 Generated with Claude Code

The QueryEditor root had no height constraint, so a tall query grew the
editor past its fixed-height parent. Focusing the textarea near the bottom
then scrolled the overflow:hidden parent to reveal the caret, dragging the
Run/Explain/Format toolbar out of view.

Constrain the root with `flex: 1; min-height: 0` so the editor fills its
parent and the textarea scrolls internally (via its existing overflowY:auto)
while the toolbar stays pinned. Sync the line-number gutter's scrollTop to
the textarea so line numbers stay aligned now that the textarea scrolls on
its own instead of the whole editor moving as a unit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@eugene-mobile

Copy link
Copy Markdown
Contributor Author

@claude review this PR

@claude

claude Bot commented Jun 13, 2026

Copy link
Copy Markdown

Claude finished @eugene-mobile's task in 1m 13s —— View job


PR Review

  • Read CLAUDE.md for repo guidelines (no CLAUDE.md found — using general standards)
  • Read changed file and git diff
  • Analyze the fix for correctness
  • Post review feedback

Overall: This is a clean, correct, minimal fix. The root cause analysis is accurate and the solution is idiomatic.

Root Fix — src/components/QueryEditor.tsx:496

- root: { display: 'flex', flexDirection: 'column', ... }
+ root: { flex: 1, minHeight: 0, display: 'flex', flexDirection: 'column', ... }

flex: 1; minHeight: 0 is exactly the right CSS pattern here. The parent in App.tsx:575 is flex: '0 0 240px' with overflow: hidden — by default flex items have min-height: auto, meaning a tall query would push the editor to grow past 240px, causing the browser to scroll the hidden overflow to follow the caret and drag the toolbar offscreen. Setting minHeight: 0 removes that implicit lower bound, letting the flex layout contain the editor properly. No issues here.

Line-Number Scroll Sync — src/components/QueryEditor.tsx:933-937

onScroll={e => {
  if (lineNumsRef.current) lineNumsRef.current.scrollTop = e.currentTarget.scrollTop;
}}

Correct approach. The gutter has overflowY: 'hidden' (line 503), so no scrollbar is shown but scrollTop is still writable, keeping line numbers aligned. Directly mutating the DOM in the scroll handler avoids a React re-render on every scroll event — good.

One observation: The if (lineNumsRef.current) guard is technically unnecessary — the scroll handler can only fire while the component is mounted, and lineNumsRef is attached to a sibling element in the same render tree, so it will always be non-null here. It's harmless though, just slightly defensive.

No Concerns With

  • Scope: The diff is 10 lines; no unrelated changes.
  • Correctness: editorWrap already has flex: 1; overflow: hidden; minHeight: 0 (line 502), so the textarea's overflowY: auto correctly engages once the root is bounded.
  • Toolbar pinning: toolbar already has flexShrink: 0 (line 497), so it stays at the top of the bounded root.
  • Tests: PR notes tsc --noEmit and 76 unit tests pass.

Verdict: LGTM. Ready to merge.

@eugene-mobile eugene-mobile merged commit 424056d into main Jun 13, 2026
1 check passed
@eugene-mobile eugene-mobile deleted the fix/big-query-toolbar-scroll branch June 13, 2026 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect scrolling with big queries

1 participant