Closed Bug 912494 Opened 11 years ago Closed 11 years ago

Implement the conference call detail screen

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::Dialer
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: etienne, Assigned: rexboy)

References

Details

Attachments

(2 files)

the disclosure button
100.62 KB, image/png
Details
patch
188 bytes, text/html
etienne
: review+
Details 
Visuals:
https://bugzilla.mozilla.org/attachment.cgi?id=795485
https://bugzilla.mozilla.org/attachment.cgi?id=795934

What needs to happen:

* Add a "disclosure button" to the group call section [1]

* Make this button display the conference group detail screen (we need not to interfere with the hold toggle)

* Format the group call details div [2] properly. The handled calls nodes are already moved here automatically, but we need to style them.

* Add a hang up button to the handled calls node template [3] it should not be displayed when the nodes are in the call screen, only when they are under #group-call-details

* Make the button hang up the call :)

(the disconnect notification will be done in a separate bug)


[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/oncall.html#L61-72
[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/oncall.html#L74-75
[3] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/oncall.html#L48-60
Blocks: 887686
The visuals for this are very close to the suggestion bar popover, probably some factoring to do.

Rex, would you be available to work on this? :)
Blocks: 909236
ni? rexboy on comment 1
Flags: needinfo?(rexboy)
It's ok for me to start work on it. Taking!
Assignee: nobody → rexboy
Flags: needinfo?(rexboy)
From what I investigated until now we need two images:
1. The "disclosure button", which is a right arrow shown in
   https://mozilla.app.box.com/s/o79mkjw8oetqhu6uu26u/1/1055064395/10007061032/1
   The most similar one I can find is in building block:
   https://github.com/mozilla-b2g/gaia/blob/master/shared/style/headers/images/icons/back-rtl.png

2. The hangup image shown in attachment. I think we already have one:
   https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/style/images/ActionIcon_40x40_hangup.png

Victoria may you confirm if I can use these images? Especially for the first one, if not may you provide
the image needed. thanks a lot!
Flags: needinfo?(vpg)
Depends on: 906775
Hi KM,

For the first image, the overlay, please use the provided design, as I used our current patterns. The interaction spec is a bit off in this sense, and uses a new pattern, while our OS overall pattern to close overlays is this button, we never use an arrow or a cross to close an overlay as it is a special kind of navigation, needs a different kind of way out.

Regarding the second image, the hang up icon, you can use it.

Thanks!
V


(In reply to KM Lee [:rexboy] from comment #4)
> From what I investigated until now we need two images:
> 1. The "disclosure button", which is a right arrow shown in
>   
> https://mozilla.app.box.com/s/o79mkjw8oetqhu6uu26u/1/1055064395/10007061032/1
>    The most similar one I can find is in building block:
>   
> https://github.com/mozilla-b2g/gaia/blob/master/shared/style/headers/images/
> icons/back-rtl.png
> 
> 2. The hangup image shown in attachment. I think we already have one:
>   
> https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/
> style/images/ActionIcon_40x40_hangup.png
> 
> Victoria may you confirm if I can use these images? Especially for the first
> one, if not may you provide
> the image needed. thanks a lot!
Flags: needinfo?(vpg)
Hello Victoria:
Thanks for ansering! For the second one I'll use the hangup button.
For the first one, I'm not sure if I understand well so let met confirm on it: the
disclosure button here is for *entering* the detail page. The button is located on
conference call entry of call screen (not on detail page). So the attachment on this
bug didn't include design of that. May you tell me how the button should be styled?
I'm not sure where to find the correct visual design of that.

Thanks a lot!
Flags: needinfo?(vpg)
So my understanding is that we're not going to use the  ">" shown in the attachment.
What design should I use for it then?
(In reply to KM Lee [:rexboy] from comment #8)
> So my understanding is that we're not going to use the  ">" shown in the
> attachment.
> What design should I use for it then?

And what UX? (since taping the whole line is used to toggle between holded/active call).

Let's try not to bock development on this, we can switch to the good asset at the last minute.
Hey! Sorry, there was a misunderstanding, if you refer to the arrow placed in the blue area of the conference call screen (in here: https://mozilla.box.com/s/k3c2j7d4uoq9et08kw1h), then it's ok. 

Thanks!
V

(In reply to KM Lee [:rexboy] from comment #6)
> Hello Victoria:
> Thanks for ansering! For the second one I'll use the hangup button.
> For the first one, I'm not sure if I understand well so let met confirm on
> it: the
> disclosure button here is for *entering* the detail page. The button is
> located on
> conference call entry of call screen (not on detail page). So the attachment
> on this
> bug didn't include design of that. May you tell me how the button should be
> styled?
> I'm not sure where to find the correct visual design of that.
> 
> Thanks a lot!
Flags: needinfo?(vpg)
Thanks Victoria!

Some update:
* Never get conferencecall.statechange on current build and there's a patch in bug 914631.
* Now working on case of conference call 2 to 1. I think the reasonable behavior is we just hide the
  detail screen when calls changes to 1.
* I'm not sure if waiting 5 seconds before removing the node will make the code dirty... Maybe I'll put
  code in review before implementing this one.
(In reply to KM Lee [:rexboy] from comment #11)
> Thanks Victoria!
> 
> Some update:
> * Never get conferencecall.statechange on current build and there's a patch
> in bug 914631.
> * Now working on case of conference call 2 to 1. I think the reasonable
> behavior is we just hide the
>   detail screen when calls changes to 1.

Yes it makes sense.

> * I'm not sure if waiting 5 seconds before removing the node will make the
> code dirty... Maybe I'll put
>   code in review before implementing this one.

Yes we can do this in a follow up. Let's wait for the basic details screen and the banner from bug 912508 before deciding on this.
Some problems here.
Due to comment from Hsin-Yi and test of my own, suppose there are two phones A and B in groupsCall.
If I hang-up A in groupcall, the following events occur in sequence:

1.conferenceGroup.callsChanged: conferenceGroup.call:[A], telephony.call:[]  (notice B disappeared here)
2.telephony.callsChanged: conferenceGroup.call:[A], telephony.call:[B] * (B is added since here)
3.conferenceGroup.callsChanged: conferenceGroup.call:[], telephony.call:[B] (A removed)

As you can see, telephony.callsChanged would never know when [A] is removed if we just listen to
telephony.call, since telephony.callsChanged only happens when B is added into telephony.call.
So we must in some way communicate from ConferenceGroupHandler to CallsHandler that "A is removed",
or, even more, refactor to add ConferenceGroupHandler.handledCalls..
Attached file patch — 
Patch with some known issue commened on Github.
Since it's a middle patch I decide to put it first..
Etienne would you take a look and give some suggestion?
Also it still needs unit test. Sorry for that :<.. I'll write them ASAP.
Attachment #803079 - Flags: review?(etienne)
Comment on attachment 803079 [details]
patch

This is in good shape !

Made some comments on github about testing and the missing oncallschanged issue, hope that helps, can't wait for the next version of the patch :)
Attachment #803079 - Flags: review?(etienne) → feedback+
Comment on attachment 803079 [details]
patch

Thanks for your comments Etienne!

Updated the patch with unit tests. And thanks to Hsin-Yi's patch of
bug 915638 (sends onStateChange correctly when conference call ends)
and Etienne's comment I think we solved the problem on updating |callsHandler.handleCalls|.

I think the patch is pretty much done. But I have to confirm whether
it works as expected with 3 participants.... once I have enough phones
to test it tomorrow.
Comment on attachment 803079 [details]
patch

In the meantime, I'm confirming my feedback+ for the new version :)
Made some minor comments, and pointed at 2 unit tests we should add but we're very close!
Comment on attachment 803079 [details]
patch

Updated per comments on Github.

I'll remove test codes and squash before merging.
Etienne may you take a look again?
Attachment #803079 - Flags: review?(etienne)
Comment on attachment 803079 [details]
patch

Congratulations!

r=me with the small test rename, the debug removed and the commits squashed.

Thanks again for all the hard work on this one!
Attachment #803079 - Flags: review?(etienne)
Attachment #803079 - Flags: review+
Attachment #803079 - Flags: feedback+
Thanks for all the reviewing and suggestions Etienne!

merged on master:
https://github.com/mozilla-b2g/gaia/pull/12124

\O/
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
(In reply to KM Lee [:rexboy] from comment #20)
> Thanks for all the reviewing and suggestions Etienne!
> 
> merged on master:
> https://github.com/mozilla-b2g/gaia/pull/12124
> 
> \O/

\o/
Depends on: 915638
Note that we need patch of bug 912494 to make this patch work correctly. If we want to try on
it the patch should be applied on Gecko.
(Or come to me for flashing a version with bug 912494 applied)
Sorry, I mean, bug 915638
Depends on: 944618
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: