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

NotebookWidgetFactory can set default toolbar #5363

Closed
futurist opened this issue Sep 22, 2018 · 7 comments · Fixed by #5370
Closed

NotebookWidgetFactory can set default toolbar #5363

futurist opened this issue Sep 22, 2018 · 7 comments · Fixed by #5370

Comments

@futurist
Copy link
Contributor

@futurist futurist commented Sep 22, 2018

I want to set my own toolbars when init a notebook widget, looked into the source and found it's hard coded in NotebookWidgetFactory.

Is there any way to do this right? or it's should be a feature?

@bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Sep 23, 2018

@futurist
Copy link
Contributor Author

@futurist futurist commented Sep 23, 2018

If I want replace the whole toolbar, instead of insert some?

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Sep 23, 2018

It looks like (a) you can't remove toolbar items with the toolbar api and (b) you're right that the default buttons are hardcoded in the notebook panel. Perhaps the default buttons should be abstracted out to a plugin? Would you like to help with a PR to do that? We can help you get started if you'd like.

@jasongrout jasongrout added this to the Future milestone Sep 23, 2018
@futurist
Copy link
Contributor Author

@futurist futurist commented Sep 23, 2018

Besides a plugin, I think the toolbar can be set:

1. add toolbar option in NotebookWidgetFactory.IOptions, like below code:

interface IToolbarItem {
    [name: string]: Widget;
}
let myToolbar: IToolbarItem[] = CreateMyToolbar();
let wFactory = new NotebookWidgetFactory({
    name: 'Notebook',
    toolbar: myToolbar,  // fallback to DefaultToolbar
    ... ...
});

2. add toolbar option in DocumentRegistry.IContext, like below definition:

readonly toolbar?: IToolbarItem[];

This enables different registry can have different toolbar, like in markdown context the toolbar will have h1, h2, bold buttons etc.

The 1 and the 2 can also be combined, the 1 mostly as fixed buttons like save, run etc, and the 2 is mostly dynamic.

Just a stone for the problem, point me if I'm missing something 😄

@bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Sep 23, 2018

you can't remove toolbar items with the toolbar api

Putting my Evil Hat on:

    let children = [...(panel.toolbar.children() as any)._source];
    for (let i = 0; i < children.length; i++) {
      panel.toolbar.layout.removeWidget(children[i]);
    }
    panel.toolbar.insertItem(666, '', evilButton);

problem is, you aren't guaranteed to "win" (other plugins can come along after you), so yeah, something a bit more structured might make sense.

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Sep 24, 2018

Putting my Evil Hat on:

Yep. I carefully chose my words: "toolbar api" :).

blink1073 added a commit that referenced this issue Sep 27, 2018
* NotebookWidgetFactory add toolbarItems option (#5363)

* add toolbarItems option to docregistry

* add test for docregistry/default

* make _defaultToolbarItems protected

* use _overrideToolbarItems to save from options

* options: overrideToolbarItems -> toolbarItems
@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Sep 28, 2018

It looks like (a) you can't remove toolbar items with the toolbar api

I was wrong. See #5395 (comment)

@blink1073 blink1073 mentioned this issue Sep 28, 2018
31 tasks
@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 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