1139179 - [RTL][Contacts] Two SIMs, Contact Information has SIM Selection text overlapping the Dialer/Call Icon

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Contacts, defect, P2)

Description

[Oliver Nelson [:oliverthor]]

Attachment: Contacts_SIMSelection_OverlapDialerIcon.png

Description:
When a user observes the contact information for a contact that they have added a phone number for, they will observe overlapping text with the dialer icon. This issue only occurs when the user has a second SIM in their phone.

PreReq:
* Phone has 2 SIMs
* Phone in an RTL language (Arabic tested)
Repro Steps:
1) Update a Flame to 20150303010233
2) Open the 'Contacts' app.
3) Create a new contact: name + phone number.
4) Tap the completed contact to view more info.
5) Observe the Dialer button to call the contact.

Actual:
SIM Selection text is overlapping the Dialer icon in RTL.

Expected:
SIM Selection text is not overlapping the Dialer icon in RTL.

Environmental Variables:
--------------------------------------------------
Device: Flame 3.0
Build ID: 20150303010233
Gaia: c8ed1085a67490a1ecd7f275e5de9487e1b93b1d
Gecko: 0b3c520002ad
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 39.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0
--------------------------------------------------
Device: Flame 2.2
BuildID: 20150303002527
Gaia: 3d188c414e30acc392253d5389a42352fcfbc183
Gecko: c89aad487aa5
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
--------------------------------------------------

Repro frequency: 5/5
See attached: 
screenshot

Comment 1

[Delphine Lebédel [:delphine - Use needinfo]]

Triage: P2

Comment 2

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

triage: obvious UI truncation, on a new feature

Comment 3

[Delphine Lebédel [:delphine - Use needinfo]]

Can we please get someone assigned to this as this is a blocker? thanks

Comment 4

[Francisco Jordano [:arcturus] [:francisco]]

Hei Paco,

I saw that you did the last modification for ltr here, do you have time to take a look for the rtl version?

Comment 5

[Lancy(Leave from Mozilla)]

Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15396/

Comment 6

[Steve Chung [:steveck]]

It seems that current contact button structure couldn't handle this case properly, because we assume the SIM indicator field as fixed width, and the icon position and phone number length are all depending on this width. It's quite fragile since SIM indicator will be translated into other language with unknown length. I have no better solution than refactor contact button for multisim, but maybe we can propose other easier way for 2.2 like moving the icon to the end of the button and use flex to control the width of indicator/phone. Hi Francisco, any suggestion about this bug? Maybe I'm worng about the use of contact button and there's other simpler way to fix it.

Comment 7

[Francisco Jordano [:arcturus] [:francisco]]

