Skip to content

Refactor profile picture image cropper #3060

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.

Sign up for GitHub

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

Open
samuelwei wants to merge 11 commits into
base: develop
Choose a base branch
from
Open

Refactor profile picture image cropper #3060

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e0a7d52
Replace vue-cropperjs with cropperjs
samuelwei Apr 15, 2026
63a8c5c
Refactor image copper style
samuelwei Apr 16, 2026
2f4acc0
Refactor file size validation, show file size and allowed formats in …
samuelwei Apr 20, 2026
4ef538c
Add keyboard support
samuelwei Apr 20, 2026
663d8df
Fix locale key
samuelwei Apr 20, 2026
ef7a558
Fix code style
samuelwei Apr 20, 2026
5d20eec
Prevent event propagation
samuelwei Apr 20, 2026
a3e6986
Improve file size calc.
samuelwei Apr 20, 2026
aadd6d9
Fix tests
samuelwei Apr 20, 2026
2b1ed92
Fix tests
samuelwei Apr 20, 2026
35480a3
Fix cs
samuelwei Apr 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
"axios": "^1.14.0",
"chart.js": "^4.5.1",
"chartjs-adapter-date-fns": "^3.0.0",
"cropperjs": "^1.6.2",
"date-fns": "^4.1.0",
"dompurify": "^3.3.3",
"dotenv": "^17.4.1",
Expand All @@ -76,7 +77,6 @@
"vue": "^3.5",
"vue-axe": "^3.1.2",
"vue-chartjs": "^5.3.3",
"vue-cropperjs": "^5.0.0",
"vue-i18n": "^11.3",
"vue-multiselect": "^3.5.0",
"vue-router": "^5.0.4"
Expand Down
48 changes: 48 additions & 0 deletions resources/css/app/_cropperjs.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
@import "cropperjs/dist/cropper.css";

.cropper-container {
overflow: hidden;

img {
display: block;

/* This rule is very important, please don't ignore this */
max-width: 100%;
}

.cropper-view-box,
.cropper-face {
border-radius: 50%;
}

.cropper-view-box {
outline: 0;
}

.cropper-face {
background-color: transparent;
opacity: 1;
border: 1px dashed #fff;
}

.cropper-line {
display: none;
}

.cropper-point {
background-color: #fff;
border-radius: 50%;
border: 1px solid #767676;
opacity: 1;
width: 10px;
height: 10px;
}

.cropper-point.point-n,
.cropper-point.point-w,
.cropper-point.point-s,
.cropper-point.point-e,
.cropper-point.point-se::before {
display: none;
}
}
1 change: 1 addition & 0 deletions resources/css/app/app.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@
@import "./_profile-image.css";
@import "./_color-select.css";
@import "./_stretched-link.css";
@import "./_cropperjs.css";
8 changes: 2 additions & 6 deletions resources/js/components/UserAvatar.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
<template>
<Avatar v-if="props.image" :image="props.image" :size="size" :shape="shape" />
<Avatar v-if="props.image" :image="props.image" :size="size" shape="circle" />
<Avatar
v-else
:label="avatarLabel"
class="select-none"
:size="size"
:shape="shape"
shape="circle"
/>
</template>

Expand All @@ -30,10 +30,6 @@ const props = defineProps({
type: [String, null],
default: null,
},
shape: {
type: String,
default: "circle",
},
});

