Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Update to jupyterlab 4 #110
Update to jupyterlab 4 #110
Changes from 2 commits
466cb53
91cb220
68387e8
aadc690
6331bf3
ecac50c
3009ef1
a76d35f
68b5951
aa57841
49da0eb
80df818
b5bc4eb
573e1d7
e09b57a
5e860ee
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed to bump this to get the canvas widget to properly compile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these deps were copy/pasted from the current cookiecutter, which is configured for jupyterlab 4. I did remove the jest and babel deps that are in the cookiecutter, because we're not using them anywhere (and don't need to as far as I can tell)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So there's a possibility that I added them when wasm-bundling wasn't as straightforward as it is now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
migration script missed
hatch-nodejs-version
, but the cookiecutter has it, so added it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want this? not sure. the cookiecutter has it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got a compilation error without this complaining about assigning a string output to a
HTMLButtonElement
, based on some google/SO searching, seemed like this would work. and it does now compile. though just realized I didn't test the button, will go do that now ...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm. ok, ya, i don't even see the fullscreen button any more, so i broke something here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it totally works yet. So let's not worry about this for this PR and fix it at a later time. If I can figure it out I'll update your PR.