-
Notifications
You must be signed in to change notification settings - Fork 160
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
Default to Image/PNG output #638
Comments
PNG is definitely a better default for Plotly outputs, but maybe one of the other mime types makes more sense in other contexts? For the Github UI I don't think the HTML will ever be rendered, right? |
Html will be rendered sometimes, although we sanitize it prior to display so it depends on what it is. We also allow the view source option on the html which maybe useful. |
Currently nbdime uses the default JupyterLab logic here: nbdime/packages/nbdime/src/diff/widget/output.ts Lines 342 to 345 in 4bc7ae2
So it is hard to change the default mime type in a way that improves the ordering generally. Maybe the question becomes: Can we use some other UX to better present a diff of output changes to a user? Here there is plenty of room to be creative :) |
There is a use case for a MIME bundle where PNG is a fallback for an interactive HTML plot, so maybe the current behaviour of preferring HTML is not bad in itself. Similarly, MathML (HTML) for math equations as default and PNG/SVG as fallback - MathML is more accessible and better user experience. Could plotly possibly switch from |
We received feedback that sometimes Plotly diffs are empty, it seems that some of the output will be just javascript which is essentially always empty, however there is a png available. I think it would make more sense to default to
image/png
instead oftest/html
when available, or expose a way for the user to set a preference for which output to display when multiple are available.Here is an example of this behavior nicolaskruchten/nb_diff_test@0621dda
Current default
Preferred default
I am happy to help make a pr to expose this, but was hoping for some context as to the best path forward, setting or change the default? Is there a strong reason the current default exists?
cc @nicolaskruchten
The text was updated successfully, but these errors were encountered: