Gutenberg-mobile: [GlobalStep] Android - The Zoom indicator while editing an Image is incoherent.

Created on 8 Apr 2020  Â·  16Comments  Â·  Source: wordpress-mobile/gutenberg-mobile

Description

When editing an image using the Block Editor and using the zoom tool, a zoom percentage indicator is displayed. This value reflects the size of the image’s preview on the current screen and does not reflect the final image’s size in proportion to itself. This is immediately evident when changing the device’s orientation which causes a great disparity between zoom values even though the image size when publish is the same.

Reproduction Rate

4/4 100%

Expected behaviour

The displayed zoom value should reflect the image’s final size in proportion to the original size.

Actual behaviour

The displayed zoom value indicates the previewed image’s size while on the editing screen and does not reflect its final size.

Steps to reproduce the behaviour

  1. Install WordPress 14.6
  2. Login to a valid account.
  3. Create a New Post.
  4. Add an Image Block.
  5. Add an Image to the created Block.
  6. Press the button to Edit the Image.
  7. Select the zoom option.
  8. Change device’s orientation and observe the displayed Zoom value.
    Tested on the following

Samsung Galaxy S8+ (8.0)

Please see the attached video for more information

AndroidEditZoom.zip

Submitted by:

Luis Pimenta

Beta Request [OS] Android [Pri] High [Status] In Progress [Type] Bug

All 16 comments

@iamthomasbishop can you help out with design direction on this one? Should we make any changes based on potential points of confusion raised in this issue?

@ashiagr and @malinajirka - I'm not 100% sure what the scale number is representing.

Since it's used to reframe/scale the image and not to change the display size, I actually wonder if we should hide the number or reset it to 100% every time the edit screen is opened (if it's possible via the library).

Interesting! I haven’t seen this UI before so I also wasn’t sure what was meant by “scale” — it feels more like “zoom” to me.

What I would have expect here is a “zoom” scale that starts at 1 (or “Original”, or similar) and allows you to swipe right-to-left to to “zoom” the image in. As far as I can tell, you can’t scale the image “out” — it simply snaps to its original value when you try this.

@maxme Do we have control over this UI in a way that we can adjust the starting point, direction, and labeling?

đź‘‹ Hi there! Did a preliminary testing and found the issue exists in the uCrop (the external library we use for scaling) sample app as well. Image wasn't scaled yet it displayed 58% as the zoom level:

uCrop_zoom

We'll look into it in about a week.

I checked scale percent for images of different sizes in the library:

|Size | Scale Percent|
|-----|---------------|
| 2000x2000 | 49% |
| 1000x1000 | 99% |
| 500x500 | 198% |

It seems the library scales down the image if it is bigger in size and vice-versa as the image is fitted into a rectangle. It might look odd at first but it appears to be the right behaviour.

If it is confusing, I can override the text style from the library to hide the text. It will hide the rotate and scale text as both use the same style.

@iamthomasbishop , what do you suggest?

| Rotate | Scale |
|---|---|
| device-2020-04-24-144116_hide_rotate | device-2020-04-24-144054_hide_scale|

Thanks for looking into this @ashiagr ! ;)

It seems the library scales down the image if it is bigger in size and vice-versa as the image is fitted into a rectangle. It might look odd at first but it appears to be the right behaviour.

When you leave the library, does it save the original size of the image into a file or the scaled-down version?

  • I think the label would ideally display the size in which the image is going to be saved.
  • Moreover, I think it shouldn't scale down the image be default if it does. Imo in most cases the user will want to keep the original size as the post will be published to a web and some users will look at the image on a bigger screen.

I understand that customizability of the library is very limited. I'd just like to understand how it currently behaves and what an optimal solution would be. We can check if the optimal solution is feasible and adjust it accordingly later.

When you leave the library, does it save the original size of the image into a file or the scaled-down version?

It saves the original size of the image.

I think it shouldn't scale down the image be default if it does.

Library saves the original size.

I think the label would ideally display the size in which the image is going to be saved.

The scale "display" percentage is w.r.t. to device resolution. That's why it appears confusing.

| Pixel3a (1080x2220) | Pixel 3 XL (1440x2960) |
|---|---|
| pixel3a| pixel3xl |

Relevant piece of code from the library with matrix calculations for scaling:
https://git.io/JfLBs
https://git.io/JfLBT

I can spend more time to see if we can make it independent of device resolution, but it might take time, so suggested to hide the label.

Slightly modified just the scale "display" text logic in the library's forked repo here.

scalefix

Tested this image with size: 2000x2000 in the library's sample app by replacing the uri here.

Any inputs?

cc @malinajirka

Great job @ashiagr ! I like the solution and the code LGTM (I haven't tested the behavior). I'd suggest creating a PR against the original repo. I think the authors might like this approach more than the current behavior. If they don't merge it within a month or so we can consider using the forked repo. Wdyt?

I'd suggest creating a PR against the original repo. I think the authors might like this approach more than the current behavior. If they don't merge it within a month or so we can consider using the forked repo. Wdyt?

Agree, will create a PR against the orig repo.
EDIT: PR created here.

@ashiagr I'd like to close this even though the PR you submitted upstream at https://github.com/Yalantis/uCrop/pull/651 hasn't shipped because the work on our side looks done. What's your feeling about that? Okay to close? Is there anything that can be done to check back in on the open PR in the mean time?

@designsimply I'd preferably like to keep it open else will lose track of it. I can also target forked branch if everyone agrees.

cc @malinajirka

I can also target forked branch if everyone agrees.

I'm not sure what's our stand on targeting forked repos. It feels like we should at least fork it with a WordPress account (fork your repo so you don't lose the contribution). Wdyt @aforcier ?

I'm not sure what's our stand on targeting forked repos.

We've had to do this before, one example that comes to mind is Wellsql, used in FluxC: https://github.com/wordpress-mobile/wellsql. It's not ideal but kind of the only option if PRs aren't getting reviewed by the maintainers of the original library.

It feels like we should at least fork it with a WordPress account (fork your repo so you don't lose the contribution).

This sounds like the right move. For each change we needed to make to Wellsql we used to submit a PR to our fork, and also upstream.

So in short, how about if:

  1. We fork uCrop to wordpress-mobile
  2. @ashiagr re-creates her upstream PR on our fork, it's merged in
  3. We point our app to our fork of uCrop (might require setting up Bintray for that repo, like we did for Wellsql)

Sorry for a late response @aforcier , plan sounds good to me.
How do we fork uCrop to wordpress-mobile? Is it something you can help with?

cc @malinajirka

I've forked the repo into wordpress-mobile - https://github.com/wordpress-mobile/uCrop.

Was this page helpful?
0 / 5 - 0 ratings