Compare commits

...

4 Commits

Author SHA1 Message Date
sadpandajoe
206761adfe test(sqllab): assert selected-word override carries border + !important 2026-07-25 22:08:19 +00:00
sadpandajoe
48aba96326 test(sqllab): fix tsc failure in selected-word marker test
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>
2026-07-25 05:03:18 +00:00
sadpandajoe
156aaf3539 test(sqllab): guard selected-word marker wiring; dedupe selection color
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>
2026-07-25 04:27:35 +00:00
sadpandajoe
5c37d178f8 fix(sqllab): make dark-theme occurrence highlight readable
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>
2026-07-23 23:38:19 +00:00
2 changed files with 106 additions and 4 deletions

View File

@@ -18,7 +18,11 @@
*/
import { createRef } from 'react';
import { render, screen, waitFor } from '@superset-ui/core/spec';
import { supersetTheme } from '@apache-superset/core/theme';
import {
supersetTheme,
ThemeProvider,
type SupersetTheme,
} from '@apache-superset/core/theme';
import type AceEditor from 'react-ace';
import {
AsyncAceEditor,
@@ -30,6 +34,7 @@ import {
JsonEditor,
ConfigEditor,
aceCompletionHighlightStyles,
aceSelectedWordStyles,
} from '.';
import type { AceModule, AsyncAceEditorOptions } from './types';
@@ -55,6 +60,81 @@ test('themes the autocomplete completion highlight from the theme', () => {
expect(styles).toContain(supersetTheme.colorPrimaryText);
});
test('themes the selected-word occurrence markers from the theme', () => {
// The light `github` theme hardcodes a near-white box for the markers Ace
// paints on every other occurrence of the selected token, which makes the
// recolored token glyphs unreadable in dark mode. The shared editor overrides
// the marker to reuse the dark-aware selection color so it stays legible.
const { styles } = aceSelectedWordStyles(supersetTheme);
expect(styles).toContain('.ace_selected-word');
// The default theme leaves `colorEditorSelection` unset, so the marker uses
// the documented `colorPrimaryBgHover` fallback.
expect(styles).toContain(supersetTheme.colorPrimaryBgHover);
// The override also restyles the marker border from the theme...
expect(styles).toContain(supersetTheme.colorBorder);
// ...and carries `!important`, which is what lets it win over Ace's bundled
// non-important `.ace-github .ace_marker-layer .ace_selected-word` near-white
// rule regardless of stylesheet order (the effective-cascade guarantee that
// jsdom cannot verify by computed style).
expect(styles).toContain('!important');
});
// Collect every CSS rule the render injected: emotion's Global styles land in
// <style> tags (non-speedy in test) while Ace's bundled theme uses the CSSOM,
// so read both to reliably observe what actually reached the document.
function getInjectedCss(): string {
const fromTags = Array.from(document.querySelectorAll('style'))
.map(tag => tag.textContent ?? '')
.join('\n');
const fromSheets = Array.from(document.styleSheets)
.map(sheet => {
try {
return Array.from(sheet.cssRules)
.map(rule => rule.cssText)
.join('\n');
} catch {
return '';
}
})
.join('\n');
return `${fromTags}\n${fromSheets}`;
}
test('wires the dark-aware selected-word marker into the editor Global styles', async () => {
// Guards the actual regression: Ace's bundled `github` theme already injects
// a near-white `.ace-github ... .ace_selected-word` rule, so the fix is only
// effective if the editor's own Global block also emits a `.ace_selected-word`
// override driven by the theme. Render with a distinctive `colorEditorSelection`
// and assert that value reaches the injected selected-word rule — this fails if
// the `${aceSelectedWordStyles(token)}` interpolation is dropped from <Global>.
const markerColor = '#010203';
const darkTheme: SupersetTheme = {
...supersetTheme,
colorEditorSelection: markerColor,
};
// The shared `render` helper injects its own default `SupersetThemeProvider`,
// so nest the dark theme around the editor itself: the innermost emotion
// `ThemeProvider` is what the editor's `useTheme()` resolves.
const { container } = render(
<ThemeProvider theme={darkTheme}>
<SQLEditor />
</ThemeProvider>,
);
await waitFor(() => {
expect(container.querySelector('[id="ace-editor"]')).toBeInTheDocument();
});
const css = getInjectedCss();
// The dark-aware color is tied specifically to the selected-word marker
// (not just the plain `.ace_selection`), so the occurrence markers inherit it.
expect(css).toMatch(
new RegExp(`\\.ace_selected-word[^}]*${markerColor}`, 'i'),
);
});
test('SQLEditor uses fontFamilyCode from theme', async () => {
const ref = createRef<AceEditor>();
const { container } = render(<SQLEditor ref={ref as React.Ref<never>} />);

View File

@@ -113,6 +113,14 @@ export type AsyncAceEditorOptions = {
> | null;
};
/**
* Dark-aware color for text selection in the editor. Shared so the plain
* selection and the "selected word" occurrence markers always match, falling
* back to `colorPrimaryBgHover` on themes that don't define the token.
*/
export const editorSelectionColor = (token: SupersetTheme) =>
token.colorEditorSelection ?? token.colorPrimaryBgHover;
/**
* Theme-aware styling for the matched-prefix highlight in the autocomplete
* popup. Ace ships a hardcoded `color: #000` that is invisible on the dark
@@ -126,6 +134,20 @@ export const aceCompletionHighlightStyles = (token: SupersetTheme) => css`
}
`;
/**
* Theme-aware styling for the "selected word" markers Ace paints on every other
* occurrence of the currently selected token. The light `github` theme hardcodes
* a near-white box (`rgb(250,250,255)`); in dark mode the recolored token glyphs
* sit on it unreadably. Reuse the same dark-aware selection color as
* `.ace_selection` so the markers stay legible in every theme.
*/
export const aceSelectedWordStyles = (token: SupersetTheme) => css`
.ace_editor .ace_marker-layer .ace_selected-word {
background-color: ${editorSelectionColor(token)} !important;
border: 1px solid ${token.colorBorder} !important;
}
`;
/**
* Get an async AceEditor with automatical loading of specified ace modules.
*/
@@ -389,11 +411,11 @@ export function AsyncAceEditor(
}
/* Adjust selection color */
.ace_editor .ace_selection {
background-color: ${
token.colorEditorSelection ?? token.colorPrimaryBgHover
} !important;
background-color: ${editorSelectionColor(token)} !important;
}
${aceSelectedWordStyles(token)}
/* Improve active line highlighting */
.ace_editor .ace_active-line {
background-color: ${token.colorPrimaryBg} !important;