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

Default to Image/PNG output #638

Open
gwincr11 opened this issue Nov 18, 2022 · 5 comments
Open

Default to Image/PNG output #638

gwincr11 opened this issue Nov 18, 2022 · 5 comments

Comments

@gwincr11
Copy link
Contributor

gwincr11 commented Nov 18, 2022

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 of test/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
Screen Shot 2022-11-18 at 2 21 06 PM

Preferred default
Screen Shot 2022-11-18 at 2 21 34 PM

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

@nicolaskruchten
Copy link

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?

@gwincr11
Copy link
Contributor Author

gwincr11 commented Nov 18, 2022

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.

@vidartf
Copy link
Collaborator

vidartf commented Nov 24, 2022

Currently nbdime uses the default JupyterLab logic here:

let mt = this.rendermime.preferredMimeType(
data,
this.model.trusted ? 'any' : 'ensure'
);

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 :)

@krassowski
Copy link
Member

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 text/html to text/javascript (or application/javascript)? If model is not trusted it would then default to PNG (since JS is marked as not safe in javaScriptRendererFactory).

@ghotman
Copy link

ghotman commented Jan 26, 2023

#648

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

No branches or pull requests

5 participants