Compatibility with react-modal on iOS

[tudorpavel]

I couldn't find any previous discussion on this, so I will start one here.

Our use case involves adding ReactCrop inside a react-modal and we noticed that ReactCrop does not work properly on iOS when placed inside a modal. Basically, the crop rectangle disappears.

I'm not sure what could be causing this, but my guess is that on iOS Safari the getElementOffset code works unexpectedly because of the way the modal is positioned? I'm going to try to debug this further tomorrow.

I've modified the demo to include a modal for debugging purposes:
https://github.com/Camversity/react-image-crop/tree/debug-modal

Any thoughts on this?

[dominictobias]

Hi, are you running this on a mobile phone from a server by any chance?

[tudorpavel]

The demo? Yep, I'm using serve to open the demo on a mobile phone:

yarn global add serve
cd react-image-crop
serve -s .

And then using my local IP and port 5000 on the device.

[dominictobias]

Then please pull the latest master and npm install, otherwise you don't get ReactCrop.css because the relative path to it ../dist/ReactCrop.css in the demo index.html doesn't work when running a server. After that it should be fine in a modal.

[tudorpavel]

I did have that problem initially because I used serve with just the demo/ folder, but after that I made sure the CSS is loaded properly by serving the entire repo folder. The crop rectangle appears initially but the problems start when trying to move/resize it.

Regardless, we first noticed these issues in our app which uses create-react-app.

[dominictobias]

Hm strange, your fork works fine for me if I npm start and then open demo/index.html (without a server), even after moving, resizing etc

[tudorpavel]

Did you try it on an iPhone with Safari? It works fine everywhere else, as is usually the case with Safari... 😢

[dominictobias]

I did just try updating your branch against master so that it has the change in demo.js at the top import '../dist/ReactCrop.css'; and then I ran a simply python server python -m SimpleHTTPServer 3636 inside the demo folder. I moved it around and resized it etc:

[tudorpavel]

Hmm, interesting... It's true I tested it with 6.0.7 earlier when I was still in the office but now I don't have an iPhone with me.

I will check again tomorrow morning, thanks!

We can close this for now.

[tudorpavel]

To follow up, I have it working as expected in the demo. The popup in our app still behaves strangely on small screen (iPhone) even though it works as in the demo on large screen (iPad). I will have to investigate it further in our setup.

[tudorpavel]

I've finally gotten to the bottom of this... The weird behavior was in fact due to 2 issues:

1. My mistake from Wrong aspect ratio when setting all width, height and aspect #220: I was setting all width, height and crop. The solution was to set initial values only for width and crop, while setting height: null.

2. Issues with EXIF orientation discussed in rotate 90deg crop image on iPhone #205 and mentioned in the README (I should've RTFD), I was calculating the image width and height in memory with the uploaded File/Blob using this naive approach. I needed to do this before rendering ReactCrop. My photo was landscape in memory and portrait in ReactCrop due to EXIF orientation. The solution was to use blueimp-load-image to load the image in memory with orientation: true:

const getImageDimensions = (imageFile) => {
  return new Promise((resolve) => {
    loadImage(
      imageFile,
      (img) => {
        resolve({ width: img.width, height: img.height });
      },
      {
        orientation: true
      }
    );
  });
};

@dominictobias thanks for this library and thanks for your help!

[dominictobias]

Ok great glad you got it working 👍

…

On Thu, 15 Nov 2018 at 15:02, Tudor Pavel ***@***.***> wrote: I've finally gotten to the bottom of this... The weird behavior was in fact due to 2 issues: 1. My mistake from #220 <#220>: I was setting all width, height and crop. The solution was to set initial values only for width and crop, while setting height: null. 2. Issues with EXIF orientation discussed in #205 <#205> and mentioned in the README (I should've RTFD <https://www.urbandictionary.com/define.php?term=RTFD>), I was calculating the image width and height in memory with the uploaded File/Blob <https://developer.mozilla.org/en-US/docs/Web/API/Blob> using this naive approach <https://gist.github.com/schabluk/808f8691720d3a0e0e200a333bc61ecc>. I needed to do this before rendering ReactCrop. My photo was landscape in memory and portrait in ReactCrop due to EXIF orientation. The solution was to use blueimp-load-image <https://github.com/blueimp/JavaScript-Load-Image/> to load the image in memory with orientation: true: const getImageDimensions = (imageFile) => { return new Promise((resolve) => { loadImage( imageFile, (img) => { resolve({ width: img.width, height: img.height }); }, { orientation: true } ); }); }; @dominictobias <https://github.com/DominicTobias> thanks for this library and thanks for your help! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#219 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAuZ-ugwA7GKkEfJXBYE5yDFPkwkrOygks5uvYIOgaJpZM4YeOfW> .

[dominictobias]

Ps I’m thinking about maybe making the drag handles slightly larger for touch devices, if you have trouble dragging them let me know. Another guy was saying his users in user eating found it hard (they are only 9x9 atm I think) On Thu, 15 Nov 2018 at 15:08, dominic tobias <dominic.tobias@gmail.com> wrote:

…

Ok great glad you got it working 👍 On Thu, 15 Nov 2018 at 15:02, Tudor Pavel ***@***.***> wrote: > I've finally gotten to the bottom of this... The weird behavior was in > fact due to 2 issues: > > 1. > > My mistake from #220 > <#220>: I was > setting all width, height and crop. The solution was to set initial > values only for width and crop, while setting height: null. > 2. > > Issues with EXIF orientation discussed in #205 > <#205> and > mentioned in the README (I should've RTFD > <https://www.urbandictionary.com/define.php?term=RTFD>), I was > calculating the image width and height in memory with the uploaded > File/Blob <https://developer.mozilla.org/en-US/docs/Web/API/Blob> > using this naive approach > <https://gist.github.com/schabluk/808f8691720d3a0e0e200a333bc61ecc>. > I needed to do this before rendering ReactCrop. My photo was > landscape in memory and portrait in ReactCrop due to EXIF orientation. The > solution was to use blueimp-load-image > <https://github.com/blueimp/JavaScript-Load-Image/> to load the image > in memory with orientation: true: > > const getImageDimensions = (imageFile) => { > return new Promise((resolve) => { > loadImage( > imageFile, > (img) => { > resolve({ width: img.width, height: img.height }); > }, > { > orientation: true > } > ); > }); > }; > > @dominictobias <https://github.com/DominicTobias> thanks for this > library and thanks for your help! > > — > You are receiving this because you were mentioned. > > > Reply to this email directly, view it on GitHub > <#219 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAuZ-ugwA7GKkEfJXBYE5yDFPkwkrOygks5uvYIOgaJpZM4YeOfW> > . >

[tudorpavel]

Ps I’m thinking about maybe making the drag handles slightly larger for touch devices, if you have trouble dragging them let me know.

Sure, I guess it couldn't hurt to have them larger to improve the UX on touch devices. 👍

[osdiab]

@dominictobias assuming drag handles aren't larger yet, is there a way to configure them to be larger?

[dominictobias]

They are larger now but if you want to adjust them further the easiest way is to set some sass variables before? importing the file - https://github.com/DominicTobias/react-image-crop/blob/master/lib/ReactCrop.scss#L11 but you can also just override in CSS too. Probably the experience can be optimised further for mobile as discussed here - #201