Closed
Bug 1112539
Opened 9 years ago
Closed 9 years ago
[Messages] Number-only contact details are broken in remove recipient confirmation dialog
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: azasypkin, Assigned: azasypkin)
References
Details
(Whiteboard: [sms-sprint-2.2S3])
Attachments
(3 files)
STR: * Open Messages app and navigate to Composer/New Message panel; * Focus on recipients input and pick existing contact that doesn't have phone type and carrier defined; * Tap on selected recipient pill and observe confirmation dialog. Expected result: * Dialog should show phone number only in contact details section; Actual result: * Contact details section consists of two lines, first line contains "," only, second line contains actual phone number. Not sure if it's possible to have contact with phone number without phone type defined in real, but I see it a lot for contacts from workload data that's annoying :)
Assignee | ||
Comment 1•9 years ago
|
||
Assignee | ||
Comment 2•9 years ago
|
||
Comment on attachment 8537781 [details] [review] GitHub pull request URL Hey Julien, Here is not so nice BB rule override that solves the problem, but don't have any better idea for now other then separate webcomponent for contact details. Could you please take a look? Thanks!
Attachment #8537781 -
Flags: review?(felash)
Comment 3•9 years ago
|
||
Comment on attachment 8537781 [details] [review] GitHub pull request URL I'd prefer to get rid of the bad selector in the BB ;) If it's used somewhere we should rather move it to the specific app, possibly using a class, than have such a generic rule applied globally. I see that Arnau is away until 9th January, so maybe you'll want to look at this yourself until he comes back?
Attachment #8537781 -
Flags: review?(felash)
Assignee | ||
Comment 4•9 years ago
|
||
+ one more usecase to verify from bug 1114046 - carrier-less contacts when carrier separator isn't whitespace, eg. comma
Assignee | ||
Comment 6•9 years ago
|
||
Assignee | ||
Comment 7•9 years ago
|
||
Attachment #8542087 -
Flags: ui-review?(jelee)
Assignee | ||
Comment 8•9 years ago
|
||
https://github.com/mozilla-b2g/gaia/blob/master/shared/style/confirm.css#L170 Hey Jenny, In this patch I'm going to change some BB shared style that will affect one of the settings dialogs and :EragonJ pointed that you can help with settings stuff too :) So in attachment 8542083 [details] you can see current style and in attachment 8542087 [details] what happens if my patch is applied. I just want to confirm if I need to preserve current style because? As it's kind of weird IMO :) Thanks!
Assignee | ||
Updated•9 years ago
|
Whiteboard: [sms-sprint-2.2S3]
Comment on attachment 8542087 [details]
New call waiting dialog (Settings)
Looks good!
Attachment #8542087 -
Flags: ui-review?(jelee) → ui-review+
Assignee | ||
Comment 10•9 years ago
|
||
Comment on attachment 8537781 [details] [review] GitHub pull request URL (In reply to Jenny Lee from comment #9) > Comment on attachment 8542087 [details] > New call waiting dialog (Settings) > > Looks good! Thanks Jenny! Hey Julien and Arnau, Here's the patch that updates confirm dialog BB style from "p span" to "p > span". Ideally it should not change anything for dialogs that use "p->span" markup inside as for almost all usages I've found [1], "span" is direct child of "p". Exception from that rule is confirmed in comment 9. Thanks! [1] https://github.com/mozilla-b2g/gaia/pull/26846#discussion_r22243571
Attachment #8537781 -
Flags: review?(rnowmrch)
Attachment #8537781 -
Flags: review?(felash)
Comment 11•9 years ago
|
||
Comment on attachment 8537781 [details] [review] GitHub pull request URL r=me with one optional nit, good cleanup !
Attachment #8537781 -
Flags: review?(felash) → review+
Comment on attachment 8537781 [details] [review] GitHub pull request URL LGTM, thanks Oleg. Sorry for the late review, I was on PTO. Please ping Pavel for future BB reviews ;)
Attachment #8537781 -
Flags: review?(rnowmrch) → review+
Assignee | ||
Comment 13•9 years ago
|
||
No worries, thanks a lot, Arnau! Treeherder is green, so landed. Master: https://github.com/mozilla-b2g/gaia/commit/c1c7403fab9d59c6d400ec371499ff8a277bb141
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•