Closed
Bug 873498
Opened 11 years ago
Closed 11 years ago
[MMS] [UX] Compose. Contacts suggestion should not show the contact's photo
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(blocking-b2g:leo+, b2g18 fixed)
People
(Reporter: vicky, Assigned: rwaldron)
References
Details
Attachments
(2 files)
Showing the picture makes the contacts suggestion list too busy and it will affect performance, so since it is not shown in the specs I suggest to be removed.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → waldron.rick
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #754170 -
Flags: review?(felash)
Updated•11 years ago
|
blocking-b2g: --- → leo?
Comment 2•11 years ago
|
||
Comment on attachment 754170 [details] [review] Fix for 873498 r=me with the nit and rebase
Attachment #754170 -
Flags: review?(felash) → review+
Assignee | ||
Comment 3•11 years ago
|
||
Landed https://github.com/mozilla-b2g/gaia/commit/c7f5c27137cc36f51c9da194292e1e5055675017
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 4•11 years ago
|
||
Rick, you made the same error than me when you rebased: you rolled back a previous change (see [1], and as a result, failed the tests. Yay for the tests ! Please push a follow-up or I'll back out :) [1] https://github.com/mozilla-b2g/gaia/commit/c7f5c27137cc36f51c9da194292e1e5055675017#L2R1390
Flags: needinfo?(waldron.rick)
Comment 5•11 years ago
|
||
This fixes the bad rebase, which reverted one line from Bug 872395, and made the tests fail.
Attachment #754541 -
Flags: review?(gnarf37)
Flags: needinfo?(waldron.rick)
Comment 6•11 years ago
|
||
Comment on attachment 754541 [details] [diff] [review] fix tests Borja, could you please have a look to this one-liner fix so that we can fix the tests today ? Note that I don't know how it comes that Travis is green now, the tests are always failing on my computer without this change...
Attachment #754541 -
Flags: review?(fbsc)
Updated•11 years ago
|
Attachment #754541 -
Flags: review?(schung)
Attachment #754541 -
Flags: review?(gnarf37)
Attachment #754541 -
Flags: review?(fbsc)
Comment 7•11 years ago
|
||
Comment on attachment 754541 [details] [diff] [review] fix tests Although it's a little bit confusing that having a " |" after the type string, I think it's ok only for interpolation only.
Attachment #754541 -
Flags: review?(schung) → review+
Comment 8•11 years ago
|
||
(In reply to Steve Chung from comment #7) > Comment on attachment 754541 [details] [diff] [review] > fix tests > > Although it's a little bit confusing that having a " |" after the type > string, I think it's ok only for interpolation only. That's what the wireframe says (maybe it's better to do this with border in CSS), and that was not my code ;) thanks
Comment 9•11 years ago
|
||
master (2nd commit): 9885fc228d8b1f4cac9151d3d3afb37298b2c3cb
Comment 10•11 years ago
|
||
to uplifters, there are 2 commits on master : c7f5c27137cc36f51c9da194292e1e5055675017 9885fc228d8b1f4cac9151d3d3afb37298b2c3cb
Comment 11•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #8) > (In reply to Steve Chung from comment #7) > > Comment on attachment 754541 [details] [diff] [review] > > fix tests > > > > Although it's a little bit confusing that having a " |" after the type > > string, I think it's ok only for interpolation only. > > That's what the wireframe says (maybe it's better to do this with border in > CSS), and that was not my code ;) > > thanks Regarding the "|" in the auto suggestion. You should not be following the wireframe for this.. You should be following however Visual Design has specified the auto suggestions to be laid out. The wireframes only detail the information to be delivered and the order/hierarchy of the information being delivered. They do not detail treatment or styling.
Comment 12•11 years ago
|
||
Yep, you're right, the visual design shows the same, I checked ;) Thanks ! However I'm more concerned about accessibility, because for a screen reader, '|' is just like another character, and it will probably read it. Maybe we'll need to put it in a <span> with an aria role "presentation". Or use CSS. But this is for another time.
Comment 13•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #12) > Yep, you're right, the visual design shows the same, I checked ;) Thanks ! > > However I'm more concerned about accessibility, because for a screen reader, > '|' is just like another character, and it will probably read it. Maybe > we'll need to put it in a <span> with an aria role "presentation". Or use > CSS. But this is for another time. I agree regarding screen reading. when i refer across the applications i note we actually have a different layout s for instances where the type/phonenumber line appears. sometimes they are divided by a "|" (message app) sometimes by a "," (call log) and sometimes just space. Going to RFI Victoria to review the type/phonenumber layout again in all instances.
Flags: needinfo?(vpg)
Reporter | ||
Comment 14•11 years ago
|
||
I preffer the "|" as a separator, but taking a look at call log, and for the sake of consistency we'll go for the "," between type and phone number. Thanks!
Flags: needinfo?(vpg)
Updated•11 years ago
|
blocking-b2g: leo? → leo+
Comment 15•11 years ago
|
||
Made new bug for separator issue https://bugzilla.mozilla.org/show_bug.cgi?id=876754
Comment 16•11 years ago
|
||
This bug was partially uplifted. Uplifted c7f5c27137cc36f51c9da194292e1e5055675017 to: Uplifted 9885fc228d8b1f4cac9151d3d3afb37298b2c3cb to: v1-train: 365475a31ca3e00f863d6fd5fb0b17accd5d98ff Commit c7f5c27137cc36f51c9da194292e1e5055675017 didn't uplift to branch v1-train
Comment 17•11 years ago
|
||
(In reply to John Ford [:jhford] -- If you expect a reply from me, use needsinfo? instead of CC from comment #16) > This bug was partially uplifted. > > Uplifted c7f5c27137cc36f51c9da194292e1e5055675017 to: > Uplifted 9885fc228d8b1f4cac9151d3d3afb37298b2c3cb to: > v1-train: 365475a31ca3e00f863d6fd5fb0b17accd5d98ff > > Commit c7f5c27137cc36f51c9da194292e1e5055675017 didn't uplift to branch > v1-train I've reverted this commit with e1c59ba
Comment 19•11 years ago
|
||
Uplifted c7f5c27137cc36f51c9da194292e1e5055675017 to: v1-train: 0c379e27b8ed2983101bb68aacbc85c9670f0aea Uplifted 9885fc228d8b1f4cac9151d3d3afb37298b2c3cb to: v1-train: 0154e3133ef811ca3e1fa192b4f1be653b8a40f1
status-b2g18:
--- → fixed
Flags: needinfo?(jhford)
Assignee | ||
Comment 20•11 years ago
|
||
The spec should be updated and re-published to reflect the change: "|" => ","
Updated•11 years ago
|
Attachment mime type: text/plain → text/x-github-pull-request
You need to log in
before you can comment on or make changes to this bug.
Description
•