1096916 - [Dialer][Callscreen] The direction icon is incorrect when is hanging up a hold call

Status: RESOLVED FIXED

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

Description

[Paco Rampas [:paco]]

Attachment: call-ended-hold.png

Comment 1

[Paco Rampas [:paco]]

Attachment: patch in github

Comment 2

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8520601 [details] [review]
patch in github

Redirecting to Germán.

Comment 3

[Germán Toro del Valle (:gtorodelvalle)]

Comment on attachment 8520601 [details] [review]
patch in github

Hi Paco! I am perfectly fine with the proposed patch ;) Anyhow, it is clear that a lot of refactoring can be made at a oncall.css level, although definitely not in this bug. We currently have a lot of duplicated state information in the markup such as "#call-screen[data-layout="connected-hold"]" which could be inferred from other already existent information such as "#calls.big-duration" (which BTW is a duplicate of "body.single-line", except when the call screen is minimised), ".handled-call.held" and "#group-call.held".

Definitely something to take into account in the forthcoming refactoring / redesign ;)

Thank you very much!

Comment 4

[Germán Toro del Valle (:gtorodelvalle)]

BTW, Paco and just not to forget about it ;) Would you be so kind to include a demo of this patch at https://wiki.mozilla.org/FirefoxOS/Comms/Dialer/Sprint/v2.1-S9#Demos , please ? Thank you very much!

Comment 5

[Paco Rampas [:paco]]

Attachment: ended-icon-hold.png

Comment 6

[Paco Rampas [:paco]]

Attachment: ended-icon-hold.png

Comment 7

[Germán Toro del Valle (:gtorodelvalle)]

Hi Paco, for some strange reason the tests have not been run after you pushed your latest commit to this patch :O Would you be so kind to rebase it and to repush it to force the tests to be checked, please? Thanks!

Comment 8

[Paco Rampas [:paco]]

Attachment: patch v2

Comment 9

[Paco Rampas [:paco]]

Comment on attachment 8531209 [details] [review]
patch v2

Hi German, I have attached other patch, could you review? It is the same that the first one. The issue with the first one was that the tests weren't green never, so I decided create a new branch and the tests already are working fine.

Thanks.

Comment 10

[Germán Toro del Valle (:gtorodelvalle)]

Comment on attachment 8531209 [details] [review]
patch v2

Hi Paco! Sorry for the delay :( Looking good to me and working fine ;) Thanks!

r+ and ready to merge ;)

Comment 11

[Paco Rampas [:paco]]

merged: 5a84c3259486a589e008a5d5cf5d367984142f9b