-
Notifications
You must be signed in to change notification settings - Fork 135
fix: optimize images #1302
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: optimize images #1302
Conversation
TC-MO
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
vdusek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks 🙂
|
Docusaurus is/should be optimizing the images for us automatically, via https://docusaurus.io/docs/api/plugins/@docusaurus/plugin-ideal-image Can you provide some links where we have large images? Maybe we just need to configure it. Also, we could remove them from the git history, there are ways, as long as we are fine with rewriting all the git hashes, which sounds ok for this repository. I recall this one: https://rtyley.github.io/bfg-repo-cleaner/ |
@B4nan I didn't do proper research, my claims are based solely on the fact that I uploaded large images previously and smaller should be better 😄 I didn't realize there could be optimization already employed somewhere. Maybe it's not an issue for the reader then? But when I download the GIF from https://docs.apify.com/academy/scraping-basics-python/getting-links, it's 3.9 MB, which is the same size as in the repo. Not sure it that's getting optimized. I reduced it to 1.8 MB. It's probably a good practice to add optimized images to the repo, because the sizes could add up over time, but we're talking about small numbers, so I wouldn't make it a hard rule or stress very much about it - dunno. As far as my additions are concerned, it's savings <5MB. I don't think we should bother removing them from git history and rewriting the hashes for everyone because of this. |
It shouldn't be. But large size of the git repository is something to pay attention to - on the other hand, replacing large images with smaller ones will only make the git repo size larger unless we use a tool like BFG to remove them from the git history.
Well, that is a small image when it comes to resolution. I think docusaurus only deals with large resolutions, also not sure if it even considers gifs to begin with. |
B4nan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
anyway, lets merge this one, no problems with that
Yup, that's why I'd close this PR if I could verify that the optimization for users happen. TBH I almost did it, but then tried to download the GIF and see what's its production website size. |
Optimizing images of the Python course with optimizt. The original images will stay in the Git history forever, the damage has been done, but at least we can improve the future experience of the readers of the docs.
Not sure if we could automate this, I guess the only way to do it would be a sort of pre-commit check. I doubt there's a GitHub Action which would optimize images and then automatically rebase the branch to get rid of the old ones. Anyway, that's a challenge for a different day, this is just a small PR with a simple and straightforward manual improvement.