Remove Ivy Codemirror #14659

Monkeychip · 2022-03-23T15:58:10Z

Replacing the deprecated ivycodemirror with an ember modifier. See Deprecation and instructions here.

Notes:

This PR came about because of this issue. Unfortunately without having some regression issues there was no simple way to allow copying while also not showing the cursor. In order to do this we needed to remove IvyCodemirror.
We no longer use an options object.
This is our first custom modifier. If you're curious how they work, this documentation was very helpful.
This became a bigger PR than I had originally thought it would be. My apologies, but these changes should happen all at once.

Screen.Recording.2022-03-28.at.11.54.53.AM.mp4

Monkeychip · 2022-03-28T18:08:35Z

ui/app/components/json-editor.js

 */

-const JSON_EDITOR_DEFAULTS = {
-  // IMPORTANT: `gutters` must come before `lint` since the presence of


moved comment to the modifier.

Monkeychip · 2022-03-28T18:09:44Z

ui/app/styles/components/codemirror.scss

@@ -168,31 +168,12 @@ $gutter-grey: #2a2f36;
 }

 .readonly-codemirror {
-  .CodeMirror-cursors {


this was not working. We have it working now under a new class that only shows if the editor is readOnly.

Monkeychip · 2022-03-28T18:16:20Z

ui/app/templates/components/console/log-json.hbs

@@ -2,7 +2,10 @@
  <JsonEditor
    @showToolbar={{false}}
    @value={{stringify this.content}}
-    @options={{hash readOnly=true lineNumbers=false autoHeight=true gutters=false theme="hashi auto-height"}}


all options objects were spread out so that the individual properties were sent to the json-editor.

I'm not seeing autoHeight as an arg. Is the codemirror default true and is that something we won't ever want to set to false?

Good catch. I removed this because before it was used to set the viewportMargin. See here

Ahh ok I see now thanks!

Monkeychip · 2022-03-28T18:23:46Z

ui/tests/integration/components/console/log-json-test.js

@@ -19,8 +19,7 @@ module('Integration | Component | console/log json', function (hooks) {
    this.set('content', objectContent);

    await render(hbs`{{console/log-json content=content}}`);
-    const instance = this.codeMirror.instanceFor(find('[data-test-component=json-editor]').id);


this old way of selecting the codemirror instance no longer works. I suspect because we're initializing it through a modifier instead of on the component via ivycodemirror.

Monkeychip · 2022-03-28T18:32:03Z

ui/ember-cli-build.js

@@ -26,10 +26,6 @@ const appConfig = {
      return `${config.rootURL.replace(/\/$/, '')}${filePath}`;
    },
  },
-  codemirror: {


everything removed from this file was added into the modifier.

zofskeez

Great work on this thanks! Small wording issue on a comment and I wanted to get your thoughts on the mode arg before approving.

zofskeez · 2022-03-28T20:34:57Z

ui/app/components/json-editor.js

- * @param {object} [options] - option object that overrides codemirror default options such as the styling.
+ * @param {Object} [extraKeys] - to provide keyboard shortcut methods for things like saving on shift + enter.
+ * @param {Array} [gutters] - An array of CSS class names or class name / CSS string pairs, each of which defines a width (and optionally a background), and which will be used to draw the background of the gutters.
+ * @param {string} [mode] - right now we only import ruby so must mode but be ruby or defaults to javascript. If you wanted another language you need to import it into the modifier.


Small nit on the comment wording. Also, from reading it I'm wondering since ruby and application/json are the only supported modes at this time could the arg change to useRubyMode or something like that? If true the mode is set to ruby otherwise application/json is used. What are your thoughts?

That's a valid question. I think however I prefer mode because it aligns with the codemirror property. It would be easy for someone to look up. I also don't want someone to think they only can use Ruby in the future. They can use lots of other options they just need to import it. They'll immediately error to if they use something other than ruby.

I'll amend the comment wording.

Sounds good so if I were to pass in php right now for example codemirror would throw an error since it's not imported?

correct, it wouldn't work.

zofskeez · 2022-03-28T20:52:56Z

ui/app/modifiers/code-mirror.js

+      // `gutters` is cached internally when `lint` is toggled
+      gutters: this.args.named.gutters || ['CodeMirror-lint-markers'],
+      matchBrackets: true,
+      lint: { lintOnChange: true },


The previous version of JsonEditor had lintOnChange defaulted to false. Just want to verify that we want that as true in the new version.

Good catch. This was left over from me playing with the linting. I'll change.

Okay, strange. I changed back and it broke the linter. Let me investigate a little tomorrow because we are calling performLint() so we shouldn't need this.

zofskeez · 2022-03-28T20:57:30Z

ui/app/templates/components/console/log-json.hbs

@@ -2,7 +2,10 @@
  <JsonEditor
    @showToolbar={{false}}
    @value={{stringify this.content}}
-    @options={{hash readOnly=true lineNumbers=false autoHeight=true gutters=false theme="hashi auto-height"}}


I'm not seeing autoHeight as an arg. Is the codemirror default true and is that something we won't ever want to set to false?

Monkeychip added 2 commits March 23, 2022 09:57

setup

d16a374

fix mode issue

0d1126e

vercel bot temporarily deployed to Preview – vault March 23, 2022 16:11 Inactive

vercel bot deployed to Preview – vault-storybook March 23, 2022 16:11 View deployment

actions

fd8ae4a

vercel bot deployed to Preview – vault-storybook March 23, 2022 16:55 View deployment

vercel bot temporarily deployed to Preview – vault March 23, 2022 16:55 Inactive

Monkeychip added 2 commits March 23, 2022 11:28

readonly styling

2364396

remove ivycodemirror from package json

f5e4935

vercel bot temporarily deployed to Preview – vault March 23, 2022 17:31 Inactive

vercel bot had a problem deploying to Preview – vault-storybook March 23, 2022 17:31 Failure

Monkeychip added 2 commits March 23, 2022 14:12

wip

0a40a76

test coverage for json editor

4d4591d

vercel bot temporarily deployed to Preview – vault March 24, 2022 20:00 Inactive

vercel bot deployed to Preview – vault-storybook March 24, 2022 20:00 View deployment

text fixes

14b2f90

vercel bot temporarily deployed to Preview – vault March 24, 2022 20:18 Inactive

vercel bot deployed to Preview – vault-storybook March 24, 2022 20:18 View deployment

Monkeychip added 2 commits March 28, 2022 10:50

fix tests

c582582

fix cursor issue

1a7c47b

vercel bot deployed to Preview – vault-storybook March 28, 2022 17:41 View deployment

vercel bot temporarily deployed to Preview – vault March 28, 2022 17:41 Inactive

changelog

0de2185

vercel bot temporarily deployed to Preview – vault March 28, 2022 17:46 Inactive

Monkeychip added the ui label Mar 28, 2022

Monkeychip added this to the 1.11.0-rc1 milestone Mar 28, 2022

Monkeychip marked this pull request as ready for review March 28, 2022 17:47

vercel bot temporarily deployed to Preview – vault-storybook March 28, 2022 17:54 Inactive

Monkeychip commented Mar 28, 2022

View reviewed changes

clean up

31a2565

vercel bot deployed to Preview – vault-storybook March 28, 2022 18:26 View deployment

vercel bot temporarily deployed to Preview – vault March 28, 2022 18:27 Inactive

Monkeychip commented Mar 28, 2022

View reviewed changes

fix

e2364fd

vercel bot deployed to Preview – vault-storybook March 28, 2022 19:46 View deployment

vercel bot temporarily deployed to Preview – vault March 28, 2022 19:46 Inactive

zofskeez requested changes Mar 28, 2022

View reviewed changes

address pr comments

d36665f

vercel bot temporarily deployed to Preview – vault March 28, 2022 21:44 Inactive

vercel bot deployed to Preview – vault-storybook March 28, 2022 21:44 View deployment

unused css and it overides other styling.

14893c3

vercel bot deployed to Preview – vault-storybook March 28, 2022 23:22 View deployment

vercel bot temporarily deployed to Preview – vault March 28, 2022 23:22 Inactive

fix

91139d8

vercel bot deployed to Preview – vault-storybook March 28, 2022 23:44 View deployment

vercel bot temporarily deployed to Preview – vault March 28, 2022 23:44 Inactive

zofskeez approved these changes Mar 29, 2022

View reviewed changes

fix comment

549c3b4

vercel bot temporarily deployed to Preview – vault March 29, 2022 15:46 Inactive

vercel bot deployed to Preview – vault-storybook March 29, 2022 15:46 View deployment

Monkeychip merged commit e1c89ec into main Mar 29, 2022

Monkeychip deleted the ui/add-modifier branch March 29, 2022 16:25

This was referenced Mar 30, 2022

Remove nocursor from json-editor #14784

Merged

Remove nocursor from json-editor #14785

Merged

Remove nocursor from json-editor #14786

Merged

philrenaud mentioned this pull request Jun 27, 2022

Trimmed the ivy from the codemirror hashicorp/nomad#13503

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove Ivy Codemirror #14659

Remove Ivy Codemirror #14659

Monkeychip commented Mar 23, 2022 •

edited

Loading

Monkeychip Mar 28, 2022

Monkeychip Mar 28, 2022

Monkeychip Mar 28, 2022

zofskeez Mar 28, 2022

Monkeychip Mar 28, 2022

zofskeez Mar 28, 2022

Monkeychip Mar 28, 2022

Monkeychip Mar 28, 2022

zofskeez left a comment

zofskeez Mar 28, 2022

Monkeychip Mar 28, 2022

zofskeez Mar 28, 2022

Monkeychip Mar 28, 2022

zofskeez Mar 28, 2022

Monkeychip Mar 28, 2022

Monkeychip Mar 28, 2022

zofskeez Mar 28, 2022

Remove Ivy Codemirror #14659

Remove Ivy Codemirror #14659

Conversation

Monkeychip commented Mar 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zofskeez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Monkeychip commented Mar 23, 2022 •

edited

Loading