Closed Bug 1068365 Opened 5 years ago Closed 5 years ago

Make collapsed Remote Clients look grayed out

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 35
Tracking Status
firefox35 --- verified

People

(Reporter: nalexander, Assigned: nalexander)

References

Details

Attachments

(3 files)

Bug 1064304 restored functionality but does not match antlam's most recent mocks.  The latest has collapsed remote clients totally grayed out.

Visual at https://bugzilla.mozilla.org/attachment.cgi?id=8490383 and assets at https://bugzilla.mozilla.org/show_bug.cgi?id=1068051.
Perfect, thanks for filing this.

Attaching mock.
There are two major ways to achieve this.  One is to painstakingly
update UI elements every time we fetch a group.  I've done that.  The
other is to maintain expanded and collapsed layouts.  I went quite far
down that path; doing it is less pleasant than I expected for the
following reasons:

1) we have to update the view graphics depending on the client device
type anyway;

2) Android's view recycling does not track the expanded/collapsed state,
so we either manage the expanded states and have multiple group
types (see modern versions of SimpleExpandableListAdapter) or we
duplicate the work we're doing here;

3) our expanded and collapsed layouts are so similar that duplicating
them is worse than futzing with them in this manner.

rnewman: you looked at a lot of this already, might as well pile it
on.  Known issue: antlam's collapsed/expanded icons are "backwards".
Will fix as a second part of this ticket.
Attachment #8497154 - Flags: review?(rnewman)
Assignee: nobody → nalexander
Status: NEW → ASSIGNED
Comment on attachment 8497154 [details] [diff] [review]
Make collapsed remote clients look grayed out. r=rnewman

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

If it works, I'm happy.

::: mobile/android/base/RemoteTabsExpandableListAdapter.java
@@ +113,5 @@
> +        if (client.tabs.isEmpty()) {
> +            // If there are no tabs to display, don't show an indicator at all.
> +            deviceExpandedResId = 0;
> +        } else {
> +            deviceExpandedResId = isExpanded ? R.drawable.home_group_expanded : R.drawable.home_group_collapsed;

Is this any more pleasant if we have a top-level if (isExpanded) { } clause?
Attachment #8497154 - Flags: review?(rnewman) → review+
antlam: we need a call or assets on the arrow icons that represent expanded/collapsed state.

The choice we made earlier about the arrow orientation was that when expanded, the arrow points up because when you click it, it folds things up.  I think this follows Chrome's lead.

This makes sense to me, but means that we need to turn the arrows over.  I can do that pretty easily with a script; let me know if that's okay.
Flags: needinfo?(alam)
Attached file panel_sync_assets.zip
Are these the icons you need Nick?

Screenshots aren't loading for me..
Flags: needinfo?(alam) → needinfo?(nalexander)
https://hg.mozilla.org/mozilla-central/rev/1d7215725764
https://hg.mozilla.org/mozilla-central/rev/d927c0ba44ad
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Verified as fixed in
Build: Firefox for Android 35.0a1 (2014-10-03)
Device: Nexus 4 (Android 4.4.4)
Status: RESOLVED → VERIFIED
I've landed with the icons that I think look good.  (Words that scare any UX person, I'm certain.)  We can iterate in a new ticket if antlam feels the need.
We're good here.
Flags: needinfo?(nalexander)
You need to log in before you can comment on or make changes to this bug.