Changes needed for VSCode webview extension #186

TranquilMarmot · 2020-05-25T05:32:31Z

What is this?

This PR contains changes needed for the VSCode extension to have an embedded webview of the editor.

Checkout out the VSCode extension PR here: YarnSpinnerTool/VSCodeExtension#1

The goal of this PR was to leave the editor intact for current use cases (in-browser app and Electron app) but still allow for it to be embedded in a VSCode webview.

This is my first time working with anything Knockout related, so bear with me! I might have done some silly things.

All in all, it was surprisingly easy to shove this into a VSCode extension. The way that the events and everything are hooked up worked nicely for this use case!

Changes

Assets in the `public` folder

For more info on this read the "Loading local content" section of the VSCode webview docs.

Basically, for security reasons, the webview only has access to specific assets. These specific assets need to be loaded with a URI that is generated by the webview with a scheme of vscode-resource:. As you can imagine, there are a lot of hoops to jump through here.

As far as this PR is concerned, this means a new function in Utils called getPublicPath that can be used to get the path for an asset that would normally be acessible in the public folder. Any assets that were referring to public are now piped through this (with the exception of some assets in index.html that are find-and-replace'd with the proper URI by the extension when it renders the HTML).

`localStorage` and settings

Accessing window.localStorage at all within a webview will immediately throw an exception and cause the app to stop executing. It will now fake an object with noops for getting/setting values.

All settings from inside the extension are instead handled by VSCode's built-in settings mechanisms; see the other PR for more info.

The "Start" node

This one was a bit of a doozy; you'll see that I added in app.js:

if (!window.editingVsCodeFile) {
  self.newNode().title('Start');
}

This is for the use case of opening an existing file in VSCode. If opening a file, the extension will set window.openingVsCodeFile to true so that the "Start" node will not be created.

This has to be done because the extension listens for the yarnReady and then calls out to (among other things) data.loadData. At this point, app would be trying to call workspace.updateArrows(); but the "Start" node would be null (since we tell loadData to empty the node list) which would throw an exception and kill the app.

…ebviewUri" function if it's available The VSCode extension will put this function on "window" when starting the editor, and calling it will return the proper URI to get an asset from the "public" folder.

TranquilMarmot · 2020-05-25T07:38:02Z

src/scss/style.css

@@ -76,6 +76,8 @@ input[type='checkbox'] {

 #app {
  position: absolute;
+  top: 0px;
+  left: 0px;


For some reason, in the VSCode webview, #app was a shifted few pixels to the right which was causing issues like arrows not rendering in the right spot.

Adding this gets it to the right spot, and doesn't seem to have any effect on the web app.

TranquilMarmot · 2020-05-25T20:53:43Z

src/js/classes/app.js

+  // This should be called whenever we want to mark the document that the VSCcode
+  // extension has opened as changed. If we're not in the extension, this is a no-op.
+  // This should be called after every action that will result in a changed text document.
+  this.updateVsCodeExtensionDocument = function() {


The extension relies on this being called to update its internal document and mark the document as "dirty" so that it can be saved.

I think I got all the places that this needs to be called...

Node added/removed

Node dragged

Node text editor closed

Node color changed

We can benefit from this documentIsDirty in the electron version to add an * to the window title after a file has been edited.

Can we perhaps refactor this so yarn in general knows when a document is dirty (when running in electron or vscode).

If not, thats fine :)

actually can you just rename this function from
this.updateVsCodeExtensionDocument
to
this.setYarnDocumentIsDirty

later on we can use it to handle the electron case

@daviddq do you think it would be a good idea?

Sounds good.

command, data -> type, payload

TranquilMarmot · 2020-05-26T02:24:03Z

Okay, I think this is finished!

Check out this comment on the other PR for instructions to download/install and test it: YarnSpinnerTool/VSCodeExtension#1 (comment)

blurymind · 2020-05-27T12:35:11Z

src/js/classes/data.js

+      // if we're editing a file in the VSCode extension, it handles
+      // saving the file on its end so we do nothing here
+      return;
+    } else if (self.editingPath())


the else is redundant here, having that return there will prevent if (self.editingPath() from being evaluated anyway

blurymind · 2020-05-27T13:14:03Z

I gave this a test and am quite impressed. Some notes are due however

The settings window is not accessible via the editor (thats fine, however...)
the the Night Mode settings established in yarnSpinner's settings inside vscode dont seem to work - I am constantly stuck in dark mode regardless if its ticked
Same for markup language setting and possibly others

Are those thing you need fixing at the other PR?

…om/TranquilMarmot/YarnEditor into tranquilmarmot/vscode-yarn-spinner

TranquilMarmot · 2020-05-28T05:33:25Z

Settings:
I went back and forth on the settings window in the editor.

Originally, I had added a bunch of messaging to sync the settings when they're changed in the editor but it ended up being pretty weird; it would create a .vscode/settings.json file at the root of the project repo which may or may not be what the user wants.

In the end I opted for just having VSCode handle all the settings and removing the options from the editor altogether.

Night mode:
As far as night mode goes, the extension will try and detect if your VSCode theme is "dark" and auto switch to it.

If you want to go back to not-night mode (day mode?) there's another option called "Override Dark Theme Night Mode" that you have to turn on.

If this is too confusing, it can be removed and just have the user be able to change the setting.

Markup language:
Good catch with the markup language setting not changing! It looks like app.setMarkupLanguage was missing from here:
https://github.com/YarnSpinnerTool/YarnEditor/blob/6073f32980d3b99fb04b13b51b5c1636ee30e247/src/js/classes/settings.js#L15-L20

blurymind · 2020-05-28T10:53:16Z

Other stuff seems to be missing too - if thats where you apply it. Can you make sure those work?

@daviddq is this right? Should it be applied at another place for those?
Or do they need to be added there too?

daviddq · 2020-05-28T11:36:50Z

First of all, let me say I find this feature impressively wonderful, @TranquilMarmot. I won't be using anything different but this extension when it's fully ready. Thanks a lot.

I tested the extension some commits ago and settings were not fully working. I'll give it a try this afternoon. I find removing the "File/Settings" menu absolutely the way to go inside VSCode.I wish something similar was possible for the "search" box.

@blurymind Those settings you mention don't need to be "applied" since they are queried when needed. toogleNightMode() is needed the same way setTheme() is. Nice catch!

…ranquilmarmot/vscode-yarn-spinner

src/index.html

…ranquilmarmot/vscode-yarn-spinner

blurymind · 2020-06-05T16:16:06Z

Sorry to ask again, but can you update this to the latest version of yarnEditor please? @daviddq fixed a couple of nasty ones

…ranquilmarmot/vscode-yarn-spinner

TranquilMarmot · 2020-06-06T02:06:29Z

@blurymind alright, updated this and the extension PR!

blurymind · 2020-06-08T12:43:54Z

Do you feel confident this is ready to merge? Testing it, I couldn't find any regressions to Yarn

TranquilMarmot · 2020-06-08T17:26:06Z

I missed your comment about renaming this.updateVsCodeExtensionDocument to this.setYarnDocumentIsDirty; I'll do that later today and then this should be good to go

TranquilMarmot · 2020-06-10T02:40:33Z

Okay @blurymind this should be good to go now!

blurymind · 2020-06-10T09:04:37Z

Thank you for working on this 👍

Merged now 🎉

TranquilMarmot added 8 commits May 23, 2020 11:47

Fix accessing localStorage crashing VSCode webview

2360563

Add "getPublicPath" to Utils that will use a "window.getPublicVsCodeW…

68ce41b

…ebviewUri" function if it's available The VSCode extension will put this function on "window" when starting the editor, and calling it will return the proper URI to get an asset from the "public" folder.

Don't create start node if opening a file with VS Code extension

3242f84

Send save event to VSCode extension

6e9770f

Send message to extension when settings are changes

1e74549

Update some comments

d7463aa

More specific comment for Utils.getPublicPath

dd1fd9f

Lint

4012451

TranquilMarmot mentioned this pull request May 25, 2020

Embedded YarnEditor webview custom editor YarnSpinnerTool/VSCodeExtension#1

Closed

Fix #app positioning

4564968

TranquilMarmot commented May 25, 2020

View reviewed changes

TranquilMarmot force-pushed the tranquilmarmot/vscode-yarn-spinner branch from 4564968 to 4012451 Compare May 25, 2020 20:45

TranquilMarmot added 5 commits May 25, 2020 13:48

Fix #app positioning

e399311

Tell VSCode extension when document has changed

d97d13e

Fully rely on VSCode settings

585b6a5

Fix references to updateVsCodeExtensionDocument

4e5b6b3

Don't try and save if we're editing in VSCode

2b9ff93

TranquilMarmot commented May 25, 2020

View reviewed changes

TranquilMarmot added 4 commits May 25, 2020 14:17

Lint

4ad0be1

Don't attempt to send message to extension unless we're editing a file

c588396

openingVsCodeFile -> editingVsCodeFile

17cb77f

command, data -> type, payload

Mark document as dirty when changing node colors

b80f150

blurymind reviewed May 27, 2020

View reviewed changes

TranquilMarmot added 3 commits May 27, 2020 22:31

Fix markup language setting not being applied

856d55f

FIx crash when node styles are undefined

404aa85

Merge branch 'tranquilmarmot/vscode-yarn-spinner' of https://github.c…

7ef7e4c

…om/TranquilMarmot/YarnEditor into tranquilmarmot/vscode-yarn-spinner

Clean up if/else statement

f676bf1

TranquilMarmot added 2 commits May 28, 2020 20:48

Merge branch 'master' of github.com:YarnSpinnerTool/YarnEditor into t…

0488a35

…ranquilmarmot/vscode-yarn-spinner

Ability to open nodes in a text editor

9e5d250

blurymind reviewed May 29, 2020

View reviewed changes

src/index.html Show resolved Hide resolved

TranquilMarmot added 7 commits May 29, 2020 18:48

Clean up extension message listener

400b212

Add alwaysOpenNodesInVisualStudioCodeEditor option

4205b19

Lint...

265c9b8

Fix "Edit in Visual Studio Code Text Editor" showing up in web app

7ec91ae

Lint

8d99f8f

Only update VSCode document when dragging ends

b398e6e

Merge branch 'master' of github.com:YarnSpinnerTool/YarnEditor into t…

64156b9

…ranquilmarmot/vscode-yarn-spinner

Merge branch 'master' of github.com:YarnSpinnerTool/YarnEditor into t…

8d693c0

…ranquilmarmot/vscode-yarn-spinner

updateVsCodeExtensionDocument -> setYarnDocumentIsDirty

84f9039

blurymind merged commit eb9537c into blurymind:master Jun 10, 2020

TranquilMarmot deleted the tranquilmarmot/vscode-yarn-spinner branch June 11, 2020 05:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes needed for VSCode webview extension #186

Changes needed for VSCode webview extension #186

TranquilMarmot commented May 25, 2020 •

edited

Loading

TranquilMarmot May 25, 2020

TranquilMarmot May 25, 2020 •

edited

Loading

blurymind May 28, 2020 •

edited

Loading

blurymind May 28, 2020

daviddq May 28, 2020

TranquilMarmot commented May 26, 2020

blurymind May 27, 2020

blurymind commented May 27, 2020 •

edited

Loading

TranquilMarmot commented May 28, 2020

blurymind commented May 28, 2020 •

edited

Loading

daviddq commented May 28, 2020

blurymind commented Jun 5, 2020

TranquilMarmot commented Jun 6, 2020

blurymind commented Jun 8, 2020

TranquilMarmot commented Jun 8, 2020

TranquilMarmot commented Jun 10, 2020

blurymind commented Jun 10, 2020

Changes needed for VSCode webview extension #186

Changes needed for VSCode webview extension #186

Conversation

TranquilMarmot commented May 25, 2020 • edited Loading

What is this?

Changes

Assets in the public folder

localStorage and settings

The "Start" node

TranquilMarmot May 25, 2020

Choose a reason for hiding this comment

TranquilMarmot May 25, 2020 • edited Loading

Choose a reason for hiding this comment

blurymind May 28, 2020 • edited Loading

Choose a reason for hiding this comment

blurymind May 28, 2020

Choose a reason for hiding this comment

daviddq May 28, 2020

Choose a reason for hiding this comment

TranquilMarmot commented May 26, 2020

blurymind May 27, 2020

Choose a reason for hiding this comment

blurymind commented May 27, 2020 • edited Loading

TranquilMarmot commented May 28, 2020

blurymind commented May 28, 2020 • edited Loading

daviddq commented May 28, 2020

blurymind commented Jun 5, 2020

TranquilMarmot commented Jun 6, 2020

blurymind commented Jun 8, 2020

TranquilMarmot commented Jun 8, 2020

TranquilMarmot commented Jun 10, 2020

blurymind commented Jun 10, 2020

TranquilMarmot commented May 25, 2020 •

edited

Loading

Assets in the `public` folder

`localStorage` and settings

TranquilMarmot May 25, 2020 •

edited

Loading

blurymind May 28, 2020 •

edited

Loading

blurymind commented May 27, 2020 •

edited

Loading

blurymind commented May 28, 2020 •

edited

Loading