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

clone method for ABCWidgetFactory? #6044

Closed
jasongrout opened this issue Feb 27, 2019 · 5 comments
Closed

clone method for ABCWidgetFactory? #6044

jasongrout opened this issue Feb 27, 2019 · 5 comments

Comments

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Feb 27, 2019

When we "Create new view for X", the command

let child = docManager.cloneWidget(widget);

calls the document manager clone method:
/**
* Clone a widget.
*
* @param widget - The source widget.
*
* @returns A new widget or `undefined`.
*
* #### Notes
* Uses the same widget factory and context as the source, or returns
* `undefined` if the source widget is not managed by this manager.
*/
cloneWidget(widget: Widget): IDocumentWidget | undefined {
return this._widgetManager.cloneWidget(widget);
}
which calls the widget manager clone method:
/**
* Clone a widget.
*
* @param widget - The source widget.
*
* @returns A new widget or `undefined`.
*
* #### Notes
* Uses the same widget factory and context as the source, or throws
* if the source widget is not managed by this manager.
*/
cloneWidget(widget: Widget): IDocumentWidget | undefined {
let context = Private.contextProperty.get(widget);
if (!context) {
return undefined;
}
let factory = Private.factoryProperty.get(widget);
if (!factory) {
return undefined;
}
let newWidget = this.createWidget(factory, context);
this.adoptWidget(context, newWidget);
return newWidget;
}
which then creates a brand new widget. For notebooks, this means a new rendermime is created:
protected createNewWidget(
context: DocumentRegistry.IContext<INotebookModel>
): NotebookPanel {
let rendermime = this.rendermime.clone({ resolver: context.urlResolver });

However, there may be view state that should be shared among the views. For a cloned notebook view, I think it makes more sense for the rendermime to be shared. For example, we add a notebook-specific renderer to the rendermime for ipywidgets. We could share the rendermime if we knew it was a clone, rather than a brand new view.

Can we push the notion of 'clone' all the way down to the WidgetFactory creating views? We could even have a default implementation that just creates a new view, but then let specific widget factories override the clone method.

We do have an alternative for ipywidgets - we could attach the rendermime renderer to the context, which is shared between the views, and that way have access to the original document's renderer. However, this seems somewhat hacky, when perhaps you want the views to actually share some state.

@jasongrout jasongrout changed the title ABCWidgetFactory should have a clone method clone method for ABCWidgetFactory? Feb 27, 2019
@jasongrout jasongrout added this to the 1.0 milestone Feb 27, 2019
@afshin
Copy link
Member

@afshin afshin commented Feb 28, 2019

If anybody wants to work on this issue before @jasongrout gets to it, please ping here and we'll re-assign the issue.

@Madhu94
Copy link
Collaborator

@Madhu94 Madhu94 commented Mar 2, 2019

@afshin I'd like to try this.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Mar 2, 2019

Great!

I figured out a way to do the specific thing I was trying to do with ipywidgets, but I think it probably is still a nice thing to have a clone method on widget factories.

@Madhu94
Copy link
Collaborator

@Madhu94 Madhu94 commented Mar 3, 2019

Opened #6060 to address this

@lock
Copy link

@lock lock bot commented Aug 6, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related discussion.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants