Skip to content
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

Pinch to zoom: see if we can align to top/center for taller than 9:16 devices (currently it aligns to top/left) #653

Open
mzorz opened this issue Mar 18, 2021 · 1 comment
Labels
[Type] UI Bug The app works, user can complete their task, but the UX is not as we designed it to be.

Comments

@mzorz
Copy link
Contributor

mzorz commented Mar 18, 2021

With devices taller than 9:16 and images with a width equal or greater than the device's screen width, we're causing the pivot point in PhotoView library introduced in #651 to be anchored at the top-left of the screen (by using FIT_START).

IMG_1187.MOV

This behavior comes from this part of code:

                    if (isScreenTallerThan916(screenWidth, screenHeight) &&
                            (drawable.intrinsicHeight > drawable.intrinsicWidth)
                    ) {
                        val transformToUse = if (drawable.intrinsicWidth >= screenWidth) {
                            // this aligns to top so there's no top black bar
                            photoEditorView.source.scaleType = FIT_START
                            null
                    } [...]

The thing is on Android we can’t really anchor the image to top|center but only to either top|left or top|right as per the predefined ScaleType (see FIT_START, FIT_END). We could probably override this but it’d involve programming the scaling behavior ourselves to anchor to the center.

@mzorz mzorz added the [Type] UI Bug The app works, user can complete their task, but the UX is not as we designed it to be. label Mar 18, 2021
@mzorz
Copy link
Contributor Author

mzorz commented Mar 18, 2021

As an alternative, I thought of disabling pinch to “zoom out” when the condition to use FIT_START is met (this is only for screens taller than 9:16 using an image whose width is equal or larger than the device’s screen width - that image is 1080x1920 and the Pixel 5 is 1080x2340, hence it falls into this case where we'd want to be “aligning to top” but we’re using FIT_START for that as we can’t “center to top” as explained).

I tried enabling / disabling zoom attaching the call to a ScaleChangedListener but the thing is it internally resets the matrix each time, so decided to file the issue here for later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] UI Bug The app works, user can complete their task, but the UX is not as we designed it to be.
Projects
None yet
Development

No branches or pull requests

1 participant