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 double import breaking all splits #17

Merged
merged 8 commits into from
Jun 16, 2022
Merged

Conversation

christopher-besch
Copy link
Member

@christopher-besch christopher-besch commented Jun 15, 2022

Closes #14

The injected_dependency nodes don't get deleted properly; this has to be fixed before merging.

@kolibril13
Copy link
Member

Thanks for this contribution!
Reloading the import cell does not break the splitview widget anymore, however closing and re-opening the notebook will now make jupyter forget the css and javascript after these pr changes.

Screen.Recording.2022-06-15.at.16.02.01.mov

Only restarting the kernel of the reopened notebook and running the import brings back the javascirpt+css functionality.

@christopher-besch
Copy link
Member Author

Sounds like a couple of hours wasted...
Maybe a Jupyter Widget makes more sense in the end of you can cleanly add your own CSS and JS.

@christopher-besch
Copy link
Member Author

That's strange; I just tested this on my machine and I can't reproduce the issue...
I added some more debug logs. Could you reopen the notebook and paste the browser console output?

@kolibril13
Copy link
Member

Programs are behaving weirdly from time to time.
Here is the testing with the new version:

Screen.Recording.2022-06-15.at.17.19.37.mov

Interestingly, the bug does only occur after I re-run the import after reopening the notebook.

@kolibril13
Copy link
Member

And one more important thing to note:
This bug does not occur, when one restarts the kernel before closing the notebook

@christopher-besch
Copy link
Member Author

Ok, I know what the issue is. I'll fix it later today.

@christopher-besch
Copy link
Member Author

Could you try this again, @kolibril13?

@kolibril13
Copy link
Member

Thanks for the next update!
With this version, the widget does not show at all, only white space is remaining where the images should be displayed:
image

@christopher-besch
Copy link
Member Author

No idea how this slipped through the cracks...
Please try again.

@kolibril13
Copy link
Member

Really nice, now everything is working as expected.
Excellent work!

@christopher-besch christopher-besch marked this pull request as ready for review June 16, 2022 13:50
@christopher-besch
Copy link
Member Author

Feel free to merge this now.

@kolibril13
Copy link
Member

Looks great and does what it should do, thanks a lot for this contribution!

@kolibril13
Copy link
Member

image

There are two merge conflicts right now,
can you once merge the last main into this branch?

@christopher-besch christopher-besch merged commit 0f79bd8 into main Jun 16, 2022
@christopher-besch christopher-besch deleted the fix_double_import branch June 16, 2022 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rerunning import cell kills splits
2 participants