(In reply to Steve Chung [:steveck] from comment #6)
> It seems that current contact button structure couldn't handle this case
> properly, because we assume the SIM indicator field as fixed width, and the
> icon position and phone number length are all depending on this width. It's
> quite fragile since SIM indicator will be translated into other language
> with unknown length. I have no better solution than refactor contact button
> for multisim, but maybe we can propose other easier way for 2.2 like moving
> the icon to the end of the button and use flex to control the width of
> indicator/phone. Hi Francisco, any suggestion about this bug? Maybe I'm
> worng about the use of contact button and there's other simpler way to fix
> it.

Hi Steve, you are totally right, I had a quick look and as you commented there is no easy way of doing a correct implementation without refactory, and perhaps, using the web component gaia-sim-picker.

I would rather go with the solution that you propose:
- For 2.2 get a workaround that get a better experience, even if it's not the ideal solution.
- Refactor for 3.0 that button to use the gaia-sim-picker

I have the feeling that bug 1141464 is a dupe of this one, if not, pretty much the same root cause.

Thanks for taking a look to this!

Comment 9

[Steve Chung [:steveck]]

Hi Carrie, we have some issue about the call button in call log information view and need your suggestion: In attachment 8572214 [details], the localized sim indicator will overlap the original, because we assume the sim indicator's width is fixed. If the indicator length will grow, is it possible to set the icon to the end of button, or using other simpler way to repersent the sim picker(like message app)?

Comment 10

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

Hi, 


I'd say, the icon should be dynamically positioned besides the texts, just like LTR. I don't suggest to change the design only for this RTL issue. In addition, both the phone numbers and email are not aligning right. This should be fixed as well. ni? Fang if you need more visual input here. Thanks!

Comment 11

[Fang Shih [:fang]]

Attachment: RTL_contact detail.png

Attached the screenshot, the phone numbers and email should be right aligned like this. Thanks!

Comment 12

[howie [:howie]]

Hi Steve, can you take this one?

Comment 13

[Steve Chung [:steveck]]

I'll create patch that refacotr the call button entirely since VD declined further visual changes at this stage. But these button refactor might also introduce some risk so maybe Francisco can evaluate whether we need take this or not.

Comment 14

[GitHub Autolander Bot]

Attachment: [gaia] steveck-chung:contact-call-button-fix > mozilla-b2g:master

Comment 15

[Steve Chung [:steveck]]

Comment on attachment 8580581 [details] [review]
[gaia] steveck-chung:contact-call-button-fix > mozilla-b2g:master

Hey Francisco/Doug,

I made some changes to the call button with flex for proper width depends on content instead of fixed width. That's the minimal chages I could think of if we want to keep the same layout. The bad news is flex could not work inside the button element. Unlike dialer, the button's layout is bind with button tag rule, so we will need to migrate shared button styling into contact_button. This will be done in this patch, so it's just a WIP that fix the button in call information view.

Comment 16

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8580581 [details] [review]
[gaia] steveck-chung:contact-call-button-fix > mozilla-b2g:master

I would like to keep the semantic of the button, but if flex box is not working for the button, I'm ok with this work around until we migrate to gaia-sim-picker.

I've filled bug 1145622 to track this.

Comment 17

[Steve Chung [:steveck]]

Comment on attachment 8580581 [details] [review]
[gaia] steveck-chung:contact-call-button-fix > mozilla-b2g:master

Some unnecessary styling removed and commit squashed. I didn't add additional test for it because all the structure is almost the same, but just ping me if you think test is required, thanks!

Comment 18

[Francisco Jordano [:arcturus] [:francisco]]

Attachment: rtl-single-sim.png

Hi Steve, I'm attaching a screenshot with a rtl language and single sim, you can see how the text and the icon are overlapped.

When trying with two sims I must say that looks pretty good.

Comment 19

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8580581 [details] [review]
[gaia] steveck-chung:contact-call-button-fix > mozilla-b2g:master

The comment at the top describes a horrible hack in bug 1059336 that we had to do. We copied and pasted all the code from https://github.com/mozilla-b2g/gaia/blob/master/shared/style/buttons.css into https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/style/buttons.css

I'm setting f- only because we should get these files in sync for now. Please copy and paste (urgh) all of the shared buttons.css into the Dialer buttons.css.

We will have to keep doing this until we fix bug 1060293. I will put this bug on our radar for after 2.2 FC.

Comment 20

[Steve Chung [:steveck]]

Comment on attachment 8580581 [details] [review]
[gaia] steveck-chung:contact-call-button-fix > mozilla-b2g:master

Oops! sorry for some button styling missed. In the follow up commit I introduce .button from shared button and we don't have to copy/paste whole button css anymore.

Hi Doug,
About the button styling in dialer, I copied whole css from shared as you suggested. But I'm not sure it's a good move since there's lot of unused styling for dialer. It should not cause regression because contacts works fine with shared botton styling, but it would be safer if you could revisit again, thanks.

Comment 21

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8580581 [details] [review]
[gaia] steveck-chung:contact-call-button-fix > mozilla-b2g:master

Looks good. Thanks for taking this. We'll remove the unnecessary styling in bug 1060293. I tested this to make sure that I didn't see any of the extra CSS obviously bleeding out into other areas, and it seemed to be fine.

Comment 22

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8580581 [details] [review]
[gaia] steveck-chung:contact-call-button-fix > mozilla-b2g:master

LGTM, great job Steve, thanks a lot for the help!

Comment 23

[GitHub Autolander Bot]

http://docs.taskcluster.net/tools/task-graph-inspector/#wjKskjnWRjmRe1O915JJ0Q

The pull request failed to pass integration tests. It could not be landed, please try again.

Comment 24

[Steve Chung [:steveck]]

GIJ failed because of the test element selector. Fix right away.

Comment 25

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/f42afb94f987404a64b2bc99d951228da21c0bff

Comment 26

[Steve Chung [:steveck]]

Comment on attachment 8580581 [details] [review]
[gaia] steveck-chung:contact-call-button-fix > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): RTL feature request
[User impact] if declined: Call button's layout will be broken in dual sim with Arabic language setting.
[Testing completed]: N/A
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: N/A

Comment 27

[Sue]

Attachment: details.png

This issue has been verified successfully on Flame 3.0 with the same steps in comment 0. SIM Selection text does not overlap with the Dialer icon in RTL mode.
See attachment:details.png
Rate:0/5

Device: Flame 3.0 (pass)
Build ID               20150327160203
Gaia Revision          9cc496cecc37d7a29f9279827cdf6e4891211f67
Gaia Date              2015-03-27 13:55:18
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/44e454b5e93b
Gecko Version          39.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150327.192632
Firmware Date          Fri Mar 27 19:26:42 EDT 2015
Bootloader             L1TC000118D0

Comment 28

[SandKing]

Attachment: Verify picture: 2015-03-28-14-25-57.png

This issue has been verified successfully on Flame3.0
STR:
"STR as comment 0"
See picture: 2015-03-28-14-25-57.png
Repro rate:5/5

Flame 3.0:(unaffected)
Build ID               20150327160203
Gaia Revision          9cc496cecc37d7a29f9279827cdf6e4891211f67
Gaia Date              2015-03-27 13:55:18
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/44e454b5e93b
Gecko Version          39.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150327.192632
Firmware Date          Fri Mar 27 19:26:42 EDT 2015
Bootloader             L1TC000118D0

Comment 29

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/2e2ad80e0e0045b0faac2b7668c8688798c5e0b6

Comment 30

[SandKing]

Attachment: Bug picture: 2015-04-01-09-35-28.png

This issue has been verified successfully on Flame3.0
STR:
"STR as comment 0"
See picture:  2015-04-01-09-35-28.png
Repro rate:5/5

Flame 3.0:(pass)
Build ID               20150331002503
Gaia Revision          cc11248ab69f13e89416c8e6bb2e184187e72088
Gaia Date              2015-03-30 22:22:58
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/90a26917ee8f
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150331.034811
Firmware Date          Tue Mar 31 03:48:21 EDT 2015
Bootloader             L1TC000118D0