[ui] Reimplement CodeMirror wrapper #16347
Conversation
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
|
This is WIP-ish, since I want to do some more manual testing to make sure I haven't missed anything. |
| @@ -683,6 +683,7 @@ function findAutocompletionContext( | |||
|
|
|||
| // Find context for a fully- or partially- typed key or value in the YAML document | |||
| export function expandAutocompletionContextAtCursor(editor: any) { | |||
| debugger; | |||
There was a problem hiding this comment.
yeesh... this was definitely me. I'll see if we can get a linter rule forbidding these, I'm surprised it doesn't warn about them already.
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 46e3180. |
|
Deploy preview for dagit-storybook ready! ✅ Preview Built with commit 46e3180. |
| @@ -683,6 +683,7 @@ function findAutocompletionContext( | |||
|
|
|||
| // Find context for a fully- or partially- typed key or value in the YAML document | |||
| export function expandAutocompletionContextAtCursor(editor: any) { | |||
| debugger; | |||
There was a problem hiding this comment.
yeesh... this was definitely me. I'll see if we can get a linter rule forbidding these, I'm surprised it doesn't warn about them already.
| [], | ||
| ); | ||
|
|
||
| return <StyledRawCodeMirror value={`hello\n world`} options={options} />; |
There was a problem hiding this comment.
Ahh having stories for this is really nice 👍 Maybe we can add one that presents a complex YAML document too -- there are so many weird supported features in YAML it'd be nice to have an exhaustive test-case document. I can add one in a follow-up!
0df49ad
to
46e3180
Compare
|
We've done a lot of manual testing and things look good, so I'm merging this! We can add fixes in followups as needed. |
## Summary & Motivation Implement our own React wrapper for CodeMirror 5. It turns out that react-codemirror2 has some bugs that appear related to React 18, so if we just set up the CodeMirror ourselves, we should be able to sidestep the existing problems and more effectively debug anything else that comes up. This will also hopefully help set us up to upgrade to CodeMirror 6 much more easily. In this PR, I have replaced the existing Launchpad config editor with the new implementation. ## How I Tested These Changes Using Storybook example for the new CodeMirror component, test basic text editing. View a Launchpad in Dagster. Verify: - Scaffolding the config works correctly - The config is used properly for autocomplete - All basic editing behavior is correct - Newline entry and multiple cursors work correctly - Syntax highlighting is correct
Summary & Motivation
Implement our own React wrapper for CodeMirror 5. It turns out that react-codemirror2 has some bugs that appear related to React 18, so if we just set up the CodeMirror ourselves, we should be able to sidestep the existing problems and more effectively debug anything else that comes up.
This will also hopefully help set us up to upgrade to CodeMirror 6 much more easily.
In this PR, I have replaced the existing Launchpad config editor with the new implementation.
How I Tested These Changes
Using Storybook example for the new CodeMirror component, test basic text editing.
View a Launchpad in Dagster. Verify: