-
-
Notifications
You must be signed in to change notification settings - Fork 344
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.
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
Muliple instances of cropper in same page lead to performance problems #553
Comments
Oh that's interesting, if you inspect the HTML of the blinking one do you see some changes to the DOM flashing? Do you have some global state that is shared between the components that you are setting onChange or something? Although it's strange that I only see 1 of the others blinking 🤔 |
I made a quick test here and don't have the same issue: https://codesandbox.io/s/react-image-crop-demo-with-react-hooks-forked-znh5zf?file=/src/Cropper.tsx:4656-5414 Screen.Recording.2023-08-17.at.9.10.06.AM.movMy guess would be that you have some global state which is causing the other components to mount and re-mount |
Hi @dominictobias , thanks for the quick response. The problem starts when we have more than 30 croppers in the same page. I am attaching a new video just in case: However, I will keep digging in the mounting/re-render logic maybe that is the case. |
Interesting, I didn't realise you had so many on the page. My guess then would not be on the JS side but on the CSS side, specifically I think the Plz try this experiment: .ReactCrop__crop-selection {
box-shadow: none;
} Of course then you don't have the box shadow so it's hard to test as the flashing will be gone. But it would be interesting if it feels faster, and if a recording of the Performance tab in devtools is better. You could also turn off the marching ants animation: There is another technique to darken the image by having two images with a dark layer in between. This most likely uses more memory so I avoided it, but the tradeoff could be worth it if the performance of box-shadow is actually worse than a bit more memory 🤔 There is one other way I know using clip-path #530 but the performance of that was too bad. |
Thanks @dominictobias! I think that additionally there might be some redundant re-rendering related to our custom react logic. I will dig a bit into our code and check that as well, and see what we find there too. |
Hi! first of all thanks for this library it's been pretty useful for us.
We are running into some performance issues when we have multiple croppers in the same page and it makes it difficult to users to be able to crop images.
When there are multiple croppers in the same page, the cropper starts to blink and becomes difficult to manage.
We tried implementing lazy loading with React.lazy(), to see if we could get improvements, but the issue remains.
What would you suggest for us to do to avoid the cropper from blinking?
Attached a video to show blinking example:
blinking cropper.webm
The text was updated successfully, but these errors were encountered: