Closed Bug 915081 Opened 7 years ago Closed 7 years ago

[wasabi] The 2nd incoming call screen does not follow spec's UI definition.

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:koi+, b2g18 unaffected, b2g-v1.1hd unaffected, b2g-v1.2 fixed)

RESOLVED FIXED
1.2 C3(Oct25)
blocking-b2g koi+
Tracking Status
b2g18 --- unaffected
b2g-v1.1hd --- unaffected
b2g-v1.2 --- fixed

People

(Reporter: hlu, Assigned: gsvelto)

References

Details

(Whiteboard: [FT:RIL])

Attachments

(8 files, 5 obsolete files)

189.18 KB, image/png
Details
1.22 MB, application/pdf
Details
1.76 MB, application/zip
Details
382.94 KB, application/pdf
Details
358 bytes, text/html
Details
1.76 MB, application/zip
Details
75.50 KB, image/png
Details
6.77 KB, patch
Details | Diff | Splinter Review
Attached image screenshot.png
[Version Info]
Gaia:     2d870100a945272824f8e42a8c52aac095fe907e
  B-D     2013-09-11 04:59:20
Gecko:    46c335b301fdb32f47e58c665190ffc41522ae00
BuildID   20130911062051
Version   26.0a1

* Symptom:
1. The 2nd incoming call screen does not follow spec's UI defintion.

* Reproduce Steps
1. Make a call to other device or MT call to DUT.
2. MT a second call to DUT.

* Actual results:
1. The 2nd incoming call screen UI is incorrect.

