Closed
Bug 1034600
Opened 10 years ago
Closed 10 years ago
[Messages] Suggestions list is badly located
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(blocking-b2g:2.0+, b2g-v2.0 verified, b2g-v2.1 verified)
People
(Reporter: julienw, Assigned: azasypkin)
References
Details
(Keywords: regression, Whiteboard: [p=2])
Attachments
(3 files)
Hey Victoria, I think the suggestion list is badly placed now: there is a huge padding between the list and the recipients panel. Can you confirm it's wrong?
Flags: needinfo?(vpg)
Reporter | ||
Comment 2•10 years ago
|
||
I think this is a regression from bug 949457, but the underlying reason is that we use the same element to show the messages in a thread and to show the contact suggestion list. This is really wrong and in this bug we need to change it too.
Reporter | ||
Updated•10 years ago
|
Blocks: sms-sprint-2.0S6
Whiteboard: [p=2]
Reporter | ||
Updated•10 years ago
|
Target Milestone: --- → 2.0 S6 (18july)
Updated•10 years ago
|
blocking-b2g: 2.0? → 2.0+
Updated•10 years ago
|
Keywords: regression
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
OS: Linux → Gonk (Firefox OS)
QA Contact: azasypkin
Hardware: x86_64 → ARM
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → azasypkin
QA Contact: azasypkin
Assignee | ||
Comment 3•10 years ago
|
||
Hey Julien, Here is the patch I came up with, could you please review it? Thanks!
Attachment #8454303 -
Flags: review?(felash)
Reporter | ||
Comment 4•10 years ago
|
||
Comment on attachment 8454303 [details] [review] GitHub pull request URL Looks good but I added some comments on the PR. Especially vh is slow (bug 1020469); it's ok to use it if the selector is not used in a place where the keyboard can be displayed/hidden but otherwise we should unfortunately avoid it. I also think this is a good time to get rid of the dynamic <ul> (maybe even simply replace your new <div> with that <ul>) used for the suggestions list. Good work, let's do another round !
Attachment #8454303 -
Flags: review?(felash)
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8454303 [details] [review] GitHub pull request URL (In reply to Julien Wajsberg [:julienw] (PTO Monday 14th July) from comment #4) > Comment on attachment 8454303 [details] [review] > GitHub pull request URL > > Looks good but I added some comments on the PR. Especially vh is slow (bug > 1020469); it's ok to use it if the selector is not used in a place where the > keyboard can be displayed/hidden but otherwise we should unfortunately avoid > it. > > I also think this is a good time to get rid of the dynamic <ul> (maybe even > simply replace your new <div> with that <ul>) used for the suggestions list. > > Good work, let's do another round ! Hey Julien, Please see updated PR (in a separate commit). Thanks!
Attachment #8454303 -
Flags: review?(felash)
Reporter | ||
Comment 6•10 years ago
|
||
Comment on attachment 8454303 [details] [review] GitHub pull request URL I added some more comments; especially I'd rather set up the events once for all at init; and I think we should show/hide the element explicitely instead of relying to its height being 0.
Attachment #8454303 -
Flags: review?(felash)
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8454303 [details] [review] GitHub pull request URL (In reply to Julien Wajsberg [:julienw] (PTO Monday 14th July) from comment #6) > Comment on attachment 8454303 [details] [review] > GitHub pull request URL > > I added some more comments; especially I'd rather set up the events once for > all at init; and I think we should show/hide the element explicitely instead > of relying to its height being 0. Fixed nits and added show\hide behaviour. Thanks!
Attachment #8454303 -
Flags: review?(felash)
Reporter | ||
Comment 8•10 years ago
|
||
Comment on attachment 8454303 [details] [review] GitHub pull request URL r=me with some non-blocking nits, so it's up to you. thanks !
Attachment #8454303 -
Flags: review?(felash) → review+
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #8) > Comment on attachment 8454303 [details] [review] > GitHub pull request URL > > r=me with some non-blocking nits, so it's up to you. > > thanks ! Thanks for review! Master: https://github.com/mozilla-b2g/gaia/commit/0a0b4c9d4bccb4fb96fba1e8b855b1afdc96fb31
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 10•10 years ago
|
||
v2.0: https://github.com/mozilla-b2g/gaia/commit/bcecf586feeb900cab97f5cac0d51800c60917ed
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
Comment 11•10 years ago
|
||
Hi Julien, Could you please provide a repro steps, thanks!
Flags: needinfo?(felash)
Reporter | ||
Comment 12•10 years ago
|
||
STR: 0. Prerequisite: have several contacts starting with "A" 1. Open Messages app 2. press "new message" button 3. tap the recipient panel to be able to enter new recipients 4. enter "A" => The suggestions list panel should start right below the recipient panel.
Flags: needinfo?(felash)
Comment 13•10 years ago
|
||
According to the STR in Comment#12, this issue has been verified successfully on Flame 2.0 and 2.1 See attachment: Verify_1034600.MP4 Reproducing rate: 0/5 Flame 2.0 build: Gaia-Rev 856863962362030174bae4e03d59c3ebbc182473 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/e40fe21e37f1 Build-ID 20141208000206 Version 32.0 Flame 2.1 build: Gaia-Rev 38e17b0219cbc50a4ad6f51101898f89e513a552 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/8b92c4b8f59a Build-ID 20141205001201 Version 34.0
Updated•10 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•