Skip to content

Commit 156aaf3

Browse files
sadpandajoeclaude
andcommitted
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>
1 parent 5c37d17 commit 156aaf3

2 files changed

Lines changed: 70 additions & 10 deletions

File tree

superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/AsyncAceEditor.test.tsx

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,13 @@
1616
* specific language governing permissions and limitations
1717
* under the License.
1818
*/
19-
import { createRef } from 'react';
19+
import { createRef, type ReactNode } from 'react';
2020
import { render, screen, waitFor } from '@superset-ui/core/spec';
21-
import { supersetTheme } from '@apache-superset/core/theme';
21+
import {
22+
supersetTheme,
23+
ThemeProvider,
24+
type SupersetTheme,
25+
} from '@apache-superset/core/theme';
2226
import type AceEditor from 'react-ace';
2327
import {
2428
AsyncAceEditor,
@@ -64,8 +68,60 @@ test('themes the selected-word occurrence markers from the theme', () => {
6468
const { styles } = aceSelectedWordStyles(supersetTheme);
6569

6670
expect(styles).toContain('.ace_selected-word');
67-
expect(styles).toContain(
68-
supersetTheme.colorEditorSelection ?? supersetTheme.colorPrimaryBgHover,
71+
// The default theme leaves `colorEditorSelection` unset, so the marker uses
72+
// the documented `colorPrimaryBgHover` fallback.
73+
expect(styles).toContain(supersetTheme.colorPrimaryBgHover);
74+
});
75+
76+
// Collect every CSS rule the render injected: emotion's Global styles land in
77+
// <style> tags (non-speedy in test) while Ace's bundled theme uses the CSSOM,
78+
// so read both to reliably observe what actually reached the document.
79+
function getInjectedCss(): string {
80+
const fromTags = Array.from(document.querySelectorAll('style'))
81+
.map(tag => tag.textContent ?? '')
82+
.join('\n');
83+
const fromSheets = Array.from(document.styleSheets)
84+
.map(sheet => {
85+
try {
86+
return Array.from(sheet.cssRules)
87+
.map(rule => rule.cssText)
88+
.join('\n');
89+
} catch {
90+
return '';
91+
}
92+
})
93+
.join('\n');
94+
return `${fromTags}\n${fromSheets}`;
95+
}
96+
97+
test('wires the dark-aware selected-word marker into the editor Global styles', async () => {
98+
// Guards the actual regression: Ace's bundled `github` theme already injects
99+
// a near-white `.ace-github ... .ace_selected-word` rule, so the fix is only
100+
// effective if the editor's own Global block also emits a `.ace_selected-word`
101+
// override driven by the theme. Render with a distinctive `colorEditorSelection`
102+
// and assert that value reaches the injected selected-word rule — this fails if
103+
// the `${aceSelectedWordStyles(token)}` interpolation is dropped from <Global>.
104+
const markerColor = '#010203';
105+
const darkTheme: SupersetTheme = {
106+
...supersetTheme,
107+
colorEditorSelection: markerColor,
108+
};
109+
110+
const { container } = render(<SQLEditor />, {
111+
wrapper: ({ children }: { children?: ReactNode }) => (
112+
<ThemeProvider theme={darkTheme}>{children}</ThemeProvider>
113+
),
114+
});
115+
116+
await waitFor(() => {
117+
expect(container.querySelector('[id="ace-editor"]')).toBeInTheDocument();
118+
});
119+
120+
const css = getInjectedCss();
121+
// The dark-aware color is tied specifically to the selected-word marker
122+
// (not just the plain `.ace_selection`), so the occurrence markers inherit it.
123+
expect(css).toMatch(
124+
new RegExp(`\\.ace_selected-word[^}]*${markerColor}`, 'i'),
69125
);
70126
});
71127

superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/index.tsx

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,14 @@ export type AsyncAceEditorOptions = {
113113
> | null;
114114
};
115115

116+
/**
117+
* Dark-aware color for text selection in the editor. Shared so the plain
118+
* selection and the "selected word" occurrence markers always match, falling
119+
* back to `colorPrimaryBgHover` on themes that don't define the token.
120+
*/
121+
export const editorSelectionColor = (token: SupersetTheme) =>
122+
token.colorEditorSelection ?? token.colorPrimaryBgHover;
123+
116124
/**
117125
* Theme-aware styling for the matched-prefix highlight in the autocomplete
118126
* popup. Ace ships a hardcoded `color: #000` that is invisible on the dark
@@ -135,9 +143,7 @@ export const aceCompletionHighlightStyles = (token: SupersetTheme) => css`
135143
*/
136144
export const aceSelectedWordStyles = (token: SupersetTheme) => css`
137145
.ace_editor .ace_marker-layer .ace_selected-word {
138-
background-color: ${
139-
token.colorEditorSelection ?? token.colorPrimaryBgHover
140-
} !important;
146+
background-color: ${editorSelectionColor(token)} !important;
141147
border: 1px solid ${token.colorBorder} !important;
142148
}
143149
`;
@@ -405,9 +411,7 @@ export function AsyncAceEditor(
405411
}
406412
/* Adjust selection color */
407413
.ace_editor .ace_selection {
408-
background-color: ${
409-
token.colorEditorSelection ?? token.colorPrimaryBgHover
410-
} !important;
414+
background-color: ${editorSelectionColor(token)} !important;
411415
}
412416
413417
${aceSelectedWordStyles(token)}

0 commit comments

Comments
 (0)