* Expected results:
1. It should follow spec's UI defintion.
Attached file Call waiting UI spec.pdf (obsolete) —
Whiteboard: [FT:RIL
blocking-b2g: --- → koi?
Component: RIL → Gaia::Dialer
Whiteboard: [FT:RIL → [FT:RIL]
I'm CC'ing David and Etienne because I'm not entirely sure how to proceed here. The 2nd incoming call screen as per spec is completely different compared to what we have now in the GSM code and would require significant effort. Besides I'm not sure we'd want different incoming screens for the two networks considering that the functionality is fundamentally the same.
Add UX team members.
Update the new call waiting spec. The diff part is as below.
- To define the case while no contact picture for 2nd incoming phone call.
- To define the case while no contact name for 2nd incoming phone call.
Attachment #802922 - Attachment is obsolete: true
Thanks for the new spec, CC'ing Anthony too to get the discussion started on this. To sum up the issue here, the problem we're trying to solve with this spec is the following: in CDMA mode it's possible to answer an incoming call, putting the first call on hold, or to ignore the incoming call; it is not possible however to end the incoming call and answer the second one. The current UI presents you with hold-and-answer and end-and-answer options instead but the second one doesn't work as excepted in CDMA mode.

The spec in attachment 803537 [details] puts forward a brand new UI for answering the incoming call when in CDMA mode. In this design the user would chose between answering the incoming call (green button) or ignoring it (red button). The spec calls for a new pane that's overlaid over the call screen and displays the caller's picture is available or the background if not.

While this spec addresses the requirement of providing the user with options corresponding to actual CDMA behavior it has the drawback that it would require brand new code for the pane (essentially HTML+CSS) but also a separate JS code path to activate it and respond to interaction in CDMA mode. My gripe is that this effectively means duplicating the incoming call code for GSM and CDMA with all the drawbacks of duplicated code (increased test burden, necessity to fix both codepaths in case of bugs or refactoring, etc...). Additionally the scenario in which a developer only has access to a call waiting mode but not the other is quite common (we don't have CDMA networks in Europe) so it's likely that a developer might accidentally break it w/o being able to notice it during testing.

What do you guys think about this? I've got an alternative route to quickly fix this which I described in bug 914967 comment 7 which essentially amounts to adapting the current call waiting UI to CDMA behavior without introducing a new UI specifically for it.
Flags: needinfo?(etienne)
Flags: needinfo?(anthony)
Hi Neo,

In page 6, show unknown for caller name one(before answering 2nd call), isn't unknown usually for call we don't even know the number? And check with current UI on GSM, no matter on dialer or text message, there is no "Unknown" wording for number which is not saved in contact. Just want to double check if this will be for CDMA only, feel free to let me know your comment, thanks.
Flags: needinfo?(nhsieh)
My preference would be: hiding the button for the function we don't have and making the other one on the same line larger.

This will probably be a 3 lines CSS divergence instead of having a whole code path rarely used begging to regress.
Flags: needinfo?(etienne)
Blocks: 890807
Flags: needinfo?(anthony)
Blocks: 914967
according to 9/25 triage result: this issue need to be reproduced on real device, since it only happens on wasabi now.
blocking-b2g: koi? → -
Gabriele, please take this bug. Thanks.
Assignee: nobody → gsvelto
please ignore previous comment, it was a mistake.
according to 9/25 triage result, change to koi+
Assignee: gsvelto → nobody
blocking-b2g: - → koi+
Again, can we please work out a compromise.
Having a completely different UX just for the sake of it will have a high engineering/qa/maintenance cost.

The call waiting screen of CDMA and GSM should be kept as similar as possible (ie. just hiding buttons for features unavailable on CDMA).
If we want to improve _both_ that's find by me but probably not koi+.

Can we keep the changes to a minimum for 1.2 and revisit for 1.3?
Flags: needinfo?(hlu)
(In reply to Etienne Segonzac (:etienne) from comment #11)
> Again, can we please work out a compromise.
> Having a completely different UX just for the sake of it will have a high
> engineering/qa/maintenance cost.
> 
> The call waiting screen of CDMA and GSM should be kept as similar as
> possible (ie. just hiding buttons for features unavailable on CDMA).
> If we want to improve _both_ that's find by me but probably not koi+.

The solution I'm working on is to replace the buttons (hide / end) of the regular GSM call screen with the buttons shown in the CDMA call waiting UX spec (answer / ignore). The functionality of the buttons will be changed accordingly. This will allow for two things: first of all the behavior in CDMA mode will be consistent with the UX spec, second it won't deviate much from the existing code lowering the maintenance. I'll have to revisit the call-screen unit-tests accordingly but that shouldn't take too long.
Assignee: nobody → gsvelto
Hi Etienne,

I think we need UX team to is the correct owner to clarify your question. For QA we just need final correct design for test, or we can't be sure whether we should treat bug 914967 a real bug or not since it's totally different design from what UX provided.

Hi Neo,

Please must help to clarify all questions we have so far for you, thanks.
Flags: needinfo?(hlu)
Hi Everyone, 

We updated the page 6 layout. Please follow the new specification. 
And yes, Since this design also fixes some UX bugs, We need this feature be finished in 1.2.  Thank you very much.
Flags: needinfo?(nhsieh)
Target Milestone: --- → 1.2 QE1(Oct11)
I finally had some time to work on this and this is the best compromise I could come up with. This patch implements what Etienne suggested in comment 7 effectively hiding the end-and-answer button while in CDMA call-waiting mode and enlarging the hold-and-answer one to fill up the emptied space. Unit-tests are included and I took the liberty of refactoring the CDMA CSS code I had already put in to better follow the new style we're using.

The pros of this patch are:
- Minimal logic changes compared to the GSM call-waiting code
- Good unit-test coverage as we already leverage the tests for GSM call-waiting
- User interaction is consistent with the limitations of CDMA

The con:
- Does not follow the attached UX spec

Considering the amount of things I have in my backlog I think this is the best way to go: I won't be able to implement the attached UX spec in time for 1.2 so my proposal is to take this to solve the user-interaction problems and open a follow-up to change the UX according to the spec (provided we want GSM and CDMA modes to diverge, something that I'm still not convinced is a good idea).
Attachment #813658 - Flags: feedback?(nhsieh)
Attachment #813658 - Flags: feedback?(etienne)
Comment on attachment 813658 [details] [diff] [review]
[PATCH WIP] Hide the end-and-answer button during CDMA call waiting

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

This is just lovely :)

::: apps/communications/dialer/js/call_screen.js
@@ +64,5 @@
> +   * @param {Boolean} enabled Enables hold-and-answer-only operation.
> +   */
> +  set holdAndAnswerOnly(enabled) {
> +    this.incomingAnswer.classList.toggle('hold-and-answer-only', enabled);
> +    this.incomingEnd.classList.toggle('hold-and-answer-only', enabled);

might be clearer to toggle the class on the `#incoming-container`.

::: apps/communications/dialer/style/oncall.css
@@ +609,4 @@
>      font-size: 0.9em;
>    }
>  
> +  #incoming-answer.hold-and-answer-only {

so this would become #incoming-container.hold-and-answer-only #incoming-answer ...

@@ +612,5 @@
> +  #incoming-answer.hold-and-answer-only {
> +    width: 100%;
> +  }
> +
> +  #incoming-end.hold-and-answer-only {

you get the gist :)
Attachment #813658 - Flags: feedback?(etienne) → feedback+
Attached image Proposed UI screenshot (obsolete) —
Thanks Etienne, an updated patch is coming. Here's how the UI looks with the patch applied (I've simulated it on the desktop with a bunch of ugly hacks and took a snapshot of it).
I've adjusted the patch as per comment 17 and modified the unit tests accordingly. I've tested the UI result on the desktop by using the fake-call JS code to display the oncall.html screen.
Attachment #813658 - Attachment is obsolete: true
Attachment #813658 - Flags: feedback?(nhsieh)
Attachment #814426 - Flags: review?(etienne)
Attachment #814433 - Attachment description: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/12729 → [PULL REQUEST] Hide the end-and-answer button during CDMA call waiting
Comment on attachment 814426 [details] [diff] [review]
[PATCH] Hide the end-and-answer button during CDMA call waiting

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

