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

Implement support for converting values to GIFs #232

Merged
merged 4 commits into from
May 30, 2020
Merged

Implement support for converting values to GIFs #232

merged 4 commits into from
May 30, 2020

Conversation

jackfirth
Copy link
Sponsor Contributor

@jackfirth jackfirth commented May 20, 2020

Closes #231. This pull request only touches the HTML rendering implementation code; it has no tests and does not yet update the docs. I'd like to get some feedback on whether this is the right implementation approach before getting to tests and docs.

@jackfirth jackfirth changed the title Implement support for convertible GIFs. Implement support for convertible GIFs May 20, 2020
@jackfirth jackfirth changed the title Implement support for convertible GIFs Implement support for converting values to GIFs May 20, 2020
@jackfirth
Copy link
Sponsor Contributor Author

jackfirth commented May 21, 2020

Manually tested this on my machine and it works. But I don't know how to write an automated test for this, or which places in the documentation need to be updated. Suggestions greatly appreciated!

@samth
Copy link
Sponsor Member

samth commented May 21, 2020

Looking at the existing tests, I don't see anything that tests any aspects of the html rendering.

A number of places (core.scrbl, running.scrbl) seem to mention the supported formats.

There's also the --convert flag, which should maybe also accept gif, and the render-convertible-as property. I don't know if it makes sense to change all these, but it might be worth looking at.

@jackfirth
Copy link
Sponsor Contributor Author

I see a ++convert flag, but not a --convert one. The ++convert one already accepts gifs I think. From what I can decipher it's only the prop:convertible mechanism that doesn't accept gifs, images added with @image{} can be png, svg, or gif.

@jackfirth jackfirth marked this pull request as ready for review May 21, 2020 03:38
@jackfirth
Copy link
Sponsor Contributor Author

Changed render-convertible-as? to allow gifs. Skipping tests since manual testing seems to work and I have no idea how to set up html rendering tests.

@mflatt
Copy link
Member

mflatt commented May 21, 2020

Ok by me. Is it worth extracting the GIF width and height to include as attributes in the img?

@soegaard
Copy link
Member

soegaard commented May 21, 2020 via email

@jackfirth
Copy link
Sponsor Contributor Author

@mflatt Yes, that's a good idea. Will update the PR.

@jackfirth
Copy link
Sponsor Contributor Author

@mflatt Done!

@mflatt mflatt merged commit 4b3d3a8 into racket:master May 30, 2020
@jackfirth jackfirth deleted the gif-conversion branch May 30, 2020 23:03
samth added a commit to samth/scribble that referenced this pull request Jul 20, 2020
Repairs a problem with racket#232.
Relevant to racket/racket#3300.
samth added a commit to samth/scribble that referenced this pull request Jul 20, 2020
Repairs a problem with racket#232.
Relevant to racket/racket#3300.
jbclements pushed a commit that referenced this pull request Jul 24, 2020
Repairs a problem with #232.
Relevant to racket/racket#3300.

(cherry picked from commit 9415df2)
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.

Support for converting values to GIFs
4 participants