fix(sqllab): make dark-theme occurrence highlight readable#42403
Draft
sadpandajoe wants to merge 4 commits into
Draft
fix(sqllab): make dark-theme occurrence highlight readable#42403sadpandajoe wants to merge 4 commits into
sadpandajoe wants to merge 4 commits into
Conversation
The Ace editor uses the light github theme with token-based CSS overrides for dark mode, but .ace_selected-word (the marker on every other occurrence of a selected token) was left at the theme default near-white box. In dark mode the recolored token glyphs became unreadable on it. Override .ace_selected-word to reuse the dark-aware colorEditorSelection token, mirroring the existing .ace_selection and completion-highlight overrides. Light theme is unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
sadpandajoe
added
the
🎪 ⚡ showtime-trigger-start
github-actions
Bot
added
🎪 5c37d17 🚦 building
🎪 ⌛ 48h
Contributor
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #42403 +/- ##
==========================================
- Coverage 65.25% 65.25% -0.01%
==========================================
Files 2794 2794
Lines 157396 157398 +2
Branches 35982 35982
==========================================
- Hits 102706 102703 -3
- Misses 52713 52718 +5
Partials 1977 1977
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
🎪 Showtime deployed environment on GHA for 5c37d17 • Environment: http://35.87.156.37:8080 (admin/admin) |
Address review feedback on the dark-theme occurrence-highlight fix: - Replace the vacuous helper-only assertion with an integration test that renders the editor under a distinctive colorEditorSelection and asserts the value reaches the injected .ace_selected-word rule. Ace's bundled github theme already emits its own near-white .ace_selected-word rule, so a selector-only check would pass even if the Global interpolation were dropped; tying the assertion to the theme color makes it a real regression guard. - Assert the concrete colorPrimaryBgHover fallback in the helper test instead of re-deriving the value under test. - Extract a shared editorSelectionColor helper so .ace_selection and .ace_selected-word cannot drift apart. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Contributor
|
🎪 Showtime deployed environment on GHA for 156aaf3 • Environment: http://44.249.177.195:8080 (admin/admin) |
The shared `render` helper from `@superset-ui/core/spec` omits `wrapper` from its options (it always supplies its own SupersetThemeProvider), so passing a custom `wrapper` tripped TS2353 and failed the lint-frontend tsc step. Nest the dark theme around the editor element instead; the innermost emotion ThemeProvider is what the editor's useTheme() resolves, so the assertion still exercises the dark-aware marker color. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
github-actions
Bot
added
🎪 48aba96 🚦 building
🎪 48aba96 📅 2026-07-25T05-07
Contributor
github-actions
Bot
added
🎪 48aba96 🚦 deploying
🎪 48aba96 🚦 running
Contributor
|
🎪 Showtime deployed environment on GHA for 48aba96 • Environment: http://54.186.2.19:8080 (admin/admin) |
github-actions
Bot
added
🎪 206761a 🚦 building
Contributor
github-actions
Bot
added
🎪 206761a 🚦 deploying
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.
SUMMARY
Fixes SQL Lab dark theme so that highlighting a token readably highlights its other occurrences (sc-102561).
Root cause: SQL Lab's Ace editor uses the light
githubtheme, with a token-based dark-mode<Global>CSS override block inAsyncAceEditor/index.tsx. That block overrode.ace_selectionetc. but not.ace_selected-word(the marker Ace paints on every other occurrence of the selected token), so it kept the light theme's near-white default and the recolored dark-mode glyphs became unreadable on it. This adds a token-driven.ace_selected-wordoverride (dark-awarecolorEditorSelection, falling back tocolorPrimaryBgHover, pluscolorBorder) carrying!importantso it wins over Ace's bundled rule, mirroring the existingaceCompletionHighlightStylespattern (#41005). The.ace_selectioncolor helper was deduped in the process with no behavior change; light theme is unaffected.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before (master): in dark mode, double-clicking a repeated column name leaves the other occurrences near-invisible.
After (this PR): the other occurrences are highlighted with readable contrast; light theme is unchanged.
Before/after videos: pending capture on the showtime env; will be attached here and on the Shortcut story before this PR leaves draft.
TESTING INSTRUCTIONS
Unit tests assert the helper emits the dark-aware color, border, and
!important, and that the override is wired into the editor's Global styles:AsyncAceEditor.test.tsx.ADDITIONAL INFORMATION