Closed Bug 784395 Opened 7 years ago Closed 7 years ago

Make "Request Desktop Site" checkbox clearer

Categories

(Firefox for Android :: Theme and Visual Design, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 18
Tracking Status
firefox18 --- verified

People

(Reporter: ibarlow, Assigned: sriram)

References

Details

Attachments

(3 files, 1 obsolete file)

Attached image mockup
Our "Request Desktop Site" can be toggled on and off, but we don't make that clear enough to our users -- for one thing, we should be indicating that this is a toggle before users even touch it. We don't show an empty checkbox next to the menu item right now, so it isn't really clear that it works like an on/off switch. 

Attached is a design that adds an "off" state to the checkbox. 

--

Note: This should also affect the "Bookmark" menu item.
Should this be added for "Bookmark" too?
Yessir.
Attached patch WIP (obsolete) — Splinter Review
The images are bigger : 24x24. We've been using something like 20x18 so far. It's better to use a smaller one (or increase the width of the menu).

Also, this uses ImageView instead of changing to CheckBox, as the CheckBox tends to receive focus -- which means the check changes and entire row doesnt get touch event and things get worse.

A final patch will be ready with smaller images.
Depends on: 791076
OS: Mac OS X → Android
Hardware: x86 → ARM
Attached patch PatchSplinter Review
This patch changes the checkbox's size, adds an unchecked state, and fits it smaller for our custom menu.
Additionally there was a bug with checkable/checked state code, which is fixed now.
Attachment #653983 - Attachment is obsolete: true
Attachment #662703 - Flags: review?(mark.finkle)
Comment on attachment 662703 [details] [diff] [review]
Patch

>diff --git a/mobile/android/base/GeckoMenuInflater.java b/mobile/android/base/GeckoMenuInflater.java

>     public void setValues(ParsedItem item, MenuItem menuItem) {
>         menuItem.setChecked(item.checked)
>                 .setVisible(item.visible)
>                 .setEnabled(item.enabled)
>                 .setCheckable(item.checkable)
>-                .setCheckable(item.checked)
>+                .setChecked(item.checked)
>                 .setIcon(item.iconRes)
>                 .setShowAsAction(item.showAsAction);
>     }

setChecked is called twice in this code. I assume that setCheckable must be called first. If so, remove the first call to setChecked

r+, but fix this
Attachment #662703 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/4023d860c363
Assignee: nobody → sriram
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
Verified fixed:

Build: Firefox 18.0a1 (2012-09-20), Firefox 17.0a2 (2012-09-20)
Device: Samsung Galaxy Nexus
OS: Android 4.1.1
Status: RESOLVED → VERIFIED
Correction:

Build: Firefox 18.0a1 (2012-09-21)
You need to log in before you can comment on or make changes to this bug.