Asynchronously detect image aspect ratios and apply proper attributes #15170
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
citizen428
approved these changes
Oct 25, 2021
pr-triage
bot
added
PR: partially-approved
Co-authored-by: Michael Kohl <citizen428@forem.com>
pr-triage
bot
added
PR: unreviewed
benhalpern
commented
Oct 25, 2021
citizen428
approved these changes
Oct 26, 2021
pr-triage
bot
added
PR: partially-approved
jeremyf
approved these changes
Oct 26, 2021
maestromac
approved these changes
Oct 26, 2021
pr-triage
bot
added
PR: merged
citizen428
pushed a commit
that referenced
this pull request
Oct 29, 2021
…#15170) * Initial work to expand detection functionality * Finish up functionality and tests * Fix class name * Fix tests * Update spec/services/articles/enrich_image_attributes_spec.rb Co-authored-by: Michael Kohl <citizen428@forem.com> * Update spec/services/articles/enrich_image_attributes_spec.rb Co-authored-by: Michael Kohl <citizen428@forem.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this? (check all applicable)
Description
This PR builds on a couple past PRs which have made use of
FastImagefor detection of size and/or whether an image was animated.For ideal UX and SEO, it is best when
heightandwidthattributes are present on an image. This prevents cumulative layout shift, which is something users hate and so do search engine quality checkers. Plenty about that here:https://web.dev/optimize-cls/
Before/after videos below, but good examples of current behavior on DEV are the welcome threads where we use a gif: https://dev.to/thepracticaldev/welcome-thread-v147-4mkj
This PR extends an existing pattern we have in place to inspect images for whether they are animated gifs, and applies a check for height/width. It also refactors another place we put this functionality to put it all in one place.
This is performed asynchronously and changes are only made if there are images to change. While
FastImageis a great utility for querying for image meta data in the fastest/most minimal way, it is still a query we would rather do asynchronously.This functionality does current open an image twice in order to check for size and whether it is animated, but I looked through the
FastImagedocs and code, and it seems like this is the only way they provide via their API. If anyone wants to double check that, please feel welcome.Note: I left the feature flags in place which were added here: #13906 — I think we could disable them at some point if this has been deemed stable? Seems like the feature flag has just been on with DEV and things seem okay. Will need more info.
Related Tickets & Documents
https://github.com/forem/rfcs/issues/243
#13766
#14789
QA Instructions, Screenshots, Recordings
Before
before_detection.mov
After
after_detection.mov
Added/updated tests?