Fix profile picture crop drift for off-centre crops#143
Merged
Conversation
applyProfilePictureCrop was combining object-position: P% Q% with a
matching transform-origin and scale(). That only rendered correctly for
centred crops — for any offset, object-position aligned the crop centre
with the element's P% point (not its centre), and the matching
transform-origin kept it pinned there under scale, so the rendered
picture appeared shifted and "more zoomed in" compared to the cropper
box, with non-crop image content bleeding in on the opposite edge.
Replace with translate+scale around the element centre:
tx% = -zoom · (W / L) · offsetX
ty% = -zoom · (H / L) · offsetY
where L = min(W, H). This puts the crop centre exactly at the container
centre for any aspect ratio. The helper now needs naturalWidth/Height,
so it defers via a one-shot addEventListener('load', …) when the image
isn't loaded yet — using addEventListener rather than .onload so callers
that wire their own pic.onload display toggle aren't clobbered.
Stored crop encoding (offsetX, offsetY, zoom) is unchanged; existing
data renders correctly without migration.
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
2 participants
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.
Description
Fixes a geometry bug in
applyProfilePictureCropthat caused rendered profile pictures to drift and appear "more zoomed in" than the cropper box when crops were off-centre.The previous implementation combined
object-position: P% Q%with a matchingtransform-origin: P% Q%andtransform: scale(zoom). This is only geometrically correct for centred crops. For off-centre crops,object-positionanchors the chosen image point at the element's P% position (not the container's centre), and the matchingtransform-originkeeps it there through the scale, causing the rendered crop to sit off-axis, get clipped on one side, and show image content from outside the crop box.The fix: Replace with the correct model using
translate(tx%, ty%) scale(z)where:tx% = -zoom·(W/L)·offsetXty% = -zoom·(H/L)·offsetYL = min(W, H)This makes the crop centre land exactly at the container centre for any aspect ratio. The helper now defers via a one-shot
loadlistener whennaturalWidth/naturalHeightaren't yet available, usingaddEventListener(not.onload) to avoid clobbering caller-wired load handlers.Type of Change
Checklist
Required for all code changes
npm testpasses)package.json,package-lock.json,version.json)CHANGELOG.mdhas been updated with a new entry under the correct versionIf adding or changing user-visible strings
If documentation-only change
Test Plan
Added comprehensive unit tests in
tests/frontend.test.jscovering:All tests verify the transform formula against the documented geometry contract.
https://claude.ai/code/session_01V6g4sngEajmbmKFm5GCsNg