Render related-project sidebar thumbnails at the Project crop ratio (#1416) - #1447
Open
eastagiletracker wants to merge 1 commit into
Conversation
The related-projects sidebar rendered gallery_image at 160x90 (16:9) while Project.cropping is locked to a 5:3 crop box (PROJECT_THUMBNAIL_SIZE 500x300). crop_corners applies the editor's box and scale_and_crop then center-crops it a second time to reach 16:9, silently trimming 6.25% of the height the editor framed. The admin preview only ever shows the 5:3 box, so the trim was invisible to editors -- the same defect the news detail render hit at 750x350. Render at 160x96 (5:3) instead: same width, no model change and no re-cropping of existing content. .project-sidebar-related-thumbnail is width:80px with height:auto, so the thumbnail is 3 CSS pixels taller and nothing else moves. Generalize test_news_crop_aspect.py into test_crop_aspect.py so the invariant covers every crop field rather than only news. Renders are matched by their box= expression and each expression maps to the ImageRatioField that defines its ratio, so the test reads the ratio off the model instead of hardcoding it. A crop box the test doesn't know about now fails instead of being skipped, which surfaced the previously unchecked member.person.easter_egg_crop (245x245 against a 245x245 crop -- correct, just uncovered). CropProcessorTests pins the pixel-level behavior through the real crop_corners -> scale_and_crop chain.
This file contains hidden or 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
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.
This PR proposes rendering the related-projects sidebar thumbnail at the
Project.croppingaspect ratio so the editor's crop box is no longer silently re-cropped, and generalizing the crop-ratio guard test from news images to every cropped image in the templates (Fixes #1416). We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/233. You can sign in with your GitHub ID to claim ownership of the project.What was wrong
website/templates/website/project.htmlrendered the related-projects sidebar thumbnail at160x90(16:9) whileProject.croppingis anImageRatioFieldlocked toPROJECT_THUMBNAIL_SIZE=500x300(5:3). Becausecrop_cornersapplies the editor's box first and easy_thumbnails'scale_and_cropthen forces the target ratio, the chosen crop got a second center-crop — trimming its top and bottom, exactly the class of defect the news detail render (750x350) hit.Reproduced on
masterat8a43c0f4by driving your own processor chain in the project's container, with the top and bottom edges of the editor's box painted red and blue so anything trimmed is visible:So 6.25% of the height an editor deliberately framed never reached the page, and the admin preview gave no hint of it.
The change
160x90becomes160x96— same width, 5:3, no model change and no re-cropping of existing content, as the issue proposes..project-sidebar-related-thumbnailiswidth: 80px; height: auto, so the rendered box is 3 CSS pixels taller and nothing else in the sidebar moves.website/tests/test_news_crop_aspect.pybecomeswebsite/tests/test_crop_aspect.pyand now enforces the invariant for every crop field rather than only news — the issue's "consider generalizing it to project renders too". Three things changed in it:box=expression, and each expression maps to the model field that defines its ratio (Project.cropping,Person.easter_egg_crop,Sponsor.icon_cropping, ...). The ratio is read off theImageRatioFielditself, so changingPROJECT_THUMBNAIL_SIZEcan never leave the test asserting a stale number.member.person.easter_egg_cropindisplay_person_snippet.html, which nothing had been checking. It renders at 245x245 against a 245x245 crop, so it is correct — it just was not covered.CropProcessorTestspins the pixel-level behavior the rule exists to prevent, running the realcrop_corners→scale_and_cropchain and asserting a 5:3 box survives a 160x96 render intact and loses both edges at 160x90. That turns the template scan from a convention into something the suite proves.The 1200x630 Open Graph exception is unchanged and now cross-references #1417.
How I verified it
python manage.py test website --settings=makeabilitylab.settings_testwas run on a clean checkout ofmasterat8a43c0f4and again on this branch, each against its own postgres:16. Identical outcomes: 767 tests before, 770 after (the three added here), with the same two failures intest_backup_status.AdminBackupWarningTestsin both runs — they want thebackup-statusvolume the compose stack mounts, so they are unrelated to this change and were already red. Nothing new went red.The new coverage is genuinely load-bearing in both directions. With the template left at
160x90, the generalized guard fails and names the render:and with
160x96in place all five tests in the module pass. The only image on the page whose bytes change is the related-projects thumbnail; everything else renders exactly as before.One thing I deliberately left alone:
ML_WEBSITE_VERSIONis untouched, since cutting the release is yours to time.How this was managed
This work was tracked on a board imported from this repository's own issues and pull requests — 1322 stories and 51 labels — with the fix carried by the imported #1416 story: Related-project thumbnail on project page clips the crop. The full board is at https://eastagiletracker.com/projects/233.
If you'd rather not receive contributions like this, reply
no-more-prson this pull request and we won't open any further ones on your repositories.Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com