1043029 - Active style for search history/suggestion items

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Search Activity, defect)

Description

[:Margaret Leibovic]

This is a follow-up to bug 1041738.

According to antlam's feedback, we should just change the background color of the card items when they are pressed. NI for some mocks :)

Comment 1

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

Attachment: prev_samvp_activestate.png

Let's give this color a go: #DCDCE1, it's a direct descendent of the background color.

Comment 2

[:Margaret Leibovic]

This is turning out to be trickier than I thought it would be. I put a WIP here, but it doesn't work right:
https://github.com/leibovic/FirefoxSearch/commit/6e3b118d52814f9c8bd98acd9a5263e01d266d1b

I only started by working on the search history list, not the search suggestions.

The main issue is that it seems we need to use the listSelector property to set a different background when a list item is selected, but that isn't interacting well with the background drawable we're using for the individual items.

Cc'ing lucasr in case he has some wisdom about list item selection styles :)

Comment 3

[Lucas Rocha (:lucasr)]

(In reply to :Margaret Leibovic from comment #2)
> This is turning out to be trickier than I thought it would be. I put a WIP
> here, but it doesn't work right:
> https://github.com/leibovic/FirefoxSearch/commit/
> 6e3b118d52814f9c8bd98acd9a5263e01d266d1b
> 
> I only started by working on the search history list, not the search
> suggestions.
> 
> The main issue is that it seems we need to use the listSelector property to
> set a different background when a list item is selected, but that isn't
> interacting well with the background drawable we're using for the individual
> items.

List selectors are mostly useful for keyboard navigation (when not in 'touch mode'), you don't necessarily need to use it for showing pressed state on individual items. Just use a background drawable that handles pressed state (i.e. selector drawable and focused state) on each list item and remove the list selector from your ListView.

Comment 4

[:Margaret Leibovic]

(In reply to Lucas Rocha (:lucasr) from comment #3)
> (In reply to :Margaret Leibovic from comment #2)
> > This is turning out to be trickier than I thought it would be. I put a WIP
> > here, but it doesn't work right:
> > https://github.com/leibovic/FirefoxSearch/commit/
> > 6e3b118d52814f9c8bd98acd9a5263e01d266d1b
> > 
> > I only started by working on the search history list, not the search
> > suggestions.
> > 
> > The main issue is that it seems we need to use the listSelector property to
> > set a different background when a list item is selected, but that isn't
> > interacting well with the background drawable we're using for the individual
> > items.
> 
> List selectors are mostly useful for keyboard navigation (when not in 'touch
> mode'), you don't necessarily need to use it for showing pressed state on
> individual items. Just use a background drawable that handles pressed state
> (i.e. selector drawable and focused state) on each list item and remove the
> list selector from your ListView.

Hm... I tried that originally but it wasn't working for me. Maybe I was missing something or using the wrong state, I'll try that again!

Comment 5

[:Margaret Leibovic]

Attachment: Pressed style for search history/suggestion items

Not sure what I was doing wrong the first time I tried this, but this does the job.

I still had to set the listSelector to transparent, because otherwise it was displaying a default selection color underneath.

Comment 6

[Eric Edens [:eedens]]

Comment on attachment 8464364 [details] [diff] [review]
Pressed style for search history/suggestion items

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

Looks good!

Comment 7

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/eed3189b2c66
https://github.com/ericedens/FirefoxSearch/commit/5c2bcb59310a7d35374480e78d489bdda275d097

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/eed3189b2c66

Comment 9

[:Margaret Leibovic]

Oops, forgot to hg add the new files:

https://hg.mozilla.org/integration/fx-team/rev/ac4d0d124429

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ac4d0d124429