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

Refine createWebview #44579

Closed
jrieken opened this issue Feb 27, 2018 · 5 comments
Closed

Refine createWebview #44579

jrieken opened this issue Feb 27, 2018 · 5 comments
Assignees
Labels
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented Feb 27, 2018

re #44466

/**
* Create and show a new webview.
*
* @param uri Unique identifier for the webview.
* @param column Editor column to show the new webview in.
* @param options Content settings for the webview.
*/
export function createWebview(uri: Uri, column: ViewColumn, options: WebviewOptions): Webview;
  1. It should only create the webview not show it (see output channel and terminal for reference)
  2. Should it ask for the title? How else do we represent it in the UI?
@mjbvz
Copy link
Collaborator

mjbvz commented Feb 27, 2018

I'll add back a title parameter.

Unsure about not automatically showing though. Definitely can't fit in the change for this iteration. The current model assumes that a webview is always backed by webview editor (although this editor could be in a background tab). When the editor is closed, either programmatically or by the user, the webview is destroyed. We can discuss this further in March as we work to finalize the API

(Also note that the current design assumes webviews will only be used in editors. If we want to support them elsewhere, such as in a tree view for example, we would need to change things up)

@mjbvz mjbvz added api webview Webview issues labels Feb 27, 2018
@octref
Copy link
Contributor

octref commented Feb 27, 2018

Automatically showing the webview feels more natural to me. Also see a similar API for chrome.tabs.create.

For 2 I think it should ask for a title and an icon on creation, if the default behavior is to present the webview immediately after creating it.

mjbvz added a commit that referenced this issue Feb 28, 2018
@mjbvz mjbvz added this to the March 2018 milestone Feb 28, 2018
@jrieken
Copy link
Member Author

jrieken commented Feb 28, 2018

Automatically showing the webview feels more natural to me

I am on the other side, you create your webview, compute and set all its state (like html which the create method doesn't take) and when you are ready you show it. For me this like calling appendChild after assembling a subtree of pieces.

@octref
Copy link
Contributor

octref commented Feb 28, 2018

Yeah, depending on the way you create the Webview. If you pass in all the initialization info during creation having creation and presentation together makes sense, but with the current API having creation separate from presentation definitely feels more natural.

However if that's the way to go having ViewColumn in the createWebview call does not make sense any more.

@mjbvz
Copy link
Collaborator

mjbvz commented Mar 23, 2018

Added title back. Our current model still assumes that a webview is much more like a OutputPanel than a TextDocument (which may not show anywhere in the UI). From our discussions, I believe this is what we agreed on

@mjbvz mjbvz closed this as completed Mar 23, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators May 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants