987075 - Suggestion list should use the new visual refresh colors

Status: RESOLVED DUPLICATE of bug 995128

(Firefox OS Graveyard :: Gaia::Dialer, defect)

Description

[Noemí Freire (:noemi)]

Attachment: dialer.png

The modal launched when cliking on "3" (see dialer.png and modal.png screenshots) should link to Value Selector BB instead of including all the styles within overlay.css (it should be just contained what is app specific)

Comment 1

[Noemí Freire (:noemi)]

Attachment: modal.png

Comment 2

[Paco Rampas [:paco]]

Attachment: Patch in github

Comment 3

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

Comment on attachment 8396312 [details] [review]
Patch in github

Looks goo to me, but asking for r+ to a peer

Comment 4

[Etienne Segonzac (:etienne)]

Comment on attachment 8396312 [details] [review]
Patch in github

Looks like this patch is causing a severe regression :/

We also have an overlay menu (using overlay.css) in the call screen (for bt audio input).
And with this patch applied, trying to hang up a call will instead open the audio input overlay menu. So you can't hang up the call :/

One a more general note, I think we'd be better off adding specific classes for the dialer-specific tweaks we want to make (the resulting file should get shorter).

Comment 5

[Etienne Segonzac (:etienne)]

Comment on attachment 8396312 [details] [review]
Patch in github

Forwarding to Rik because part of the visual refresh.

Also this path will probably need rebasing now that bug 990003 landed, sorry :/

Comment 6

[Paco Rampas [:paco]]

Ok. Rebase done.

Comment 7

[Anthony Ricaud (:rik)]

Comment on attachment 8396312 [details] [review]
Patch in github

- This doesn't address the problem in the call screen in comment 4.
- I'm not sure to understand the purpose of this patch. Comment 0 is mentioning "Value Selector BB" but Dialer is not using that BB (maybe it should, maybe that's what bug 995128 is about). Can you describe what you are trying to do here?

Comment 8

[Paco Rampas [:paco]]

> - This doesn't address the problem in the call screen in comment 4.
Sorry, it was my fault. Before doing the rebase it was working. I'm going to fix it.

> Can you describe what you are trying to do here?
It is to fix the suggestion modal screen. Because the list items don't start at top and don't finish at bottom of the container list. Also, the colors of suggestion items are different with the visual refresh. 

The initial idea for this bug was to link to Value selector BB instead of using Action menu, we could go back to that, but we should use a <select> to populate the content, which means redoing the logic.

Please let me know if you prefer that, continue with the PR as is or wait for a Action menu like VD as you propose in bug 995128.

Comment 9

[Paco Rampas [:paco]]

> - This doesn't address the problem in the call screen in comment 4.
It is fixed now.

Comment 11

[Anthony Ricaud (:rik)]

I think we should go ahead and use action menu for this bug because that's a UX most wanted. But action menus don't use <select>. There's certainly a bit of logic to change but we would have done it anyway. I'll work on bug 995128 tomorrow. Then this bug will just be about using the new colors (I think).

Comment 12

[Anthony Ricaud (:rik)]

Comment on attachment 8396312 [details] [review]
Patch in github

Marking as r- per comment 11. Sorry to have made you rebase for nothing.

Comment 13

[Noemí Freire (:noemi)]

Clearing the current target milestone since this issue is now blocked by Bug 995128

Comment 14

[Anthony Ricaud (:rik)]

Attachment: suggestion-list-active.png

The highlight state has a background color very close to the highlighted color of the phone. Should we have another highlight color?

Comment 15

[Victoria Gerchinhoren [:vicky]]

(In reply to Anthony Ricaud (:rik) from comment #14)
> Created attachment 8412157 [details]
> suggestion-list-active.png
> 
> The highlight state has a background color very close to the highlighted
> color of the phone. Should we have another highlight color?

Hi Rik,
Thanks for noticing it, but this hit state lasts just 1 second (or less) and it's just a feedback state, so it wouldn't be a problem, the hitstates are consistent across the system, so that's more important from a UX POV.

Comment 16

[Victoria Gerchinhoren [:vicky]]

(In reply to Victoria Gerchinhoren [:vicky] from comment #15)
> (In reply to Anthony Ricaud (:rik) from comment #14)
> > Created attachment 8412157 [details]
> > suggestion-list-active.png
> > 
> > The highlight state has a background color very close to the highlighted
> > color of the phone. Should we have another highlight color?
> 
> Hi Rik,
> Thanks for noticing it, but this hit state lasts just 1 second (or less) and
> it's just a feedback state, so it wouldn't be a problem, the hitstates are
> consistent across the system, so that's more important from a UX POV.

MOreover, shouldn't this be a value selector? It looks very wierd put as an action menu... just because it has information and not actions...

Comment 18

[Paco Rampas [:paco]]

I leave this bug because I don't know if is necessary to do something by the new bug: Bug 995128.

Comment 19

[Noemí Freire (:noemi)]

Anthony, will there be something pending to be implemented here once Bug 995128 has been landed?. Thanks!

Comment 20

[Anthony Ricaud (:rik)]

The patch in bug 995128 contains all the color changes, so let's close it as a duplicate.