Page MenuHomePhabricator

Add toggle button for the image cropper
Closed, ResolvedPublic3 Estimated Story Points

Assigned To
Authored By
Daimona
Aug 11 2021, 9:00 PM
Referenced Files
F34625807: image.png
Aug 30 2021, 8:16 PM
F34625804: image.png
Aug 30 2021, 8:16 PM
F34625788: image.png
Aug 30 2021, 8:16 PM
F34625792: image.png
Aug 30 2021, 8:16 PM
F34625784: image.png
Aug 30 2021, 8:16 PM
F34625797: image.png
Aug 30 2021, 8:16 PM
F34625786: image.png
Aug 30 2021, 8:16 PM
F34625790: image.png
Aug 30 2021, 8:16 PM

Description

The advanced options form has a visual annoyance when you zoom the image, as you cannot then click and drag to move around the image (the cropper will move instead).

The proposed solution is to add a ButtonSelectWidget, above the image, to toggle the cropper between 'move' and 'crop' modes (see docs). Additionally, a new ButtonWidget next to it should be added to update the transcription when you change the selection, without having to scroll up and hit "transcribe" again. This means that the existing transcribe button should not apply the crop, but rather OCR the whole image.

The move modes are:

  • 'crop': create a new crop box (default, and this must be the initial mode because otherwise it's not possible to start cropping)
  • 'move': move the canvas
  • 'none': do nothing

It is currently possible to switch between them by double-clicking on the crop box, but this is hard to discover so we should remove it.

Event Timeline

Thanks @Daimona, yes exactly. Here's a visual reference of how this could look with a ButtonGroupWidget (including a toggle button to "select area" grouped with a normal button to run the OCR on the selection):

Empty state
"Select area" is untoggled and "Transcribe selection" is inactive

Cropping_UI-1.jpg (1Γ—1 px, 464 KB)

Active state
"Select area" is selected/toggled, and "Transcribe selection" becomes active

Cropping_UI-2.jpg (1Γ—1 px, 413 KB)

Let me know what you think! cc @NRodriguez

We could switch to drag-mode 'move' rather than the default of 'crop'. This still moves the crop box when you're zoomed in and drag the crop box, but it moves the image when you drag outside the crop box. See the first buttons under the demo at https://fengyuanchen.github.io/cropperjs/

That's a great idea @Samwilson! I can provide updated visual reference if you want.

ldelench_wmf set the point value for this task to 3.Aug 18 2021, 11:32 PM
Samwilson updated the task description. (Show Details)

I've updated the description to explain what we talked about in the meeting the other day. I think I've captured it all.

Hi @Samwilson, thanks for updating the description - sounds great! To my understanding, this would be the visual reference for the updated UI:
(give or take non-OOUI assets + I used the button from cropper.js docs, with our colors though)

OCR-cropper.jpg (1Γ—1 px, 438 KB)

Let me know what you think. Thanks!

Samwilson added a subscriber: β€’ imaigwilo.

Thanks @nayoub that's great. Do you think it's worth adding a sentence above the buttons, e.g. "Drag a rectangle on the image below to transcribe only one section.", because otherwise it's a bit hard to tell that there's any interactivity there (the only hint is the cross-hairs cursor when hovering over the image).

Also, I've set it to disable the buttons (all three) until a rectangle has been made.

Moving the clipboard button up also avoids obscuring the text, as reported by @imaigwilo in T269818#7298154, so that's good.

PR: https://github.com/wikimedia/wikimedia-ocr/pull/57

Here's what the above PR is looking like:

InitialAfter dragging
Screenshot 2021-08-23 at 15-05-11 WikimediaOCR.png (485Γ—1 px, 105 KB)
Screenshot 2021-08-23 at 15-03-39 WikimediaOCR.png (447Γ—1 px, 75 KB)

Looks perfect, thanks @Samwilson! The one tiny change would be the wording to be "Drag a rectangle on the image below to transcribe only one area of the page" since the button says "Transcribe area", it would be more consistent for users.

@Samwilson @nayoub
I fail to mention this observation earlier, however:

  1. It was not easy for a user to deselect the transcribe area(rectangle). I had to click outside the rectangle 4 or so times to get rid of the rectangle. Would be nice to make that easier in case a user changes their mind about OCR section Vs whole page.
  2. When user clicks "Transcribe" to OCR a section. The page refreshes to top of the page, user has to scroll down to view transcribed text. Would be nice for the page focus to stay at the transcribe section.

Thank you.

Hi @imaigwilo, my testing has been completed for this ticket...

Compatibility Testing was performed on the following:
DesktopΒ  -- Firefox version 91 Windows 10 / Chrome version 92 Windows 10 / Microsoft Edge version 92 Windows 10 / Safari version 14 Mac OS 11 / Internet Explorer version 11 Windows 10 / Opera version 78.04093.112 Windows 10.
Mobile Devices -- Android version 11 Samsung FE20 phone / Android version 8.1 Galaxy A10 tablet / iOS version 14.5 iPad Pro 5th generation / iOS version 14.5 iPhone 12 Pro Max.

Screenshots of new code changes on various devices and browsers:
Chrome version 92 Windows 10:

image.png (954Γ—1 px, 172 KB)

Β Opera version 78.04093.112 Windows 10:

image.png (897Γ—1 px, 109 KB)

Β 
iOS version 14.5 iPhone 12 Pro Max:
image.png (872Γ—412 px, 216 KB)

image.png (873Γ—419 px, 210 KB)

iOS version 14.5 iPad Pro 5th generation / Portrait Mode:

image.png (873Γ—644 px, 233 KB)

iOS version 14.5 iPad Pro 5th generation / Landscape Mode:

image.png (697Γ—818 px, 257 KB)

Samsung FE20 phone using Android Version 11:

image.png (934Γ—480 px, 192 KB)

Β ____________________________________________________________________________________________________________________________________________________
Please Note the following items:
1:Β  Ticket Description says the crop mode button should be the default and initial mode (Ticket Description needs updating because it does not include the new Github change to the defaultΒ mode button):
Β Β Β Β According to Github the following change was made:
Β Β Β Β Per Sam Wilson:
Β Β Β Β I've also updated to @nayoub 's suggestion of starting in move mode, and requiring a click onΒ Β Β Β Β Β 
Β Β Β Β crop to start cropping.Β 


2:Β  Also in regards to the Move Modes listed in the Ticket Description it says β€œIt is possible toΒ switch between them by double-clicking on the crop box, but this is hard to discover. IfΒ someone does double-click, our new buttons should toggle in state.”
Β Β Β Β I was not able to duplicate this function when I double-clicked the crop box (it did not toggleΒ as mentioned in Description).
Β Β Β Β I don’t think its an issue but figured I should mention it just in case.


3:Β  If I select the Crop button, and then decide not to use the crop function, I am not able toΒ deselect or remove the rectangle…
Β Β Β Β Β I have no way of removing the rectangle or deselecting either of the mode functions unless I re-transcribe the whole page again (by selecting the Transcribe Whole Page button).


4: Β  Using Mobile Phones in Landscape Mode or Portrait Mode, or just using an Android Tablet in Portrait Mode only:Β Β 
Β Β Β Β Β Β Β When I select the Crop button β†’ then select an area to crop β†’ and then select TranscribeΒ Area button, you will notice the Copy the Clipboard button could use a little moreΒ Β Β Β Β Β Β 
Β Β Β Β Β Β white-spacing between the hi-lighted section and the Copy to Clipboard button.Β 
Β Β Β Β Β Β (Same issue with spacing is also occurring after selecting Copy to Clipboard button), seeΒ screenshots below:Β 

Samsung FE20 phone using Android Version 11:

image.png (423Γ—511 px, 60 KB)

Android version 8.1 Galaxy A10 tablet:

image.png (441Γ—547 px, 86 KB)

Thanks @Djackson-ctr for testing this ticket. Will now move ticket to done column for review

2: Also in regards to the Move Modes listed in the Ticket Description it says β€œIt is possible to switch between them by double-clicking on the crop box, but this is hard to discover. If someone does double-click, our new buttons should toggle in state.”
I was not able to duplicate this function when I double-clicked the crop box (it did not toggle as mentioned in Description).
I don’t think its an issue but figured I should mention it just in case.

Yes, sorry, this is my fault: the default behaviour of CropperJS is to have that double-clicking, so I added it to the description here. In the end, it turned out to be a bit weird to have both that and the new buttons (plus it was overly-complicating things for a feature that no one knew about anyway), so I removed the double-clicking and forgot to update the ticket.

This looks and works great! Thanks for incoorporating these changes @Samwilson and for designing along @nayoub