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

🪴external rendermime management #632

Merged
merged 2 commits into from May 31, 2023
Merged

🪴external rendermime management #632

merged 2 commits into from May 31, 2023

Conversation

stevejpurves
Copy link
Member

in #630 I was discussing how to improve the current very buggy rendermime creation. I pushed though on option (3) [making rendermime registry creation an explicit part of the API] in this PR, and...

Things are immediately much better, specific improvements are:

  • much easier to reason about the expected ipywidgets behaviour
  • some confusing addWidgetRenderer calls in notebooks have been removed
  • the ThebeManager implementation is now just simple without the bizzare behaviour of adding widgets to some other rendermime.
  • the only place where a rendermime registry is completed is now in the PassiveCellRenderer which feels right as that class is intended for standalone usage on static content.

So this is a keeper 🤘

@stevejpurves stevejpurves changed the title 🪴external rendermime managment 🪴external rendermime management May 31, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 31, 2023

PR Preview Action v1.4.4
Preview removed because the pull request was closed.
2023-05-31 15:42 UTC

@rowanc1
Copy link
Member

rowanc1 commented May 31, 2023

This is awesome to see, really like the simplifications and explicit passing of the rendermime!

@stevejpurves stevejpurves merged commit 6eea92e into main May 31, 2023
4 checks passed
@stevejpurves stevejpurves deleted the fix/rendermime-api branch May 31, 2023 15:38
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.

None yet

2 participants