Closed Bug 987075 Opened 6 years ago Closed 6 years ago

Suggestion list should use the new visual refresh colors

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(tracking-b2g:backlog)

RESOLVED DUPLICATE of bug 995128
tracking-b2g backlog

People

(Reporter: noemi, Unassigned)

References

Details

(Whiteboard: ux-most-wanted)

Attachments

(3 files, 1 obsolete file)

Attached image 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)
Attached image modal.png
Assignee: nobody → pacorampas
Status: NEW → ASSIGNED
Blocks: 951604
No longer blocks: dialer-visual-refres
Attached file Patch in github (obsolete) —
Attachment #8396312 - Flags: review?(arnau)
Comment on attachment 8396312 [details] [review]
Patch in github

Looks goo to me, but asking for r+ to a peer
Attachment #8396312 - Flags: review?(arnau) → review+
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).
Attachment #8396312 - Flags: review?(etienne) → review-
Target Milestone: --- → 1.4 S6 (25apr)
Attachment #8396312 - Flags: review- → review?(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 :/
Attachment #8396312 - Flags: review?(etienne) → review?(anthony)
Ok. Rebase done.
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?
Attachment #8396312 - Flags: review?(anthony) → review-
> - 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.
Flags: needinfo?(anthony)
> - This doesn't address the problem in the call screen in comment 4.
It is fixed now.
Attachment #8396312 - Flags: review- → review?(anthony)
Duplicate of this bug: 994558
Blocks: 994991
Flags: needinfo?(anthony)
Whiteboard: ux-most-wanted
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).
Summary: [Dialer] Overlay.css should be just contained what is app specific → Suggestion list should use the new visual refresh colors
Comment on attachment 8396312 [details] [review]
Patch in github

Marking as r- per comment 11. Sorry to have made you rebase for nothing.
Attachment #8396312 - Attachment is obsolete: true
Attachment #8396312 - Flags: review?(anthony)
Depends on: 995128
Clearing the current target milestone since this issue is now blocked by Bug 995128
Target Milestone: 1.4 S6 (25apr) → ---
The highlight state has a background color very close to the highlighted color of the phone. Should we have another highlight color?
Flags: needinfo?(vpg)
(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.
Flags: needinfo?(vpg)
(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...
Duplicate of this bug: 1007103
I leave this bug because I don't know if is necessary to do something by the new bug: Bug 995128.
Assignee: pacorampas → nobody
Anthony, will there be something pending to be implemented here once Bug 995128 has been landed?. Thanks!
Flags: needinfo?(anthony)
blocking-b2g: --- → backlog
The patch in bug 995128 contains all the color changes, so let's close it as a duplicate.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: needinfo?(anthony)
Resolution: --- → DUPLICATE
Duplicate of bug: 995128
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.