diff --git a/js_modules/dagster-ui/packages/ui-components/src/components/Popover.tsx b/js_modules/dagster-ui/packages/ui-components/src/components/Popover.tsx index cc5068ed58f68..6167c87548a1e 100644 --- a/js_modules/dagster-ui/packages/ui-components/src/components/Popover.tsx +++ b/js_modules/dagster-ui/packages/ui-components/src/components/Popover.tsx @@ -3,22 +3,40 @@ import {Popover2, Popover2Props} from '@blueprintjs/popover2'; import deepmerge from 'deepmerge'; import * as React from 'react'; -import {createGlobalStyle} from 'styled-components'; +import {createGlobalStyle, css} from 'styled-components'; import {Colors} from './Color'; import {FontFamily} from './styles'; import searchSVG from '../icon-svgs/search.svg'; +export const PopoverWrapperStyle = css` + box-shadow: ${Colors.shadowDefault()} 0px 2px 12px; +`; + +export const PopoverContentStyle = css` + background-color: ${Colors.popoverBackground()}; + border-radius: 4px; + + > :first-child { + border-top-left-radius: 4px; + border-top-right-radius: 4px; + } + + > :last-child { + border-bottom-left-radius: 4px; + border-bottom-right-radius: 4px; + } +`; + export const GlobalPopoverStyle = createGlobalStyle` .dagster-popover.bp5-popover, .dagster-popover.bp5-popover { - box-shadow: ${Colors.shadowDefault()} 0px 2px 12px; + ${PopoverWrapperStyle} } .dagster-popover .bp5-popover-content, .dagster-popover .bp5-popover-content { - background-color: ${Colors.popoverBackground()}; - border-radius: 4px; + ${PopoverContentStyle} .bp5-menu { background-color: ${Colors.popoverBackground()}; @@ -54,16 +72,6 @@ export const GlobalPopoverStyle = createGlobalStyle` } } - .dagster-popover .bp5-popover-content > :first-child { - border-top-left-radius: 4px; - border-top-right-radius: 4px; - } - - .dagster-popover .bp5-popover-content > :last-child { - border-bottom-left-radius: 4px; - border-bottom-right-radius: 4px; - } - .dagster-popover .bp5-popover-arrow-fill { fill: ${Colors.popoverBackground()}; } diff --git a/js_modules/dagster-ui/packages/ui-core/src/asset-selection/input/AssetSelectionInput.oss.tsx b/js_modules/dagster-ui/packages/ui-core/src/asset-selection/input/AssetSelectionInput.oss.tsx index eebe3914ab890..ad3bd52248fcf 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/asset-selection/input/AssetSelectionInput.oss.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/asset-selection/input/AssetSelectionInput.oss.tsx @@ -1,8 +1,8 @@ -import type {Linter} from 'codemirror/addon/lint/lint'; import {useAssetSelectionAutoCompleteProvider as defaultUseAssetSelectionAutoCompleteProvider} from 'shared/asset-selection/input/useAssetSelectionAutoCompleteProvider.oss'; import {assetSelectionSyntaxSupportedAttributes, unsupportedAttributeMessages} from './util'; import {AssetGraphQueryItem} from '../../asset-graph/useAssetGraphData'; +import {SyntaxError} from '../../selection/CustomErrorListener'; import {SelectionAutoCompleteProvider} from '../../selection/SelectionAutoCompleteProvider'; import {SelectionAutoCompleteInput} from '../../selection/SelectionInput'; import {createSelectionLinter} from '../../selection/createSelectionLinter'; @@ -13,7 +13,7 @@ export interface AssetSelectionInputProps { assets: AssetGraphQueryItem[]; value: string; onChange: (value: string) => void; - linter?: Linter; + linter?: (content: string) => SyntaxError[]; useAssetSelectionAutoComplete?: ( assets: AssetGraphQueryItem[], ) => Pick; diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/CustomErrorListener.ts b/js_modules/dagster-ui/packages/ui-core/src/selection/CustomErrorListener.ts index 39b40f9dcc68e..1a3fdf0b125f0 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/CustomErrorListener.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/CustomErrorListener.ts @@ -1,12 +1,11 @@ import {ANTLRErrorListener, RecognitionException, Recognizer} from 'antlr4ts'; -import {Token} from 'antlr4ts/Token'; -export interface SyntaxError { +export type SyntaxError = { message: string; - line: number; - column: number; - offendingSymbol: Token | null; -} + from: number; + to: number; + offendingSymbol?: string | null | undefined; +}; export class CustomErrorListener implements ANTLRErrorListener { private errors: SyntaxError[]; @@ -18,16 +17,16 @@ export class CustomErrorListener implements ANTLRErrorListener { syntaxError( _recognizer: Recognizer, offendingSymbol: any, - line: number, + _line: number, charPositionInLine: number, msg: string, _e: RecognitionException | undefined, ): void { this.errors.push({ message: msg, - line, - column: charPositionInLine, - offendingSymbol, + offendingSymbol: offendingSymbol?.text, + from: charPositionInLine, + to: charPositionInLine + (offendingSymbol?.text?.length ?? Infinity), }); } diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInput.tsx b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInput.tsx index e957f1c96e8f2..bb638a663f9eb 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInput.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInput.tsx @@ -1,17 +1,15 @@ import {Box, Colors, Icon, Popover, UnstyledButton} from '@dagster-io/ui-components'; import useResizeObserver from '@react-hook/resize-observer'; import CodeMirror, {Editor, EditorChange} from 'codemirror'; -import type {Linter} from 'codemirror/addon/lint/lint'; import debounce from 'lodash/debounce'; import React, {KeyboardEvent, useCallback, useLayoutEffect, useMemo, useRef, useState} from 'react'; import styled from 'styled-components'; +import {SyntaxError} from './CustomErrorListener'; import {SelectionAutoCompleteProvider} from './SelectionAutoCompleteProvider'; import {SelectionInputAutoCompleteResults} from './SelectionInputAutoCompleteResults'; -import { - SelectionAutoCompleteInputCSS, - applyStaticSyntaxHighlighting, -} from './SelectionInputHighlighter'; +import {SelectionAutoCompleteInputCSS} from './SelectionInputHighlighter'; +import {useSelectionInputLintingAndHighlighting} from './useSelectionInputLintingAndHighlighting'; import {useTrackEvent} from '../app/analytics'; import {useDangerousRenderEffect} from '../hooks/useDangerousRenderEffect'; import {useUpdatingRef} from '../hooks/useUpdatingRef'; @@ -25,7 +23,7 @@ import 'codemirror/addon/display/placeholder'; type SelectionAutoCompleteInputProps = { id: string; // Used for logging placeholder: string; - linter: Linter; + linter: (content: string) => SyntaxError[]; value: string; onChange: (value: string) => void; useAutoComplete: SelectionAutoCompleteProvider['useAutoComplete']; @@ -100,10 +98,6 @@ export const SelectionAutoCompleteInput = ({ lineWrapping: false, // Initially false; enable during focus scrollbarStyle: 'native', autoCloseBrackets: true, - lint: { - getAnnotations: linter, - async: false, - }, placeholder, extraKeys: { 'Ctrl-Space': 'autocomplete', @@ -160,7 +154,6 @@ export const SelectionAutoCompleteInput = ({ }); cmInstance.current.on('cursorActivity', (instance: Editor) => { - applyStaticSyntaxHighlighting(instance); const nextCursorPosition = instance.getCursor().ch; if (cursorPositionRef.current !== nextCursorPosition) { // If the cursor has moved then update the cursor position @@ -169,18 +162,16 @@ export const SelectionAutoCompleteInput = ({ setShowResults({current: true}); } }); - - requestAnimationFrame(() => { - if (!cmInstance.current) { - return; - } - - applyStaticSyntaxHighlighting(cmInstance.current); - }); } // eslint-disable-next-line react-hooks/exhaustive-deps }, []); + const errorTooltip = useSelectionInputLintingAndHighlighting({ + cmInstance, + value: innerValue, + linter, + }); + const [currentHeight, setCurrentHeight] = useState(20); const adjustHeight = useCallback(() => { @@ -392,6 +383,7 @@ export const SelectionAutoCompleteInput = ({ + {errorTooltip} ); }; diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputHighlighter.ts b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputHighlighter.ts index ecdd2f43e60c5..4cfb30a5e3c01 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputHighlighter.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputHighlighter.ts @@ -4,6 +4,7 @@ import {AbstractParseTreeVisitor} from 'antlr4ts/tree/AbstractParseTreeVisitor'; import CodeMirror from 'codemirror'; import {css} from 'styled-components'; +import {SyntaxError} from './CustomErrorListener'; import {parseInput} from './SelectionInputParser'; import { AllExpressionContext, @@ -188,15 +189,18 @@ export class SyntaxHighlightingVisitor } } -export function applyStaticSyntaxHighlighting(cm: CodeMirror.Editor): void { +export function applyStaticSyntaxHighlighting(cm: CodeMirror.Editor, errors: SyntaxError[]): void { const value = cm.getValue(); // Clear existing marks to avoid duplication cm.getAllMarks().forEach((mark) => { - // Don't clear error marks coming from the linter which uses the real grammar's parser - if ((mark as any)?.__annotation?.severity !== 'error') { - mark.clear(); - } + mark.clear(); + }); + + errors.forEach((error, idx) => { + cm.markText(cm.posFromIndex(error.from), cm.posFromIndex(error.to), { + className: `selection-input-error selection-input-error-${idx}`, + }); }); const cursorIndex = cm.getCursor().ch; @@ -255,7 +259,7 @@ export const SelectionAutoCompleteInputCSS = css` font-family: ${FontFamily.monospace}; } - .CodeMirror-lint-mark-error { + .selection-input-error { background: unset; text-decoration-line: underline; text-decoration-style: wavy; diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputParser.ts b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputParser.ts index 0c09586b1bda4..3b7f09a819e34 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputParser.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputParser.ts @@ -44,7 +44,6 @@ export const parseInput = memoize((input: string): ParseResult => { // Attach custom error listener const errorListener = new CustomErrorListener(); parser.removeErrorListeners(); - parser.addErrorListener(errorListener); // Set the error handler to bail on error to prevent infinite loops parser.errorHandler = new BailErrorStrategy(); @@ -64,7 +63,7 @@ export const parseInput = memoize((input: string): ParseResult => { if (currentErrors.length > 0) { const error = currentErrors[0]!; - const errorCharPos = error.column; + const errorCharPos = error.from; const errorIndex = currentPosition + errorCharPos; // Parse up to the error @@ -89,9 +88,8 @@ export const parseInput = memoize((input: string): ParseResult => { // Add the error to the errors array errors.push({ message: error.message, - line: error.line, - column: error.column + currentPosition, - offendingSymbol: error.offendingSymbol, + from: error.from + currentPosition, + to: error.to + currentPosition, }); // Advance currentPosition beyond the error diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/__tests__/createSelectionLinter.test.ts b/js_modules/dagster-ui/packages/ui-core/src/selection/__tests__/createSelectionLinter.test.ts index a47084b5c3b12..0ee4730f43b4b 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/__tests__/createSelectionLinter.test.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/__tests__/createSelectionLinter.test.ts @@ -1,5 +1,3 @@ -import CodeMirror from 'codemirror'; - import {AssetSelectionLexer} from '../../asset-selection/generated/AssetSelectionLexer'; import {AssetSelectionParser} from '../../asset-selection/generated/AssetSelectionParser'; import {createSelectionLinter} from '../createSelectionLinter'; @@ -39,21 +37,18 @@ describe('createSelectionLinter', () => { expect(errors).toEqual([ { message: 'tag filtering is not supported in this test', - severity: 'error', - from: CodeMirror.Pos(0, 0), - to: CodeMirror.Pos(0, 'tag'.length), + from: 0, + to: 'tag'.length, }, { message: 'column filtering is not supported in this test', - severity: 'error', - from: CodeMirror.Pos(0, 'tag:value or '.length), - to: CodeMirror.Pos(0, 'tag:value or column'.length), + from: 'tag:value or '.length, + to: 'tag:value or column'.length, }, { - message: 'Unsupported attribute: table_name', // default message - severity: 'error', - from: CodeMirror.Pos(0, 'tag:value or column:value or '.length), - to: CodeMirror.Pos(0, 'tag:value or column:value or table_name'.length), + message: 'Unsupported attribute: "table_name"', // default message + from: 'tag:value or column:value or '.length, + to: 'tag:value or column:value or table_name'.length, }, ]); }); @@ -71,10 +66,9 @@ describe('createSelectionLinter', () => { // Only expect syntax errors, not attribute errors expect(errors).toEqual([ expect.objectContaining({ - from: CodeMirror.Pos(0, 0), - message: expect.stringContaining("mismatched input 'fake'"), - severity: 'error', - to: CodeMirror.Pos(0, 10), + from: 0, + offendingSymbol: 'fake', + to: 4, }), ]); }); diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/createSelectionLinter.ts b/js_modules/dagster-ui/packages/ui-core/src/selection/createSelectionLinter.ts index 545ce00b9ff99..3da0e4360f00b 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/selection/createSelectionLinter.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/createSelectionLinter.ts @@ -1,8 +1,7 @@ import {CharStreams, CommonTokenStream, Lexer, Parser, ParserRuleContext} from 'antlr4ts'; import {AbstractParseTreeVisitor} from 'antlr4ts/tree/AbstractParseTreeVisitor'; -import CodeMirror from 'codemirror'; -import {CustomErrorListener} from './CustomErrorListener'; +import {CustomErrorListener, SyntaxError} from './CustomErrorListener'; import {parseInput} from './SelectionInputParser'; import {AttributeNameContext} from './generated/SelectionAutoCompleteParser'; import {SelectionAutoCompleteVisitor} from './generated/SelectionAutoCompleteVisitor'; @@ -27,17 +26,18 @@ export function createSelectionLinter({ if (!text.length) { return []; } - const errorListener = new CustomErrorListener(); const inputStream = CharStreams.fromString(text); const lexer = new LexerKlass(inputStream); - lexer.removeErrorListeners(); - lexer.addErrorListener(errorListener); - const tokens = new CommonTokenStream(lexer); const parser = new ParserKlass(tokens); + const errorListener = new CustomErrorListener(); + + lexer.removeErrorListeners(); + lexer.addErrorListener(errorListener); + parser.removeErrorListeners(); // Remove default console error listener parser.addErrorListener(errorListener); @@ -45,10 +45,8 @@ export function createSelectionLinter({ // Map syntax errors to CodeMirror's lint format const lintErrors = errorListener.getErrors().map((error) => ({ + ...error, message: error.message.replace(', ', ''), - severity: 'error', - from: CodeMirror.Pos(0, error.column), - to: CodeMirror.Pos(0, text.length), })); const {parseTrees} = parseInput(text); @@ -67,22 +65,17 @@ class InvalidAttributeVisitor extends AbstractParseTreeVisitor implements SelectionAutoCompleteVisitor { - private errors: { - message: string; - severity: 'error' | 'warning'; - from: CodeMirror.Position; - to: CodeMirror.Position; - }[] = []; - private sortedLintErrors: {from: CodeMirror.Position; to: CodeMirror.Position}[]; + private errors: SyntaxError[] = []; + private sortedLintErrors: SyntaxError[]; constructor( private supportedAttributes: readonly string[], private unsupportedAttributeMessages: Record, - lintErrors: {from: CodeMirror.Position; to: CodeMirror.Position}[], + lintErrors: SyntaxError[], ) { super(); // Sort errors by start position for efficient searching - this.sortedLintErrors = [...lintErrors].sort((a, b) => a.from.ch - b.from.ch); + this.sortedLintErrors = [...lintErrors].sort((a, b) => a.from - b.from); } getErrors() { @@ -93,7 +86,7 @@ class InvalidAttributeVisitor return undefined; } - private hasOverlap(from: CodeMirror.Position, to: CodeMirror.Position): boolean { + private hasOverlap(from: number, to: number): boolean { // Binary search to find the first error that could potentially overlap let low = 0; let high = this.sortedLintErrors.length - 1; @@ -102,9 +95,9 @@ class InvalidAttributeVisitor const mid = Math.floor((low + high) / 2); const error = this.sortedLintErrors[mid]!; - if (error.to.ch < from.ch) { + if (error.to < from) { low = mid + 1; - } else if (error.from.ch > to.ch) { + } else if (error.from > to) { high = mid - 1; } else { // Found an overlapping error @@ -117,15 +110,14 @@ class InvalidAttributeVisitor visitAttributeName(ctx: AttributeNameContext) { const attributeName = ctx.IDENTIFIER().text; if (!this.supportedAttributes.includes(attributeName)) { - const from = CodeMirror.Pos(0, ctx.start.startIndex); - const to = CodeMirror.Pos(0, ctx.stop!.stopIndex + 1); + const from = ctx.start.startIndex; + const to = ctx.stop!.stopIndex + 1; if (!this.hasOverlap(from, to)) { this.errors.push({ message: this.unsupportedAttributeMessages[attributeName] ?? - `Unsupported attribute: ${attributeName}`, - severity: 'error', + `Unsupported attribute: "${attributeName}"`, from, to, }); diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx b/js_modules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx new file mode 100644 index 0000000000000..fcee167ad6922 --- /dev/null +++ b/js_modules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx @@ -0,0 +1,137 @@ +import { + BodySmall, + Box, + Colors, + Icon, + MiddleTruncate, + MonoSmall, + PopoverContentStyle, + PopoverWrapperStyle, +} from '@dagster-io/ui-components'; +import {useLayoutEffect, useMemo, useState} from 'react'; +import ReactDOM from 'react-dom'; +import styled from 'styled-components'; + +import {SyntaxError} from './CustomErrorListener'; +import {applyStaticSyntaxHighlighting} from './SelectionInputHighlighter'; +import {useUpdatingRef} from '../hooks/useUpdatingRef'; + +export const useSelectionInputLintingAndHighlighting = ({ + cmInstance, + value, + linter, +}: { + cmInstance: React.MutableRefObject; + value: string; + linter: (content: string) => SyntaxError[]; +}) => { + const errors = useMemo(() => { + const errors = linter(value); + return errors.map((error, idx) => ({ + ...error, + idx, + })); + }, [linter, value]); + + useLayoutEffect(() => { + if (!cmInstance.current) { + return; + } + applyStaticSyntaxHighlighting(cmInstance.current, errors); + }, [cmInstance, errors]); + + const errorsRef = useUpdatingRef(errors); + + const [error, setError] = useState<{ + error: SyntaxError; + x: number; + y: number; + } | null>(null); + + const errorRef = useUpdatingRef(error); + + useLayoutEffect(() => { + const listener = (ev: MouseEvent) => { + if (!(ev.target instanceof HTMLElement)) { + return; + } + const error = ev.target.closest('.selection-input-error') as HTMLElement | null; + if (error) { + const regex = /selection-input-error-(\d+)/; + const errorIdx = parseInt(error.className.match(regex)?.[1] ?? '0', 10); + const errorAnnotation = errorsRef.current[errorIdx]; + if (errorAnnotation) { + setError({ + error: errorAnnotation, + x: ev.clientX, + y: ev.clientY, + }); + return; + } + } + if (errorRef.current) { + setError(null); + } + }; + document.body.addEventListener('mousemove', listener); + return () => { + document.body.removeEventListener('mousemove', listener); + }; + }, [cmInstance, errorsRef, errorRef]); + + const message = useMemo(() => { + if (!error) { + return null; + } + if (error.error.offendingSymbol) { + return ( + + Unexpected input +
+ +
+ +
+ + . + + ); + } + if (error.error.message) { + return error.error.message; + } + return null; + }, [error]); + + if (!error) { + return null; + } + + return ReactDOM.createPortal( + + + + {message} + + , + document.body, + ); +}; + +const PortalElement = styled.div<{$bottom: number; $left: number}>` + position: absolute; + top: ${({$bottom}) => $bottom - 32}px; + left: ${({$left}) => $left + 16}px; + max-width: 600px; + z-index: 20; // Z-index 20 to match bp5-overlay + ${PopoverWrapperStyle} +`; + +const Content = styled.div` + ${PopoverContentStyle} +`;