feat: 3332 - refactored the new crop page UI and added a camera

[monsieurtanuki]

New file:

• may_exit_page_helper.dart: Helper class about the "You're leaving the page with unsaved changes" case.

Impacted files:

• image_crop_page.dart: made method pickImageFile public
• new_crop_image.dart: added "camera" button to app bar, moved "rotate" button on app bar, moved "ok" button as FAB, added "exit without saving" dialog feature
• simple_input_page.dart: now using new class MayExitPageHelper

What

• This is a new step for Simplify adding photos by removing 2 of the 4 steps #3332: adding a camera button to the new crop page.
• That meant 3 buttons on bottom bar, which collided visually with the 3 bottom navigation tabs - I changed the UI to avoid the collision (cf. screenshot). I'm open to other suggestions.
• A new "exit without saving?" dialog feature has been introduced on this page, similar to what was in SimpleInputPage
• Of course it's still a bit clumsy - as we still have the old crop page as an option, it's not possible to remove the previous "camera", "edit", "confirm" page. We have to click on "OK" on the crop page to save our changes, and then to "confirm" them on the other page to save the changes to the server. That problem is already mentioned in Simplify adding photos by removing 2 of the 4 steps #3332 - to be solved when we get rid of the old crop page option.

Screenshots

light	dark

Part of

• Simplify adding photos by removing 2 of the 4 steps #3332

[monsieurtanuki]

@omkarChend1kar Would you please review it?

[teolemon]

• There's more ambiguity because the text next to the icons has been removed.
• Ok is not clear
• The UI does not allow to go forward the mockup I have proposed some time ago in Improve the Photo Manager using new features from openfoodfacts-dart #766

[omkarChend1kar]

@omkarChend1kar Would you please review it?

@monsieurtanuki Sure, Maybe after the requested changes.

[monsieurtanuki]

Hi @teolemon!

You're right, my suggested UI does not go towards your mockup.

In your mockup

• there are 4 features that are not supported now (unselect, select, change language, change field)
• the "OK" is not confusing there, because there's no "OK" button at all

If I implement something that goes towards your mockup, it will also look a lot like the page before the crop page, that we cannot get rid of as long as we keep the old crop tool "just in case".
For the record, page before the crop page:

As for do we need labels on buttons, that's probably a matter of taste. A matter of space too, especially with localizations.
In "my" solution, it's definitely not an issue as I land on a crop page with a "camera" button, a "rotation" button, and an "OK" button. The logos are explicit, standard, and even if they're not the user has only 3 buttons to try in a very specific context.

That said, I'm going to implement your suggested changes (aka buttons with labels), that will make it more obvious that we need to get rid of the old crop page and the additional "edit image" page.

[teolemon]

The big differences:

• it's one screen
• there's no "validate/ok" button
• no proeminent crop/rotate, especially since we don't have server side operations (it justs reuploads the same image, cropped, and the next user can't uncrop, correct mistakes, or even restart from the uncropped image)

Also, one question, is that supposed to be the rotate button ?

[teolemon]

If all other features are off the table, here's one version that:

• does not jeopardize the vision where you can take quick actions (such as unselecting, reporting)
• prepares the table for multilingual images
• reduces rotation time, and makes it clearer it is rotation
• The rotation buttons rotate the whole image

[monsieurtanuki]

Thank you @teolemon for your suggestions!

This is a screenshot from my latest commit:

For the record, according to flutter it did use a decent rotate right icon (but your icon choice is definitely more explicit):

[codecov-commenter]

Codecov Report

Merging #3402 (e57f610) into develop (282884a) will increase coverage by 0.02%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #3402      +/-   ##
===========================================
+ Coverage    10.32%   10.35%   +0.02%     
===========================================
  Files          260      259       -1     
  Lines        12595    12550      -45     
===========================================
- Hits          1301     1300       -1     
+ Misses       11294    11250      -44     

