Noemí Freire (:noemi) Reporter
	Description • 10 years ago
kanban-visual

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)

	Arnau March [:arnau] ( not working in Firefox OS anymore :( )
	Comment 3 • 10 years ago

Comment on attachment 8396312 [details]
Patch in github

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

	Etienne Segonzac (:etienne)
	Comment 4 • 10 years ago

Comment on attachment 8396312 [details]
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).

	Etienne Segonzac (:etienne)
	Comment 5 • 10 years ago

Comment on attachment 8396312 [details]
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 :/

	Paco Rampas [:paco]
	Comment 6 • 10 years ago

Ok. Rebase done.

	Anthony Ricaud (:rik)
	Comment 7 • 10 years ago

Comment on attachment 8396312 [details]
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?

	Paco Rampas [:paco]
	Comment 8 • 10 years ago

> - 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.

	Paco Rampas [:paco]
	Comment 9 • 10 years ago

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

	Anthony Ricaud (:rik)
	Comment 11 • 10 years ago

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).

	Anthony Ricaud (:rik)
	Comment 12 • 10 years ago

Comment on attachment 8396312 [details]
Patch in github

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

	Noemí Freire (:noemi) Reporter
	Comment 13 • 10 years ago

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

	Anthony Ricaud (:rik)
	Comment 14 • 10 years ago

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

	Victoria Gerchinhoren [:vicky]
	Comment 15 • 10 years ago

(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.

	Victoria Gerchinhoren [:vicky]
	Comment 16 • 10 years ago

(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...

	Paco Rampas [:paco]
	Comment 18 • 10 years ago

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

	Noemí Freire (:noemi) Reporter
	Comment 19 • 10 years ago

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

	Anthony Ricaud (:rik)
	Comment 20 • 10 years ago

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