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!

[Blocking Requested - why for this release]:

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

Forgot to needinfo Carrie for comment 2.

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

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!

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

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

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

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?

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.

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

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

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.

Reference screenshot for comparison with attachment 8473476 [details].

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

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

Updated PR.

Here it is with the changes listed in comment 17. Carrie, I'm flagging you to check the "Unknown" for unknown numbers.

Comment on attachment 8473833 [details] Screenshot, patch applied. Hi Doug, It looks pretty good to me!thank you!

Comment on attachment 8473833 [details] Screenshot, patch applied. Looks great! I'm so happy!

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

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

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.

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

Updated PR.

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

It looks really awesome! great job. Thanks!

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!

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

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

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