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

Webview doc/sample feedback #48705

Closed
octref opened this issue Apr 25, 2018 · 3 comments
Closed

Webview doc/sample feedback #48705

octref opened this issue Apr 25, 2018 · 3 comments
Assignees
Labels
api verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@octref
Copy link
Contributor

octref commented Apr 25, 2018

Testing #48453

  1. Should distinguish watch/compile scripts (Distinguish between watch and compile vscode-extension-samples#63)

  2. The guide follows a "building-up" approach, so I think it should link to the yeoman generator for generating a skeleton project first. By only offering the full app, some people who get directly linked to this page might not be able to code along reading the docs.

  3. In the sample it's catCoding.start whereas in the doc it's catCoding.newCat. This makes it harder to copy-paste the code and be able to run it immediately.

Still Editing this issue. Not done yet.

All through with the doc.

@octref octref added the api label Apr 25, 2018
@octref octref added this to the April 2018 milestone Apr 25, 2018
@octref
Copy link
Contributor Author

octref commented Apr 25, 2018

  1. Why would user be interested in serializing / deserializing their app's state on dispose when there is retrainContextWhenHidden? As an extension author I would always use the most simple way to do the same thing. What's the incentive for them to store state and restore app from state?

  2. You need a section devoted to debugging webview, like how to open Webview devtools and how to Refresh pages I believe.

@mjbvz
Copy link
Collaborator

mjbvz commented Apr 26, 2018

Fixed 1 and 3.

I'm not sure about 2. This is an advanced api feature so the target audience is really small. If we create a simple webview boilerplate, we are sending the message that this API is something that we think many extension authors should use (which is not the case)

4 is covered in the docs. The section on retainContextWhileHidden explains that this option has high memory overhead. We cannot stop people for using this option just because it is convenient, but now we have documented what the cost of this choice is

For 5, there's already basic section on how to open the developer tools in the docs. I'll add a note on reloading too

mjbvz added a commit to microsoft/vscode-docs that referenced this issue Apr 26, 2018
@mjbvz mjbvz added the verification-needed Verification of issue is requested label Apr 26, 2018
@mjbvz
Copy link
Collaborator

mjbvz commented Apr 26, 2018

Thanks for the feedback. I believe the points have all been addressed (except for 2 which I'm not in favor or doing for the reasons outlined in my other comment)

@mjbvz mjbvz closed this as completed Apr 26, 2018
@octref octref added the verified Verification succeeded label Apr 27, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Jun 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

2 participants