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

fix(TabBar): Event forwarding fails when hosted in a ShadowRoot #276

Merged
merged 1 commit into from
Dec 28, 2021

Conversation

GordonSmith
Copy link
Contributor

Signed-off-by: Gordon Smith [email protected]

Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Just a couple of suggestions.

packages/widgets/src/docklayout.ts Outdated Show resolved Hide resolved
packages/widgets/src/tabbar.ts Outdated Show resolved Hide resolved
@blink1073 blink1073 added the enhancement New feature or request label Dec 26, 2021
@GordonSmith
Copy link
Contributor Author

@blink1073 FYI these changes are needed so that lumino widgets can be used within custom web components, which in turn can expose lumino widgets via "simple" html, see POC repo here (not currently published):

This should greatly simplify the use of lumino widgets within frameworks like React, Vue and Angular.

@blink1073
Copy link
Contributor

This should greatly simplify the use of lumino widgets within frameworks like React, Vue and Angular.

Very nice!

@blink1073 blink1073 merged commit 4e0f37e into jupyterlab:main Dec 28, 2021
@blink1073
Copy link
Contributor

@GordonSmith, are you ready for me to make a release?

@GordonSmith GordonSmith deleted the CUSTOM_WEB_COMPONENTS branch December 28, 2021 12:07
@GordonSmith
Copy link
Contributor Author

@blink1073 no real rush as I am building off local branches for now.

@GordonSmith
Copy link
Contributor Author

@blink1073 I might request a build if your not planning to do a release for a while?

Did you have any thoughts on the above examples?

@blink1073
Copy link
Contributor

The examples are cool! Happy to feature them as part of the https:/jupyterlab/lumino#external-examples if you want to submit a PR. I'll cut a release on Monday (I don't like to make releases on a Friday).

@@ -2184,7 +2193,7 @@ namespace Private {
renderer: DockLayout.IRenderer
): TabLayoutNode {
// Create the tab bar for the layout node.
let tabBar = renderer.createTabBar();
let tabBar = renderer.createTabBar(this._document);
Copy link
Member

@hbcarlos hbcarlos Jan 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this exist in this context?
This function is inside the Private namespace.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm updating lumino in JupyterLab, and there is an issue with this._document, when changing to a single-mode document.
jupyterlab/jupyterlab#11823

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example of the breaking code - I couldn't see anything related in the failing visual regression test?

Copy link
Contributor Author

@GordonSmith GordonSmith Jan 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix incoming.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: #281

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @GordonSmith, thanks for the quick reply.

Do you have an example of the breaking code

Yes, when in JupyterLab you change the view to a single-mode document, there is an error on this line:
https:/jupyterlab/jupyterlab/blob/3b21d568035f7383dfd88332e051277130aea7d8/packages/application/src/shell.ts#L557

Following the trace, you can see that the error comes from

let tabBar = renderer.createTabBar(this._document);

where this is null.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants