Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds initial WebView extension implementation #325

Merged
merged 4 commits into from
Nov 1, 2023

Conversation

tdstein
Copy link
Collaborator

@tdstein tdstein commented Oct 31, 2023

Intent

Adds an initial implementation of embedding the Assistant (Wizard / publish-ui) in a WebView. The UI is embedded in an iframe.

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change

Approach

Implementation is based on this documentation: https://code.visualstudio.com/api/advanced-topics/remote-extensions#option-1-use-asexternaluri

Known Issues

  • The embedded iframe does not fill the screen.
  • Re-running the command fails due to a duplicate port.

Note that this is just a work-in-progress implementation.

Screenshot 2023-10-31 at 1 00 13 PM

Next Steps

  • Implement port allocation
  • Implement workspace resolution to obtain the project directory.
  • Implement a button to trigger command when viewing a Python file. Set the entry point to the file

Automated Tests

Directions for Reviewers

  • Checkout
  • Open extension in separate workspace.
  • Run just configure, which will alias the executable. This assume the executable has already been build (i.e., just build in root).
  • Press F5 to launch debugger.
  • Open publishing-client root directory in new window.
  • Run command "Publisher: Open Assistant".

You should see a terminal launch the server. The UI should open in a separate panel.

Checklist

@tdstein tdstein marked this pull request as ready for review October 31, 2023 17:09
Copy link
Contributor

@mmarchetti mmarchetti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good next step. Can you create follow-on issues for letting the agent choose a port dynamically (to avoid conflicts) and enabling agent session auth?

@tdstein
Copy link
Collaborator Author

tdstein commented Oct 31, 2023

This is a good next step. Can you create follow-on issues for letting the agent choose a port dynamically (to avoid conflicts) and enabling agent session auth?

Added: https://github.com/rstudio/publishing-client/issues?q=is%3Aissue+is%3Aopen+label%3Apositron+

Copy link
Collaborator

@dotNomad dotNomad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why it is necessary to "Reload Webviews". I'm sure that is a temporary thing, but looking forward to seeing if we can move past that requirement.


const terminal = vscode.window.createTerminal();
terminal.show();
terminal.sendText(`publisher publish-ui test/sample-content/fastapi-simple --listen=${url} --skip-browser-session-auth`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was never able to get the publisher command to work even with just configure. I had to change this to just run to get it to run.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you recall if you ran just build from the root beforehand?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did, but could never get publisher to work. Not sure why.

content="default-src 'none'; frame-src ${uri} ${cspsrc} https:; img-src ${cspsrc} https:; script-src ${cspsrc}; style-src ${cspsrc};"
/>
</head>
<body style="padding: 0;">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay this is weird... pulling this down and running it I see the CSS, but it isn't being applied?

CleanShot 2023-10-31 at 16 01 22

I've tried re-running, hard refreshing, reloading the window, am I missing something that I need to do to get this CSS to be applied? If I edit the CSS in the dev-tools it gets updated, but no matter what I've done the iFrame starts small.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Did you call Publisher: Open Assistant again from the VSCode command pallet?

Copy link
Collaborator

@dotNomad dotNomad Nov 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did. After discussing with @tdstein we discovered this was because of:

Refused to apply inline style because it violates the following Content Security Policy
directive: "style-src 'self' https://*.vscode-cdn.net". Either the 'unsafe-inline' keyword
, a hash ('sha256-mHgUAqxc8GPXM/QhzkHrHujcrQTU9fd6ZjWz+icxKcA='), or a
nonce ('nonce-...') is required to enable inline execution. Note that hashes do not
apply to event handlers, style attributes and javascript: navigations unless the
'unsafe-hashes' keyword is present.

Going to address in a follow-up

@tdstein
Copy link
Collaborator Author

tdstein commented Nov 1, 2023

@dotNomad - I am merging so that I can keep moving forward. Let's follow up on the issue you ran into.

@tdstein tdstein merged commit d61d691 into main Nov 1, 2023
17 checks passed
@tdstein tdstein deleted the tdstein/vscode-add-webview branch November 1, 2023 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants