Closed Bug 1139179 Opened 9 years ago Closed 9 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)

VERIFIED FIXED
2.2 S9 (3apr)
blocking-b2g 2.2+
Tracking Status
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: onelson, Assigned: steveck)

References

Details

(Whiteboard: [3.0-Daily-Testing])

Attachments

(7 files)

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
QA Whiteboard: [QAnalyst-Triage?][rtl-impact]
Flags: needinfo?(pbylenga)
Whiteboard: [3.0-Daily-Testing]
Flags: needinfo?(pbylenga)
QA Whiteboard: [QAnalyst-Triage?][rtl-impact] → [QAnalyst-Triage+][rtl-impact]
Blocks: dialer-rtl
Blocks: contacts-rtl
No longer blocks: dialer-rtl
Triage: P2
blocking-b2g: --- → 2.2?
Priority: -- → P2
triage: obvious UI truncation, on a new feature
blocking-b2g: 2.2? → 2.2+
Can we please get someone assigned to this as this is a blocker? thanks
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?
Flags: needinfo?(pacorampas)
Flags: needinfo?(pacorampas)
Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15396/
Flags: in-moztrap+
See Also: → 1141464
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.
Flags: needinfo?(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!
Flags: needinfo?(francisco)
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)?
Flags: needinfo?(cawang)
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!
Flags: needinfo?(cawang) → needinfo?(fshih)
Attached image RTL_contact detail.png
Attached the screenshot, the phone numbers and email should be right aligned like this. Thanks!
Flags: needinfo?(fshih)
Hi Steve, can you take this one?
Flags: needinfo?(schung)
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.
Assignee: nobody → schung
Flags: needinfo?(schung)
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.
Attachment #8580581 - Flags: feedback?(francisco)
Attachment #8580581 - Flags: feedback?(drs)
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.
Attachment #8580581 - Flags: feedback?(francisco) → feedback+
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!
Attachment #8580581 - Flags: review?(francisco)
Attached image 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.
Attachment #8580581 - Flags: review?(francisco) → review-
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.
Attachment #8580581 - Flags: feedback?(drs) → feedback-
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.
Attachment #8580581 - Flags: review?(francisco)
Attachment #8580581 - Flags: review?(drs)
Attachment #8580581 - Flags: review-
Attachment #8580581 - Flags: feedback-
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.
Attachment #8580581 - Flags: review?(drs) → review+
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!
Attachment #8580581 - Flags: review?(francisco) → review+
Keywords: checkin-needed
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.
GIJ failed because of the test element selector. Fix right away.
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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
Attachment #8580581 - Flags: approval-gaia-v2.2?(bbajaj)
Attachment #8580581 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
Attached image 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
QA Whiteboard: [QAnalyst-Triage+][rtl-impact] → [QAnalyst-Triage+][rtl-impact][MGSEI-Triage+]
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
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: