1034600 - [Messages] Suggestions list is badly located

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

Attachment: 2014-07-04-15-30-48.png

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?

Comment 1

[Victoria Gerchinhoren [:vicky]]

Yes, of course it is wrong.

Comment 2

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

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.

Comment 3

[Oleg Zasypkin [:azasypkin]]

Attachment: GitHub pull request URL

Hey Julien,

Here is the patch I came up with, could you please review it?

Thanks!

Comment 4

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

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 !

Comment 5

[Oleg Zasypkin [:azasypkin]]

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!

Comment 6

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

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.

Comment 7

[Oleg Zasypkin [:azasypkin]]

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!

Comment 8

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

Comment on attachment 8454303 [details] [review]
GitHub pull request URL

r=me with some non-blocking nits, so it's up to you.

thanks !

Comment 9

[Oleg Zasypkin [:azasypkin]]

(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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/bcecf586feeb900cab97f5cac0d51800c60917ed

Comment 11

[Shine(Leave from Mozilla)]

Hi Julien, 
Could you please provide a repro steps, thanks!

Comment 12

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

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.

Comment 13

[Shine(Leave from Mozilla)]

Attachment: video

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