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

VERIFIED FIXED in Firefox OS v2.2

Status

Firefox OS
Gaia::Contacts
P2
normal
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: oliverthor, Assigned: steveck)

Tracking

(Blocks: 1 bug)

unspecified
2.2 S9 (3apr)
ARM
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-moztrap +

Firefox Tracking Flags

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

Details

(Whiteboard: [3.0-Daily-Testing])

Attachments

(7 attachments)

(Reporter)

Description

3 years ago
Created attachment 8572214 [details]
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
(Reporter)

Updated

3 years ago
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]
(Reporter)

Updated

3 years ago
Blocks: 1071918
(Reporter)

Updated

3 years ago
Blocks: 1064489
No longer blocks: 1071918
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)

Updated

3 years ago
Flags: needinfo?(pacorampas)
Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15396/
Flags: in-moztrap+
(Assignee)

Updated

3 years ago
See Also: → bug 1141464
(Assignee)

Comment 6

3 years ago
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)
Duplicate of this bug: 1141464
(Assignee)

Comment 9

3 years ago
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)

Comment 11

3 years ago
Created attachment 8579865 [details]
RTL_contact detail.png

Attached the screenshot, the phone numbers and email should be right aligned like this. Thanks!
Flags: needinfo?(fshih)

Comment 12

3 years ago
Hi Steve, can you take this one?
Flags: needinfo?(schung)
(Assignee)

Comment 13

3 years ago
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 14

3 years ago
Created attachment 8580581 [details] [review]
[gaia] steveck-chung:contact-call-button-fix > mozilla-b2g:master
(Assignee)

Comment 15

3 years ago
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)
See Also: → bug 1145622
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+
(Assignee)

Comment 17

3 years ago
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)
Created attachment 8581799 [details]
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-
(Assignee)

Comment 20

3 years ago
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

Updated

3 years ago
Keywords: checkin-needed
Keywords: checkin-needed

Updated

3 years ago
Keywords: checkin-needed
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Updated

3 years ago
Keywords: checkin-needed

Comment 23

3 years ago
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.
(Assignee)

Comment 24

3 years ago
GIJ failed because of the test element selector. Fix right away.
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Updated

3 years ago
Keywords: checkin-needed

Comment 25

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

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Comment 26

3 years ago
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)
(Assignee)

Updated

3 years ago
status-b2g-master: affected → fixed

Updated

3 years ago
Attachment #8580581 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+

Comment 27

3 years ago
Created attachment 8584924 [details]
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

Updated

3 years ago
QA Whiteboard: [QAnalyst-Triage+][rtl-impact] → [QAnalyst-Triage+][rtl-impact][MGSEI-Triage+]
status-b2g-master: fixed → verified

Comment 28

3 years ago
Created attachment 8584961 [details]
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
v2.2: https://github.com/mozilla-b2g/gaia/commit/2e2ad80e0e0045b0faac2b7668c8688798c5e0b6
status-b2g-v2.2: affected → fixed
Target Milestone: --- → 2.2 S9 (3apr)

Comment 30

3 years ago
Created attachment 8586513 [details]
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

Updated

3 years ago
Status: RESOLVED → VERIFIED
status-b2g-v2.2: fixed → verified
Blocks: 1150802
You need to log in before you can comment on or make changes to this bug.