Skip to content

Conversation

@pulpdrew
Copy link
Contributor

@pulpdrew pulpdrew commented Dec 5, 2025

Closes HDX-2865
Closes HDX-2980
Closes #1443

Summary

This PR ensures that the schema preview / SQL query preview modal does not have a restrictive max height.

Previously, the CodeMirror component would sometimes have a max height of 150px applied. It appears that this was coming from the global cm-editor and cm-scroller classes, which were conditionally updated with max height properties in SQLInlineEditor.tsx for multi-line inputs.

To fix this, the SQLInlineEditor multiline support now makes use of separate CSS selectors to only apply the max height to inputs with allowMultiline enabled.

Demo

Modals are now >150px reliably, and multi-line editors are still capped to 150px max height

Screen.Recording.2025-12-05.at.9.20.12.AM.mov

@changeset-bot
Copy link

changeset-bot bot commented Dec 5, 2025

🦋 Changeset detected

Latest commit: d8b3420

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Dec 5, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview Comment Dec 5, 2025 2:28pm

@claude
Copy link

claude bot commented Dec 5, 2025

PR Review

No critical issues found.

The fix correctly addresses the modal height restriction issue:

What changed:

  • Before: createStyleTheme() conditionally applied maxHeight to all CodeMirror editors when allowMultiline=true
  • After: Uses scoped CSS selectors (.cm-editor-multiline &.cm-editor) to only apply height restrictions when the parent has the cm-editor-multiline class

Why this works:

  1. SQL/Schema preview modals use bare CodeMirror components (see ChartSQLPreview.tsx:68-80) without SQLInlineEditor wrapper
  2. The old approach bled global styles to all .cm-editor and .cm-scroller elements
  3. New approach uses descendant selectors that only match when parent has cm-editor-multiline class
  4. Only SQLInlineEditor with allowMultiline={true} applies the wrapper class (line 448)

Tested scenarios:

✓ Multi-line editors (allowMultiline=true) maintain 150px max height
✓ Modal editors (no wrapper) are unrestricted
✓ Single-line editors (default) remain unrestricted

The CSS specificity is correct - the descendant selector .cm-editor-multiline & .cm-scroller properly targets only children of elements with the class.

@pulpdrew pulpdrew force-pushed the drew/fix-sql-preview-height branch from 051b5e3 to d8b3420 Compare December 5, 2025 14:26
@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2025

E2E Test Results

All tests passed • 44 passed • 3 skipped • 361s

Status Count
✅ Passed 44
❌ Failed 0
⚠️ Flaky 2
⏭️ Skipped 3

View full report →

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.

Schema Page should open up to show default schema atleast.

2 participants