Crop the avatar/logo before upload with a native selection cropper#174
Merged
Conversation
Selecting an avatar or workspace logo now opens a crop dialog (drag + zoom) before uploading, so the image is framed the way it renders. The crop is performed client-side and the resized 512x512 result is what gets uploaded. This reworks the idea from #131 without its cropper dependency: vue-advanced-cropper was last released ~2 years ago and we did not want an unmaintained package for something this load-bearing. What we need is narrow (fixed 1:1, a circle/square mask, fixed-size output), so a small canvas-based cropper covers it: - imageCrop.ts: pure transform math (cover-fit, clamp, zoom, viewport->source). - ImageCropperDialog.vue: CSS-transform preview, pointer drag, wheel/button zoom, a ResizeObserver to measure the modal (no requestAnimationFrame timing hacks), and a canvas toBlob only on save. - PhotoUpload.vue: opens the cropper on file select; the mask shape follows the display shape (round avatar / square logo) instead of always being round. - crop_* strings added to all 15 locales. Also installs Pest browser testing (pest-plugin-browser + Playwright) and adds a browser test for the crop flow. TestCase only calls withoutVite() for non-browser tests, since browser tests need the real Vite assets to boot the SPA. The Pest browser server does not parse multipart uploads, so the test asserts the crop dispatches the correct upload request; endpoint persistence stays covered by ProfileUpdateTest.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
- Output a canvas-encodable mime (jpeg/png/webp, else png) and keep the File's name/extension in sync, so non-encodable input (gif/svg/heic) no longer ships PNG bytes mislabeled as the original type. - Clamp zoom to a maximum (8x cover) so scrolling in can't collapse the crop to a sub-pixel region. - Handle undecodable/zero-dimension images (@error + naturalWidth guard) with a crop_error message instead of a permanently-disabled Save. - Replace the str_contains(static::class) Vite heuristic with a dedicated BrowserTestCase ($fakesVite = false). - The browser test now decodes the dispatched blob and asserts a 512x512 image, and uses route(..., absolute: false) instead of a hardcoded path. - Remove tests/Browser/ProbeTest.php (committed debug scratch).
Replace the move-image-behind-a-fixed-frame model with a selection box over the fully visible (contain-fit) image: drag to move, corner handles to resize, locked to 1:1 for the square avatar/logo output. The default selection is inset so the crop outline is always visible, handles sit inside the frame so they are never clipped, and wheel/pinch zoom is swallowed. Reset drag state on open/close, guard pointer capture and multi-touch, and fail-safe the canvas encode. Update the crop copy in all 15 locales to match the selection model.
Add Vitest (test:unit) with exact-value tests for the crop math (containScale, defaultSelection, clampSelection, all resizeSelection corners, and mime/filename resolution), and wire it plus the Pest browser test into CI. The browser test now samples output pixels against a four-quadrant fixture, so it detects a blank, flipped, or wrong crop rather than only asserting a 512x512 canvas. Add composer test:all to run PHP + Vitest + browser tests locally.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Extract the shared PHP/Composer/Node/database/Passport setup into a composite action, and run the backend suite (Unit + Feature) and the frontend/e2e suite (Vitest + Playwright browser tests) as two parallel jobs. This isolates the flaky browser suite so it can be rerun on its own, surfaces failures by area, gives the growing e2e suite room to shard, and uploads Playwright artifacts on failure.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Move actions/checkout, setup-node, cache, and upload-artifact to their current majors (Node 24 runtime). Run the browser suite as a sharded matrix aggregated by an e2e-gate job, so the required status check stays a stable name as the shard count scales. Vitest runs once (shard 1). Ready for the incoming e2e test expansion.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Move checkout, upload-artifact, and download-artifact to their current majors so the release image build stops relying on the deprecated Node 20 runtime. Docker actions are left as-is (not flagged). Only runs on release tags, so it is not exercised by PR CI.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
paulocastellano
changed the title
The project's test strategy is backend + e2e only, so drop the Vitest setup that had been added for the crop math: the imageCrop.test.ts suite, vitest.config.ts, the dependency and test:unit script, the CI step, and the test:all reference. The crop math is exercised through the Pest browser test.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
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
1 participant
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
Adds a dependency-free image cropper so users frame their avatar (profile photo) and workspace logo before upload, replacing the abandoned
vue-advanced-cropperdependency proposed in #131.The whole image stays visible and a movable, resizable square selection box (1:1, since avatars and logos render in a square) marks the crop. Drag the box to move it, drag a corner to resize. The default selection is inset so the crop outline is always visible. The selected region is exported as a 512×512 PNG/JPEG/WebP; inputs the canvas can't encode (GIF/SVG/…) coerce to PNG.
It lives in
PhotoUpload, so both the profile photo (settings/profile) and the workspace logo go through the same cropper.How it's built
resources/js/lib/imageCrop.ts— pure, framework-free crop math: contain-fit scale, default selection, clamp-to-bounds, per-corner square resize, and mime/filename resolution.resources/js/components/ImageCropperDialog.vue— the modal: full image + selection overlay, pointer drag/resize with pointer-capture and multi-touch guards, drag state reset on open/close, wheel/pinch zoom swallowed, and a fail-safe canvas encode.Testing
A Pest browser end-to-end test selects a four-quadrant fixture, saves, and samples the output pixels — so it detects a blank, flipped, or wrong crop, not just a 512×512 canvas. (The project's test strategy is backend + e2e; there is no JS unit-test layer.)
CI
backendande2ejobs that share asetup-laravelcomposite action (no duplicated setup, no drift).e2eruns as a shard matrix aggregated by ane2e-gatejob, so the required status check keeps a stable name as the shard count scales.actions/*bumped to their Node 24 majors (checkout@v7,setup-node@v6,cache@v6,upload-artifact@v7), including the release image workflow.Merge note
The old
testsjob was split, somain's required status checks need to change tobackend,e2e-gate,quality(droptests). To scale the e2e suite later, bump thematrix.shardarray and the--shard=…/Ndivisor together — the gate and required checks don't change.