Closed Bug 855016 Opened 12 years ago Closed 12 years ago

[ouya] Make row items on about:home clickable using the gamepad action button

Categories

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

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 22

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(3 files, 2 obsolete files)

Clicking the "O" button on the gamepad while focus is on the row items (e.g. featured addons or restored tabs) does nothing. This patch activates the item. The patch depends on the GamepadUtils which is added in a patch on bug 854971 (pending review).
Attachment #729719 - Flags: review?(sriram)
Attachment #729719 - Attachment description: Patch → Part 1 - Make the item rows respond to the gamepad action button
I'm not sure if it's a good idea to reuse the action_bar_button drawable xml file for this; I can copy it into a new xml file if you prefer.
Attachment #729743 - Flags: review?(sriram)
Since I lack the requisite photoshop skillz, I filed a follow-up for getting a proper focused-highlight promo box background, and just re-used the pressed background for focus as well. It already responds to click events so I didn't have do anything there (I guess overriding onClick is better than setting an onClickListener because onClick gets run with the gamepad action button already).
Attachment #729745 - Flags: review?(sriram)
Comment on attachment 729745 [details] [diff] [review] Part 3 - Make the promo box focusable Review of attachment 729745 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me.
Attachment #729745 - Flags: review?(sriram) → review+
Comment on attachment 729719 [details] [diff] [review] Part 1 - Make the item rows respond to the gamepad action button Review of attachment 729719 [details] [diff] [review]: ----------------------------------------------------------------- LGTM.
Attachment #729719 - Flags: review?(sriram) → review+
The "part 2" patch makes the entire "more text" row highlightable. See action shots at [1] and [2]. I discussed this with sriram on IRC and he suggested I get feedback from ibarlow about this behaviour - it may be that the link there is supposed to emulate a normal web link, in which case the focus highlight should be a smaller rectangle around the text. Note also that my patch means that when the link is clicked on a touch device (e.g. a regular phone) the entire row will change color as it enters the "pressed" state. This may also be undesirable; sriram suggested that we might want to just change the text color instead. Personally I feel that trying to emulate a web link is going to be hard because we'll want the focus highlight to look like the spatial navigation highlight. The styles for that will change independently and it will be annoying to always keep these in sync. More importantly, the active click area is the entire row, not just the text (even on a phone with current nightly, you click activate the link by clicking the rightmost edge of the row) and I think the focus/press highlight should reflect that. [1] http://people.mozilla.com/~kgupta/tmp/highlight-addon.png [2] http://people.mozilla.com/~kgupta/tmp/highlight-more.png
Flags: needinfo?(ibarlow)
Talked to Ian about this today, and he also wanted the link itself to be focused, as opposed to the entire row. I'll update the patch.
Flags: needinfo?(ibarlow)
Update part 2 so that only the text piece has a background. I couldn't do this just in styles since the LinkTextView is the size of the whole row but I could do it in code by applying/removing a span on the text. This also keeps the behaviour on touch devices the same as it was previously - tapping on the more text links doesn't show any focus/pressed states.
Attachment #729743 - Attachment is obsolete: true
Attachment #729743 - Flags: review?(sriram)
Attachment #730156 - Flags: review?(sriram)
Small adjustment to part 1 to also check that event.getAction() == KeyEvent.ACTION_DOWN because the onKey method gets called on key up as well. Carrying r+
Attachment #729719 - Attachment is obsolete: true
Attachment #730303 - Flags: review+
Comment on attachment 730156 [details] [diff] [review] Part 2 - Make the more text links focusable and clickable (v2) Review of attachment 730156 [details] [diff] [review]: ----------------------------------------------------------------- This looks good to me. If there's a way to make it "wrap_content", with a "center" gravity, and have the focus in XML, I would be more happy. But then this UI is changing. Let's not do it now :D
Attachment #730156 - Flags: review?(sriram) → review+
Component: General → Theme and Visual Design
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: