[Selection input] Update error styles #28012
Conversation
salazarm
requested review from
bengotow,
hellendag,
benpankow and
clairelin135
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
...odules/dagster-ui/packages/ui-core/src/selection/useSelectionInputLintingAndHighlighting.tsx
Outdated
Show resolved
Hide resolved
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 85437ad. |
|
Deploy preview for dagit-storybook ready! ✅ Preview Built with commit 85437ad. |
| setError(null); | ||
| } | ||
| }; | ||
| document.body.addEventListener('mousemove', listener); |
There was a problem hiding this comment.
Worth targeting this more narrowly than document.body, perhaps just the CM instance element? Mousemove is going to be a pretty active listener.
There was a problem hiding this comment.
we need a global one for this unfortunately since we need to detect mousemoves outside of the container to know to hide the error.
There was a problem hiding this comment.
Hm, could you use a mouseleave on the container?
There was a problem hiding this comment.
Oh forgot to look into this. I think I could probably do that will look into that in a follow up pr
| flex={{direction: 'row', gap: 4}} | ||
| color={Colors.textLight()} | ||
| > | ||
| <Icon name="run_failed" color={Colors.accentRed()} /> |
There was a problem hiding this comment.
error_outline might look a little less loud and a little cleaner than the filled-in error icon.
| return ( | ||
| <Box> | ||
| Mismatched input{' '} | ||
| <MonoSmall color={Colors.textRed()}>'{error.error.mismatchedInput}'</MonoSmall>. |
There was a problem hiding this comment.
The ' wrappers might not be needed, the color and monospace are already good visual indicators to show the mismatched string.
Summary & Motivation
Note that updating the usefulness of the error messages is outside of the scope of this PR. Currently just returning what ANTLR gives us but I think we can do better in the future.
Stop using CodeMirror's linter functionality and instead running linting ourselves in order to have control over the error tooltip rendering beyond just the class name.
How I Tested These Changes
Manual testing