1045499 - Switch call log to a 2-line layout

Status: RESOLVED FIXED

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

Description

[Carrie Wang [New account: [e-mail]]]

Attachment: Call log list view.001.jpg

While designing the US 877971, we found the current implementation does not match the original UX spec. Hence, we did some evaluation and provide some new rules on the page. Please take a look and let me know if I've missed anything here. Thanks!

Comment 1

[Carrie Wang [New account: [e-mail]]]

[Blocking Requested - why for this release]:

Comment 2

[Anthony Ricaud (:rik)]

- For the unknown calls, we don't have a type of number ("Mobile") to display on the second row, what should we do?
- I've split the missed call highlighting in bug 1045515.
- We'll need some visual precisions for the height of each row and the right margin for the time of the call.

Comment 3

[Anthony Ricaud (:rik)]

Forgot to needinfo Carrie for comment 2.

Comment 4

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

it's a UI improvement which should not block release.

Comment 5

[Carrie Wang [New account: [e-mail]]]

Hi Anthony, 

Can we detect something else, such as location of the call? (This is Android solution and I found it's quite helpful). If not, then we can just display "unknown".

ni? Carol for visual spec. Thanks!

Comment 6

[Doug Sherk (:drs) (inactive)]

(In reply to Carrie Wang [:carrie] from comment #5)
> Can we detect something else, such as location of the call? (This is Android
> solution and I found it's quite helpful). If not, then we can just display
> "unknown".

I think this would require some kind of web services or library of area codes stored on the device, so I would venture to say it's not an option.

Comment 7

[Carol Huang [:Carol]]

Attachment: CallLog_2lines_spec.jpg

Hi Anthony,
the attachment is the visual spec changes for the call log.
let me know if you have any question.
Thanks!!

Comment 8

[Anthony Ricaud (:rik)]

Thanks! Doug: Can you take a look today and see if you have questions for Carol?

Comment 9

[Doug Sherk (:drs) (inactive)]

This looks to me like everything we need.

Comment 10

[Doug Sherk (:drs) (inactive)]

Attachment: Overflow example

A couple of questions: 

Is the number next to the name the retry count? I assume so since I think it would be bad to leave it next to the time field.

If we overflow the type+carrier field as in the attached picture, should we just ellipsize it?

Comment 11

[Carol Huang [:Carol]]

Attachment: CallLog_2lines_spec.jpg

Hi Doug,

Yes,I've moved Retry # next to the name.

For the other question that you mentioned, I'm not sure about the screenshot you attached but I assumed that you mean the type+carrier text length. I add to the visual spec as below,
-If the type+carrier is longer than 16.5 rem(w), the text need to be elipsized.
(please see the attachment: CallLog_2lines_spec.jpg)

Thanks! Let me know if you have any other question.

Comment 12

[Doug Sherk (:drs) (inactive)]

Sorry, I uploaded the wrong file. You answered my question anyways, so I think that's everything we need. Thanks.

Comment 13

[Doug Sherk (:drs) (inactive)]

Attachment: Switch call log to a 2-line layout.

PR: https://github.com/mozilla-b2g/gaia/pull/22910

Comment 14

[Doug Sherk (:drs) (inactive)]

Attachment: Fix JSHint errors.

Comment 15

[Doug Sherk (:drs) (inactive)]

Attachment: Screenshot, patch applied.

Carol, I have a couple of things to note here:

* The time on the far right looks too bunched up to me with the scrollbar, so I moved it slightly to the left. This looks much better to me on a device.

* The spec was unclear on whether or not the time should be in red. I wasn't sure if that was for the sake of highlighting or because it should actually be red.

Both of these are easy to change if we want to.

Comment 16

[Doug Sherk (:drs) (inactive)]

Attachment: Screenshot, patch not applied.

Reference screenshot for comparison with attachment 8473476 [details].

Comment 17

[Carol Huang [:Carol]]

Comment on attachment 8473476 [details]
Screenshot, patch applied.

Hi Doug,
Everything looks good, just on little thing. 
Can you help me change the Call# text color to #bc1515 ?
'time' text color is good.

I'll update the visual spec.
Thank you!!!

Comment 18

[Carol Huang [:Carol]]

Attachment: CallLog_2lines_spec.jpg

Visual spec update:
Change the call number& name to red (#bc1515)

Comment 19

[Doug Sherk (:drs) (inactive)]

Attachment: Switch call log to a 2-line layout.

Updated PR.

Comment 20

[Doug Sherk (:drs) (inactive)]

Attachment: Fix JSHint errors.

Comment 21

[Doug Sherk (:drs) (inactive)]

Attachment: Screenshot, patch applied.

Here it is with the changes listed in comment 17.

Carrie, I'm flagging you to check the "Unknown" for unknown numbers.

Comment 22

[Carol Huang [:Carol]]

Comment on attachment 8473833 [details]
Screenshot, patch applied.

Hi Doug,
It looks pretty good to me!thank you!

Comment 23

[Carrie Wang [New account: [e-mail]]]

Comment on attachment 8473833 [details]
Screenshot, patch applied.

Looks great! I'm so happy!

Comment 24

[Anthony Ricaud (:rik)]

Comment on attachment 8473832 [details] [diff] [review]
Fix JSHint errors.

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

::: apps/communications/dialer/js/call_log.js
@@ +917,3 @@
>        case 'create':
>        case 'update':
> +      */

Why commenting this and not deleting?

Comment 25

[Anthony Ricaud (:rik)]

Comment on attachment 8473831 [details] [diff] [review]
Switch call log to a 2-line layout.

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

Globally, this is the right direction. There's some more cleanup to do in this patch.

Also, we need a followup bug to delete the photos from the CallLogDb.

::: apps/communications/dialer/js/call_log.js
@@ -462,5 @@
>  
>      label.appendChild(input);
>      label.appendChild(span);
>  
>      var aside = document.createElement('aside');

This element is now empty, we should remove it.

@@ -466,5 @@
>      var aside = document.createElement('aside');
>      aside.className = 'pack-end';
> -    var img = document.createElement('span');
> -    img.dataset.type = 'img';
> -    img.className = 'call-log-contact-photo';

You need to update the comment of createGroup to match the new DOM.

@@ +511,5 @@
> +    var addInfo = document.createElement('p');
> +    addInfo.className = 'additional-info';
> +    addInfo.setAttribute('aria-hidden', 'true');
> +
> +    var typeCarrier = document.createElement('span');

typeAndCarrier is probably a better name. I originally thought it was the type of carrier.

@@ +522,4 @@
>      var callTime = document.createElement('span');
>      callTime.className = 'call-time';
>      callTime.textContent = Utils.prettyDate(date) + ' ';
> +    if (addInfo) {

No need for this if.

::: apps/communications/dialer/style/call_log.css
@@ +196,4 @@
>  }
>  
> +.log-item .additional-info {
> +  text-transform: capitalize;

I think this is frowned upon for L10n issues. Can you check with yzen?

@@ +205,5 @@
> +  overflow: hidden;
> +  text-overflow: ellipsis;
> +}
> +
> +.log-item .additional-info .call-time {

Most of the selectors above can use only the right most class and still work.

::: shared/js/dialer/utils.js
@@ -68,5 @@
>     */
>    getPhoneNumberAdditionalInfo:
>      function ut_getPhoneNumberAdditionalInfo(matchingTel) {
> -    var number = matchingTel.number || matchingTel.value;
> -    if (!number) {

This is a change in behaviour. Have you checked that all call sites (callscreen for example) are able to handle this new behaviour? Also, the mock should be updated to not return undefined anymore.

Comment 26

[Doug Sherk (:drs) (inactive)]

Attachment: Switch call log to a 2-line layout.

Updated PR. The PR has a new commit for the review fixes. This is a rollup.

Comment 27

[Doug Sherk (:drs) (inactive)]

Attachment: Screenshot, patch applied, call screen addl. info.

This raises a good question. Carrie, should the callscreen be updated to have the same "additional info" field as well? This used to contain the phone number of the contact, but now it contains the "type, carrier" combination, e.g. "Mobile, Telefonica". See attachment for an example.

Comment 28

[Doug Sherk (:drs) (inactive)]

(In reply to Anthony Ricaud (:rik) from comment #24)
> Why commenting this and not deleting?

For reference so that onlookers can see the possible values. I'm not attached to this and would be fine with removing it.

Comment 29

[Carrie Wang [New account: [e-mail]]]

Comment on attachment 8475532 [details]
Screenshot, patch applied, call screen addl. info.

Hi Doug, 

I wonder can we replace the carrier name on the call screen with the numbers (of course, only for existing contacts)?

So for the existing contacts who have been set with carrier names in Contact details, we display:

1. number type + carrier name (as the second row) in Call log
-> In this case, users can long-tap to check the numbers in call info. In addition, we don't have enough space to display numbers in call log and it does cause mess.

2. number type + numbers (as the second row) on call screen
-> In this case, users might need some info to make sure which number he is calling to or calling him.

What do you say?

Thanks!

Comment 30

[Doug Sherk (:drs) (inactive)]

Attachment: Switch call log to a 2-line layout.

Updated PR.

Comment 31

[Doug Sherk (:drs) (inactive)]

Attachment: Screenshot, patch applied, call screen addl. info.

(In reply to Carrie Wang [:carrie] from comment #29)
> Comment on attachment 8475532 [details]
> Screenshot, patch applied, call screen addl. info.

Yeah, this makes sense to me. I've implemented this in attachment 8475986 [details] [diff] [review] and you can see it in the attached screenshot.

Comment 32

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8475986 [details] [diff] [review]
Switch call log to a 2-line layout.

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

::: apps/callscreen/test/unit/handled_call_test.js
@@ +540,5 @@
>        });
>      });
>    });
>  
> +  test('should display contact info', function() {

Whoops. Will switch this back before I land this / after the next review cycle.

Comment 33

[Carrie Wang [New account: [e-mail]]]

It looks really awesome! great job. Thanks!

Comment 34

[Carol Huang [:Carol]]

Attachment: text.jpg

hey Doug,
I saw the screenshot you have and the contact name is too close to (number type + numbers). The contact name even got cropped. 
Could you add 0.5 rem space for it?
Thank you!

Comment 35

[Carol Huang [:Carol]]

Hi Doug,
Could you just make sure they have 0.5 rem space between those two lines?
Because the text on my device looks ok, didnt got cropped.
Thank you :)

Comment 36

[Doug Sherk (:drs) (inactive)]

(In reply to Carol Huang [:Carol] from comment #34)
> Created attachment 8476508 [details]
> text.jpg
> 
> hey Doug,
> I saw the screenshot you have and the contact name is too close to (number
> type + numbers). The contact name even got cropped. 
> Could you add 0.5 rem space for it?
> Thank you!

(In reply to Carol Huang [:Carol] from comment #35)
> Hi Doug,
> Could you just make sure they have 0.5 rem space between those two lines?
> Because the text on my device looks ok, didnt got cropped.
> Thank you :)

Filed bug 1056883.

Comment 37

[Doug Sherk (:drs) (inactive)]

https://github.com/mozilla-b2g/gaia/commit/ff7b518acb2c88c8e368dfd88fb132fbe8cd0158

Comment 38

[Doug Sherk (:drs) (inactive)]

This should have the feature-b2g:2.1 flag. Bhavana, would you please add this? Thanks.