Support dynamic toolbar definition

[fcollonval]

References

This addresses mainly the error seen in
jupyterlab/retrolab#319 (comment)

it also allows to change the toolbar definitions from the settings without reloading

Code changes

The toolbar factory for document widget factory can return either a list of toolbar items or an observable list. When an observable list is returned, the toolbar is updated accordingly.

The toolbar definition built using the settings are returning an observable list.

User-facing changes

Toolbar definitions can be changed without reloading.

When changing the definition, toolbar items not added through the settings (like the debug button) won't be positioned as expected - a reload will be needed for that.

Backwards-incompatible changes

N/A

[jupyterlab-probot]

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link:

[fcollonval]

I'ld like to target 3.3.x for this one as the toolbar definition from settings has been backported there.

[fcollonval]

cc @jtpio

[jtpio]

I'll like to target 3.3.x for this one as the toolbar definition from settings has been backported there.

Sounds good, thanks!

[krassowski]

Changing ranks works beautifully but I could not get the disabled option to work - the item did not disappear neither after saving settings nor after reloading. I see that a filter on item.disabled was removed - was this intentional, or just an oversight when refactoring?

[fcollonval]

Thanks for the review @krassowski
This was indeed an unfortunate oversight. I improved the unit tests to cover rank and disabled status update.

[fcollonval]

@echarles would you mind reviewing this one? This is the one missing before releasing the 3.3 beta.

[echarles]

ranking and disabled work for me. Removing the complete item does not produce the expected result (item being removed), is it expected?

[echarles]

... but I may be misunderstanding the feature. Maybe the settings aims to mutate (rank, disable) the existing toolbar, but is not there to remove any of the existing items?

[fcollonval]

... but I may be misunderstanding the feature. Maybe the settings aims to mutate (rank, disable) the existing toolbar, but is not there to remove any of the existing items?

The settings for the toolbar is taking a similar approach as for the menus and the shortcuts; aka user settings is merged with the default one. Hence the behavior when the user setting is empty.

[echarles]

@meeseeksdev please backport to 3.3.x

[lumberbot-app]

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

git checkout 3.3.x
git pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -m1 1be14de9cb5bcac7465bc6a734ddee28443f6d34

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #11963: Support dynamic toolbar definition'

4. Push to a named branch:

git push YOURFORK 3.3.x:auto-backport-of-pr-11963-on-3.3.x

5. Create a PR against branch 3.3.x, I would have named this PR:

"Backport PR #11963 on branch 3.3.x (Support dynamic toolbar definition)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

[echarles]

@fcollonval I have merged this PR in master. Backporting to 3.3.x is giving conflict.

[fcollonval]

Thanks I'll do the backport

[github-actions]

Benchmark report

The execution time (in milliseconds) are grouped by test file, test type and browser.
For each case, the following values are computed: min <- [1st quartile - median - 3rd quartile] -> max.

The mean relative comparison is computed with 95% confidence.

Results table

Test file	large_code_notebook	large_md_notebook
open
chromium
actual	3930 <- [4124 - 4184 - 4263] -> 4452	2665 <- [2770 - 2815 - 2872] -> 2980
expected	3927 <- [4096 - 4158 - 4243] -> 4656	2616 <- [2748 - 2795 - 2846] -> 3029
Mean relative change	0.6% ± 0.7%	0.7% ± 0.7%
switch-from
chromium
actual	646 <- [698 - 730 - 894] -> 1000	456 <- [494 - 510 - 522] -> 684
expected	617 <- [702 - 737 - 898] -> 1121	461 <- [496 - 511 - 522] -> 1051
Mean relative change	-0.6% ± 3.9%	-1.1% ± 2.5%
switch-to
chromium
actual	305 <- [363 - 390 - 413] -> 462	252 <- [290 - 300 - 310] -> 345
expected	330 <- [377 - 403 - 420] -> 450	253 <- [288 - 297 - 309] -> 340
Mean relative change	-2.3% ± 2.3%	0.0% ± 1.5%
close
chromium
actual	556 <- [999 - 1038 - 1071] -> 1170	475 <- [503 - 519 - 535] -> 591
expected	558 <- [975 - 1017 - 1051] -> 1123	461 <- [496 - 517 - 543] -> 610
Mean relative change	5.2% ± 4.7%	0.3% ± 1.4%

Changes are computed with expected as reference.

[jtpio]

This still seems to be an issue with 3.3.0rc0, noticed when updating RetroLab to use the latest 3.3.0-rc.0 packages in jupyterlab/retrolab#351: jupyterlab/retrolab@dd750da

[jtpio]

We still have the workaround in the test on master too:

jupyterlab/packages/apputils/test/toolbar.spec.ts

Lines 310 to 314 in c7b14ee

	await settingRegistry.load(bar.id);
	// Trick push this test after all other promise in the hope they get resolve
	// before going further - in particular we are looking at the update of the items
	// factory in `createToolbarFactory`
	await Promise.resolve();

[fcollonval]

I tested locally without the workaround. And it worked. But I presume the trouble is on the CI, isn't it?

[jtpio]

The tests seem to be passing on the JupyterLab CI without the workaround: #12096

[jtpio]

But normally this is reproducible locally with retro with jupyterlab/retrolab#351 and without the workaround (jupyterlab/retrolab@dd750da)