Impacted Files	Coverage Δ
packages/smooth_app/lib/pages/image_crop_page.dart	0.00% <0.00%> (-1.36%)	⬇️
...b/pages/preferences/user_preferences_dev_mode.dart	0.00% <0.00%> (ø)
...th_app/lib/pages/product/may_exit_page_helper.dart	0.00% <0.00%> (ø)
...th_app/lib/pages/product/product_image_viewer.dart	0.00% <0.00%> (ø)
...mooth_app/lib/pages/product/simple_input_page.dart	0.00% <0.00%> (ø)
...s/smooth_app/lib/tmp_crop_image/new_crop_page.dart	0.00% <0.00%> (ø)
...pp/lib/tmp_crop_image/rotated_crop_controller.dart	0.00% <0.00%> (ø)
...ckages/smooth_app/lib/tmp_crop_image/rotation.dart	2.50% <0.00%> (-0.36%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[teolemon]

Ok now we're back to interaction issues for saving, which was handled by the "OK" button (which added an extra step).

V1 autosaves new images, and subsequent crop/rotation is saved manually on a separate screen. On V1 We had a separate initial photo upload screen (with instant upload) with (client-side) cropping embedded, and a subsequent image manipulation screen with instant upload and server-side cropping/rotation… Crop/rotation happening server-side is to avoid loosing pictures, to generate training data for cropping, reduce server storage drain… We're trying to have the same screen optimize for 2 things.

• Initial: taking/uploading photo and cropping as quickly as possible. (What I'd like for V2 of the app is also multilingual support eventually)
• Subsequent photo editing: more operations (including multilingual, crop, unselection…)

At this point, and to go from first principles to this PR:

• Are changes instantly applied on upload ? crop ? on rotate ? or on closing the view ? what happens if clicking the navigation bar ?
• Here a solution is to save on view exit. We can a "Save" button in the top bar or below the take new photo button
• What about not showing the app navigation bar to reduce complexity ?

Honestly, I took 20 min writing this text, and I'm not happy with it, since it grossly misrepresents things. I'm not into writing novels for each feature/PR. Live and Collaborative fast iteration on images/mockups or a visio are worth a thousand words, and verbal iteration on what happens in each subcase, potential user reactions…

[monsieurtanuki]

Are changes instantly applied on upload ? crop ? on rotate ? or on closing the view ?

No server change is really applied on that screen. When we exit the screen, we provide the resulting file to the caller. That's the caller that decides to upload that file (with the "confirm" button).
The target would be to have the current screen uploading the file when clicking on "save" (or going back and being reminded that there are pending changes).

Again, this is due to the "old crop tool" / "new custom crop tool" dilemma. If we get rid of the old crop tool, we'll have a clearer view.
It looks like the new crop tool is not malfunctioning, so I'm in favor of discarding the old one altogether (especially given that we could not customize the old one at all, like adding buttons or uploading).

what happens if clicking the navigation bar ?

Good question, I haven't checked that one. It's probably not detected by WillPopScope as it's not a "back" gesture/click.

Here a solution is to save on view exit. We can a "Save" button in the top bar or below the take new photo button

That was more or less the purpose of the "OK" button. BUT, as said before, for historical reasons when we clicked on OK that would bring us back to the previous page with its "confirm" button.

What about not showing the app navigation bar to reduce complexity ?

#3337

[monsieurtanuki]

Honestly, I took 20 min writing this text, and I'm not happy with it, since it grossly misrepresents things. I'm not into writing novels for each feature/PR. Live and Collaborative fast iteration on images/mockups or a visio are worth a thousand words, and verbal iteration on what happens in each subcase, potential user reactions…

@teolemon In the visual approach, you simply ignore all the potential algorithm questions and contradictions that we coders have to deal with, like "When should we save to the server?", "What if the users forget to click on the save button?" or "What if the server rejects the cropped image?". That's not rendered on figma, is it?
Even in figma the visual questions like "what if the localizations make texts too big or too small", the aspect ratio on small / tall / big screens and the scrolling are not dealt with.
We need both a good UX and a good code underneath. You care more about the visual aspect, I care more about the code and the algorithms. Nothing wrong with that.

In the specific case of this PR, my initial point was just to add a working "camera" button. It became obvious that just adding the "camera" icon would create confusion with 2 rows of 3 icons on the bottom of the screen, and I had to change the UI accordingly. My point was not to say "This is the new UI direction!", my point was "I had to change the UI, change it as you wish but don't spend too much time on it as new features/new buttons/new UX/UI will come soon".

We're zig-zaging, but in the right direction.

That said, there are methods like Model–view–controller in order to separate the UI from the actual code.

[monsieurtanuki]

Latest commit screenshot:

TODO: remove "the previous page", which is very similar to the crop page without added value:

[monsieurtanuki]

Screenshot with the latest commit.
As far as #3332 went, the PR is complete.
Feel free to review and add suggestions.

product page	after a click on whatever picture

after a click on ingredient picture	after a click on edit

[teolemon]

Thank you very much @monsieurtanuki for the changes. We're improving the photo situation, bit by bit.

[monsieurtanuki]

Thank you @teolemon for your feedbacks!