Improve cell toolbar tracker

[fcollonval]

References

Fixes #14481
Closes #14139
A new toolbar is generated when the active cell changes - this is the only way we will be able to address feature like #13153; or to be able to customize the button depending on the cell model (like the type or tags)
Fixes #14380

Code changes

• Pass the toolbar factory instead of a list of toolbar items
• Generate a toolbar each time the active cell changes.
• Various improvements; like remove usage of two classes.

User-facing changes

None

Backwards-incompatible changes

None

[jupyterlab-probot]

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

[fcollonval]

I think it is safe to be backported 4.0.x.

[krassowski]

I think the general direction is right, but this is what I got when trying it out locally. Reproducer:

• windowing mode: full
• command mode → press and keep b to insert cells
• scroll as cells get added
• see:

[fcollonval]

Thanks a lot for the review @krassowski

The last commit should fix the full windowing mode issue.

[blink1073]

@jupyterlab-bot please restart ci

[blink1073]

@jupyterlab-bot, please restart ci

[blink1073]

cc @jtpio 🎉 🚀

[krassowski]

One more rough edge with the latest commit. Scenario:

• windowing = full
• add cells
• select multiple cells
• scroll away and back
• click on another cell
• see no cell toolbar
• click on a few more cells
• see error in console

Reproduces reliably multiple times, in both Chrome and Firefox.

DOMException: Node.removeChild: The node to be removed is not a child of this node
    detachWidget index.es6.js:2687
    removeWidgetAt index.es6.js:2574
    removeWidget index.es6.js:2551
    onChildRemoved index.es6.js:2070
    processParentMessage index.es6.js:1869
    notifyLayout index.es6.js:1285
    processMessage index.es6.js:1265
    invokeHandler index.es6.js:412
    sendMessage index.es6.js:166
    set parent index.es6.js:928
    dispose index.es6.js:790
    _removeToolbar celltoolbartracker.ts:229
    _onActiveCellChanged celltoolbartracker.ts:133
    invokeSlot index.es6.js:555
    emit index.es6.js:513
    emit index.es6.js:112
    set activeCellIndex widget.ts:1421
    _evtMouseDown widget.ts:2371
    handleEvent widget.ts:1843
    onAfterAttach widget.ts:1895
    processMessage index.es6.js:1240
    invokeHandler index.es6.js:412
    sendMessage index.es6.js:166
    onAfterAttach index.es6.js:1960
    processParentMessage index.es6.js:1860
    notifyLayout index.es6.js:1285
    processMessage index.es6.js:1239
    invokeHandler index.es6.js:412
    sendMessage index.es6.js:175
    attachWidget index.es6.js:9749
    addWidget index.es6.js:9641
    addWidget index.es6.js:11265
    _addToMainArea shell.ts:1447
    add shell.ts:933
    open index.tsx:119
    _createOrOpenDocument manager.ts:671
    open manager.ts:435
    openOrReveal manager.ts:474
    execute index.tsx:700
    promise callback*execute index.tsx:700
    execute index.es6.js:364
    createNew index.ts:1891
    execute index.ts:1922
    execute index.es6.js:364
    onclick widget.tsx:259
    React 23
    renderDOM vdom.ts:82
    renderDOM vdom.ts:79
    onUpdateRequest vdom.ts:51
    processMessage index.es6.js:1206
    invokeHandler index.es6.js:412
    sendMessage index.es6.js:166
    onAfterAttach vdom.ts:59
    processMessage index.es6.js:1240
    invokeHandler index.es6.js:412
    sendMessage index.es6.js:166
    onAfterAttach index.es6.js:1960
    processParentMessage index.es6.js:1860
    notifyLayout index.es6.js:1285
    processMessage index.es6.js:1239
    invokeHandler index.es6.js:412
    sendMessage index.es6.js:166
    attachWidget index.es6.js:9749
    addWidget index.es6.js:9641
    addWidget index.es6.js:11265
    _addToMainArea shell.ts:1447
    add shell.ts:933
[index.es6.js:377](webpack://jupyterlab/node_modules/@lumino/messaging/dist/index.es6.js)
DOMException: Node.removeChild: The node to be removed is not a child of this node
    detachWidget index.es6.js:2687
    removeWidgetAt index.es6.js:2574
    removeWidget index.es6.js:2551
    onChildRemoved index.es6.js:2070
    processParentMessage index.es6.js:1869
    notifyLayout index.es6.js:1285
    processMessage index.es6.js:1265
    invokeHandler index.es6.js:412
    sendMessage index.es6.js:166
    set parent index.es6.js:928
    dispose index.es6.js:790
[index.es6.js:377](webpack://jupyterlab/node_modules/@lumino/messaging/dist/index.es6.js)

[fcollonval]

Rebasing to include #15286 to see if the latest issue still arises.

[krassowski]

Thank you @fcollonval it does seem to work well now.

I wonder if the toolbar could live outside of cell (be parented to notebook, not cell) and positioned manually to avoid performance penalty of reattaching it to DOM on cell change, but this is less of a problem with windowed notebooks and a separate conversation.

[krassowski]

@meeseeksdev please backport to 4.0.x