Sorry I didn't found the status bar mode issue earlier.
Otherwise it's a-ok, but we need to fix this.

::: apps/communications/dialer/style/oncall.css
@@ +374,5 @@
>      display: none;
>    }
>  
> +  #calls.switch > section .switch-calls {
> +    display: block;

You need to make sure that the .switch-calls div isn't displayed in "status bar mode" (the green bar on top of the screen when the user presses home).

Good thing is, this should be pretty straightforward since Rik refactored it properly in an oncall_status_bar.css file.
Attachment #814426 - Flags: review?(etienne)
(In reply to Etienne Segonzac (:etienne) from comment #21)
> Sorry I didn't found the status bar mode issue earlier.
> Otherwise it's a-ok, but we need to fix this.
> 
> [...]
> 
> You need to make sure that the .switch-calls div isn't displayed in "status
> bar mode" (the green bar on top of the screen when the user presses home).
> 
> Good thing is, this should be pretty straightforward since Rik refactored it
> properly in an oncall_status_bar.css file.

Right, good catch. In fact I had seen Rik's patch but I somehow forgot it when I wrote this one. I'll update it ASAP.
Firefox OS UX team, we need your help to see what design we should follow. Looks like Etienne has some comments about existed design.
Flags: needinfo?(firefoxos-ux-bugzilla)
Postpone the target milestone.
Target Milestone: 1.2 C2(Oct11) → 1.2 C3(Oct25)
Revised patch, I've only added a snippet to hide the switch-calls div when in status-bar mode.
Attachment #814426 - Attachment is obsolete: true
Attachment #816494 - Flags: review?(etienne)
Neo should respond here. I will ask him to follow up and cc: Jacqueline.
Flags: needinfo?(firefoxos-ux-bugzilla) → needinfo?(nhsieh)
Hi Jacqueline, Could you please provide your comment? When I design CDMA feature, I found original design seems not very user friendly.("Hold" means hold 1st call and pick up new incoming call. "Ignore" means ignore new incoming call) Now you are charging of GSM Comms feature. Can you help to have a look?  Thanks
Flags: needinfo?(nhsieh)
Flags: needinfo?(jsavory)
fix PDF file name
Comment on attachment 816494 [details] [diff] [review]
[PATCH v2] Hide the end-and-answer button during CDMA call waiting

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

> Revised patch, I've only added a snippet to hide the switch-calls div when in status-bar mode.

That should do it :)
Attachment #816494 - Flags: review?(etienne) → review+
Attached image Proposed UI screenshot
Attachment #814423 - Attachment is obsolete: true
This is a refreshed patch on top of my recent landing-spree. I didn't do any changes to the patch besides making sure it applies on top of the recent changes so I'm carrying over the r+ (r=etienne) flag.
Attachment #816494 - Attachment is obsolete: true
Pushed to both branches:

master 669d29644d3808039ed57ca5d6a1f90f8599ecb0
v1.2   22b95ba3a4df0c2e69ac19a7300cb9d43085a860

Let's open a follow-up to implement the new unified UI that's been worked upon.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Blocks: 928700
Clearing the last needinfo as we moved the follow-up work to bug 928700.
Flags: needinfo?(jsavory)
You need to log in before you can comment on or make changes to this bug.