Closed Bug 1184937 Opened 4 years ago Closed 4 years ago

Cycling through zoom levels vs having a default level for zoom window

Categories

(Firefox for Android :: General, defect)

42 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 42
Tracking Status
firefox42 --- fixed
fennec Nightly+ ---

People

(Reporter: krudnitski, Assigned: domivinc)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

I've been playing around with a test build (https://bugzilla.mozilla.org/show_bug.cgi?id=1180890#c2). 

Although interesting to change cycle through different zoom levels, I think simplicity is more important here (just because we can, doesn't mean we should). 

For the use case of trying to tap on the correct link, having that extra degree to fiddle with seems overkill. Can we just pick a reasonable default here?

I'm open to other opinions, but my opinion here is to make it simpler.
Flags: needinfo?(alam)
I'm up for removing this feature in the name of simplicity. It's a power user-feature to begin with and that was proven to be the case in the user studies we ran as well. 

Originally, this was also used to alleviate some concerns we had around how intuitive this was for users. But I think the animations that we've added since then really helps remedy some of that. 

I agree, I don't think it's completely necessary. And, I think removing that visual around the "bar" will help users discover another feature they've been known to love more, dragging!

mcomella, thoughts?
Flags: needinfo?(alam) → needinfo?(michael.l.comella)
(In reply to Anthony Lam (:antlam) from comment #1)
> I'm up for removing this feature in the name of simplicity. It's a power
> user-feature to begin with and that was proven to be the case in the user
> studies we ran as well. 

I'm fine removing it though I don't think we have heuristics to determine what is a good zoom level for a particular bit of text - it's possible the zoomed view just might not zoom in enough and be useless.

However, for the general case, it'd probably be fine - if I can read this text enough to click at it, then the zoomed view will probably just make it easier to read and click.

> Originally, this was also used to alleviate some concerns we had around how
> intuitive this was for users. But I think the animations that we've added
> since then really helps remedy some of that. 

I don't think zoom level configuration makes the zoomed view any more-or-less intuitive to users.

> I agree, I don't think it's completely necessary. And, I think removing that
> visual around the "bar" will help users discover another feature they've
> been known to love more, dragging!

How do you figure?

Dominique, what about you? Do you have any thoughts?
Flags: needinfo?(michael.l.comella) → needinfo?(domivinc)
It’s clear that a default value without any button could cover most of the user cases regarding a cluster of links. We can also put this default value in about:config, in order to allow the user to choose the default. 

But I would prefer to add 2 gray buttons “-” and “+” (like in the desktop version). And remove the limitation of the 3 different values (150%; 200%; 300%). We can start from 150% and add +100% each time the user clicks on the “+” button.
It will help to get details on high resolution pictures for instance: see attached image for a mock-up. 
The sum of many small details could make the difference between 2 browsers ...
Flags: needinfo?(domivinc)
Using the magnifying glass on images is pretty damn cool. :)

Anthony, what do you think?
Flags: needinfo?(alam)
While that did make me lol (and it actually did!), I think the most value a feature like this can provide will be for "areas of clustered links" and I think we should stay true to that focus. We should simplify and concentrate our efforts towards bettering that experience since we know that to be the problem we want to solve here as well.

I think adding 2 buttons to that small touch area can create a lot of problems too. So, maybe we should focus on bettering this feature by simplifying it down and adding that setting in about:config. That way, we can get this in the wild and let users start testing it to get some feedback as well!
Flags: needinfo?(alam)
By default in this version (apk avalaible here [1]), no button to set the zoom factor. The default value of the zoom factor is available in about:config: “ui.zoomedview.defaultZoomFactor” = 2;

In order to be able to test the other version of the ui (display of the zoom factor with -/+ actions), I added another flag in about:config to flip between the 2 versions: “ui.zoomedview.simplified”. Set this flag to false in order to access the version with the zoom factor with -/+ actions. It will be easier to test, compare and get feedback on the 2 versions.
I also made a change on the zoom button in order to avoid the 2 small touch areas (-/+). All the zoom factor area is clickable, with 2 different actions based on the touch position (left / right) 

[1] http://rusez.com/apk/bug1184937/fennec-42.0a1.en-US.android-arm.apk
Attachment #8637302 - Flags: feedback?(michael.l.comella)
Attachment #8637302 - Flags: feedback?(alam)
Comment on attachment 8637302 [details] [diff] [review]
patch-21072015 3-Bug_1184937___Hide_cycling_zoom_button_and_set_a_default_level_of_zoom_in_config__r_mcomella.patch

Thanks for the build domivinc!
Attachment #8637302 - Flags: feedback?(alam) → feedback+
^ It takes a bit of getting used to but I think the simplicity of the "handle" bar and the 'x' is quite nice.
NI-ing to follow up. From a UX point of view, this is good to go and I think it should be in before we land the feature.
Flags: needinfo?(michael.l.comella)
Flags: needinfo?(krudnitski)
Let's get it into users' hands and gauge wider feedback.
Flags: needinfo?(krudnitski)
Assignee: nobody → domivinc
Attachment #8637302 - Flags: review?(michael.l.comella)
Attachment #8637302 - Flags: feedback?(michael.l.comella)
Attachment #8637302 - Flags: feedback?
Attachment #8637302 - Flags: feedback? → feedback-
I'll get to the review tomorrow.
Flags: needinfo?(michael.l.comella)
tracking-fennec: ? → Nightly+
Status: NEW → ASSIGNED
Comment on attachment 8637302 [details] [diff] [review]
patch-21072015 3-Bug_1184937___Hide_cycling_zoom_button_and_set_a_default_level_of_zoom_in_config__r_mcomella.patch

Review of attachment 8637302 [details] [diff] [review]:
-----------------------------------------------------------------

Looking good - just a few nits.

Thanks, Dominique!

::: mobile/android/app/mobile.js
@@ +421,5 @@
>  
>  pref("ui.zoomedview.disabled", false);
> +pref("ui.zoomedview.limitReadableSize", 8); // value in layer pixels
> +pref("ui.zoomedview.defaultZoomFactor", 2);
> +pref("ui.zoomedview.simplified", true); // Do not display all the zoomed view controls

Add a follow-up bug to determine whether we want the simplified version or not and then to remove these branches.

::: mobile/android/base/ZoomedView.java
@@ +59,5 @@
>      private static final int DELAY_BEFORE_NEXT_RENDER_REQUEST_MS = 2000;
>      private static final int OPENING_ANIMATION_DURATION_MS = 250;
>      private static final int CLOSING_ANIMATION_DURATION_MS = 150;
>      private static final float OVERSHOOT_INTERPOLATOR_TENSION = 1.5f;
> +    

nit: ws

@@ +64,3 @@
>      private float zoomFactor;
>      private int currentZoomFactorIndex;
> +    private boolean simplifiedUI;

nit: -> isSimplifiedUI

@@ +273,5 @@
>  
> +        changeZoomFactorButton.setOnTouchListener(new  OnTouchListener() {
> +            public boolean onTouch(View v, MotionEvent event) {
> +
> +                if(event.getAction() == MotionEvent.ACTION_UP) {

nit: `if (` here and elsewhere

::: mobile/android/base/resources/layout/zoomed_view.xml
@@ +28,5 @@
>              android:padding="12dip"
>              android:layout_alignLeft="@+id/zoomed_image_view"
>              android:textSize="16sp"
> +            android:textColor="@color/text_and_tabs_tray_grey"
> +            android:visibility="invisible"/>

Add a comment saying this is invisible but that is change by the preference "ui.zoomedview.simplified".
Attachment #8637302 - Flags: review?(michael.l.comella)
Attachment #8637302 - Flags: review+
Attachment #8637302 - Flags: feedback-
Blocks: 1189633
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/cc9121f8df37
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
You need to log in before you can comment on or make changes to this bug.