const avatarLabel = computed(() => {
Expand Down
70 changes: 47 additions & 23 deletions resources/js/components/UserProfileImageSelector.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<FileUpload
v-if="!imageDeleted"
mode="basic"
accept="image/*"
accept="image/png, image/jpeg"
custom-upload
auto
:disabled="disabled"
Expand Down Expand Up @@ -67,8 +67,6 @@
:image="croppedImage ? croppedImage : image"
:alt="$t('admin.users.image.title')"
size="xlarge"
class="overflow-hidden rounded-border"
shape="square"
data-test="profile-image-preview"
/>
<UserAvatar
Expand All @@ -78,7 +76,6 @@
:lastname="lastname"
:alt="$t('admin.users.image.title')"
size="xlarge"
shape="square"
/>
</div>
</div>
Expand All @@ -101,7 +98,7 @@
severity="secondary"
:disabled="isLoadingAction"
data-test="dialog-cancel-button"
@click="modalVisible = false"
@click="closeModal"
/>
<Button
:label="$t('admin.users.image.save')"
Expand All @@ -112,22 +109,20 @@
</div>
</template>

<VueCropper
v-show="selectedFile"
ref="cropperRef"
class="my-2"
:auto-crop-area="1"
:aspect-ratio="1"
:view-mode="1"
:src="selectedFile"
:alt="$t('admin.users.image.title')"
/>
<div v-if="selectedFile" class="cropper-container my-2">
<img
:src="selectedFile"
width="100%"
id="cropper"

Check warning on line 116 in resources/js/components/UserProfileImageSelector.vue

View workflow job for this annotation

GitHub Actions / Frontend Code Style Check 

Attribute "id" should go before "width"
:alt="$t('admin.users.image.title')"
/>
</div>
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
</Dialog>
</template>

<script setup>
import { ref, watch } from "vue";
import VueCropper from "vue-cropperjs";
import { nextTick, ref, watch } from "vue";
import Cropper from "cropperjs";

const props = defineProps({
image: {
Expand Down Expand Up @@ -159,7 +154,7 @@
const isLoadingAction = ref(false);
const selectedFile = ref(null);
const croppedImage = ref(null);
const cropperRef = ref();
const cropper = ref();

watch(
() => props.image,
Expand All @@ -175,7 +170,7 @@
*/
async function save() {
isLoadingAction.value = true;
const oc = cropperRef.value.getCroppedCanvas({
const oc = cropper.value.getCroppedCanvas({
width: 100,
height: 100,
fillColor: "#ffff",
Expand All @@ -185,7 +180,7 @@
oc.toBlob((blob) => {
emit("newImage", blob);
isLoadingAction.value = false;
modalVisible.value = false;
closeModal();
}, "image/jpeg");
}

Expand All @@ -198,16 +193,45 @@
selectedFile.value = null;
}

function closeModal() {
if (cropper.value) {
cropper.value.destroy();
cropper.value = null;
}
modalVisible.value = false;
selectedFile.value = null;
}

async function onFileSelect(event) {
modalVisible.value = true;
isLoadingAction.value = true;
const file = event.files[0];

const reader = new FileReader();
reader.onload = (event) => {
reader.onload = async (event) => {
selectedFile.value = event.target.result;
cropperRef.value.replace(selectedFile.value);
isLoadingAction.value = false;

await nextTick();

cropper.value = new Cropper(document.getElementById("cropper"), {
aspectRatio: 1,
autoCropArea: 0.9,
background: false,
guides: false,
center: false,
rotatable: false,
zoomable: false,
movable: false,
viewMode: 1,
dragMode: "none",
ready: async function () {
isLoadingAction.value = false;

await nextTick();

document.querySelector('[data-test="dialog-cancel-button"]').focus();
},
});
};
reader.readAsDataURL(file);
Comment on lines +215 to 255
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Make async cropper initialization cancel-safe.

If the user cancels while FileReader/nextTick/Cropper ready is still pending, the stale callback can initialize Cropper with a null cropperImgRef and leave isLoadingAction stuck.

🐛 Proposed guard for stale initialization 
 const cropper = ref();
 const cropperImgRef = ref(null);
+const cropperInitToken = ref(0);
 function closeModal() {
+  cropperInitToken.value += 1;
+  isLoadingAction.value = false;
   if (cropper.value) {
     clearKeyboardShortcuts();
     cropper.value.destroy();
     cropper.value = null;
   }
@@
 async function onFileSelect(event) {
+  const initToken = ++cropperInitToken.value;
   modalVisible.value = true;
   isLoadingAction.value = true;
 
   const file = event.files[0];
 
   const reader = new FileReader();
   reader.onload = async (event) => {
+    if (initToken !== cropperInitToken.value || !modalVisible.value) {
+      return;
+    }
+
     selectedFile.value = event.target.result;
 
     await nextTick();
 
+    if (
+      initToken !== cropperInitToken.value ||
+      !modalVisible.value ||
+      !cropperImgRef.value
+    ) {
+      selectedFile.value = null;
+      isLoadingAction.value = false;
+      return;
+    }
+
     cropper.value = new Cropper(cropperImgRef.value, {
@@
       dragMode: "none",
       ready: async function () {
+        if (initToken !== cropperInitToken.value) {
+          return;
+        }
         isLoadingAction.value = false;
 
         initKeyboardShortcuts();
       },
     });
   };
+  reader.onerror = () => {
+    if (initToken === cropperInitToken.value) {
+      isLoadingAction.value = false;
+    }
+  };
   reader.readAsDataURL(file);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function closeModal() {
if (cropper.value) {
clearKeyboardShortcuts();
cropper.value.destroy();
cropper.value = null;
}
modalVisible.value = false;
selectedFile.value = null;
}
async function onFileSelect(event) {
modalVisible.value = true;
isLoadingAction.value = true;
const file = event.files[0];
const reader = new FileReader();
reader.onload = (event) => {
reader.onload = async (event) => {
selectedFile.value = event.target.result;
cropperRef.value.replace(selectedFile.value);
isLoadingAction.value = false;
await nextTick();
cropper.value = new Cropper(cropperImgRef.value, {
aspectRatio: 1,
autoCropArea: 0.9,
background: false,
guides: false,
center: false,
rotatable: false,
zoomable: false,
movable: false,
viewMode: 1,
dragMode: "none",
ready: async function () {
isLoadingAction.value = false;
initKeyboardShortcuts();
},
});
};
reader.readAsDataURL(file);
function closeModal() {
cropperInitToken.value += 1;
isLoadingAction.value = false;
if (cropper.value) {
clearKeyboardShortcuts();
cropper.value.destroy();
cropper.value = null;
}
modalVisible.value = false;
selectedFile.value = null;
}
async function onFileSelect(event) {
const initToken = ++cropperInitToken.value;
modalVisible.value = true;
isLoadingAction.value = true;
const file = event.files[0];
const reader = new FileReader();
reader.onload = async (event) => {
if (initToken !== cropperInitToken.value || !modalVisible.value) {
return;
}
selectedFile.value = event.target.result;
await nextTick();
if (
initToken !== cropperInitToken.value ||
!modalVisible.value ||
!cropperImgRef.value
) {
selectedFile.value = null;
isLoadingAction.value = false;
return;
}
cropper.value = new Cropper(cropperImgRef.value, {
aspectRatio: 1,
autoCropArea: 0.9,
background: false,
guides: false,
center: false,
rotatable: false,
zoomable: false,
movable: false,
viewMode: 1,
dragMode: "none",
ready: async function () {
if (initToken !== cropperInitToken.value) {
return;
}
isLoadingAction.value = false;
initKeyboardShortcuts();
},
});
};
reader.onerror = () => {
if (initToken === cropperInitToken.value) {
isLoadingAction.value = false;
}
};
reader.readAsDataURL(file);
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@resources/js/components/UserProfileImageSelector.vue` around lines 215 - 255,
The onFileSelect/FileReader/Cropper path can finish after the user cancels, so
make the async cropper initialization cancel-safe: when creating the FileReader
and in reader.onload (inside onFileSelect), check that modalVisible.value is
still true, selectedFile.value still matches the file, and cropperImgRef.value
is non-null before calling new Cropper; also store the FileReader instance so
closeModal can call reader.abort() (or set a cancellation flag) and ensure
closeModal clears isLoadingAction.value and prevents any late ready callback
from initializing cropper.value or calling initKeyboardShortcuts; update the
ready handler to re-check modalVisible and cropperImgRef before setting
isLoadingAction/value or calling initKeyboardShortcuts.

}
Expand Down
1 change: 0 additions & 1 deletion resources/js/views/AdminUsersNew.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,6 @@
</template>
<script setup>
import env from "../env.js";
import "cropperjs/dist/cropper.css";
import { useApi } from "../composables/useApi.js";
import { useFormErrors } from "../composables/useFormErrors.js";
import { useSettingsStore } from "../stores/settings";
Expand Down
Loading