1068365 - Make collapsed Remote Clients look grayed out

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Nick Alexander :nalexander [he/him]]

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.

Comment 1

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_mob_syncpanel_tabs6.png

Perfect, thanks for filing this.

Attaching mock.

Comment 2

[Nick Alexander :nalexander [he/him]]

Attachment: Make collapsed remote clients look grayed out. r=rnewman

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.

Comment 3

[Nick Alexander :nalexander [he/him]]

antlam: screenshots.  Observe that the icon directions (or colors) are backwards.

https://www.dropbox.com/s/idj8b06y2yo60ju/Expanded.01.Expanded.png?dl=0
https://www.dropbox.com/s/yb4ab3nr8p8k96c/Expanded.02.All.Collapsed.png?dl=0

Comment 4

[Richard Newman [:rnewman]]

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?

Comment 5

[Nick Alexander :nalexander [he/him]]

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.

Comment 6

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/fx-team/rev/1d7215725764
https://hg.mozilla.org/integration/fx-team/rev/d927c0ba44ad

Comment 7

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: panel_sync_assets.zip

Are these the icons you need Nick?

Screenshots aren't loading for me..

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1d7215725764
https://hg.mozilla.org/mozilla-central/rev/d927c0ba44ad

Comment 9

[Cristina Madaras, QA [:CristinaM]]

Verified as fixed in
Build: Firefox for Android 35.0a1 (2014-10-03)
Device: Nexus 4 (Android 4.4.4)

Comment 10

[Nick Alexander :nalexander [he/him]]

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.

Comment 11

[Nick Alexander :nalexander [he/him]]

We're good here.