905260 - [messages] padding issues in the recipient panel

Status: RESOLVED FIXED

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

Description

[Julien Wajsberg [:julienw]]

Various problems I've seen while reviewing bug 884890:

* the margin (or padding, I don't know) top is a little too large in multiline mode
* we should remove the padding for placeholders; when we add a big big recipient (that takes all the available space), then the placeholder is at the next line, and the cursor does not seem to be at the right place. It's really aligned with the start of the text in the above recipient, but since placeholders don't have borders, it doesn't look good.

Comment 1

[Julien Wajsberg [:julienw]]

Attachment: screenshot

This screenshot shows both issues.

Comment 2

[Julien Wajsberg [:julienw]]

Note that people in our target market usually have very long names ;)

Comment 3

[Julien Wajsberg [:julienw]]

Mike's next work ? ;)

Comment 4

[Julien Wajsberg [:julienw]]

We can also see the second problem when typing a long recipient and it gets moved to the next line while the user is typing because it's too long. While it's not in "finished" state (with the border) we see the extra padding.

Comment 5

[Mike Pennisi [:jugglinmike]]

I'll take this and begin work as soon as we land Bug 905273 (which necessarily modifies the structure of this component).

Comment 6

[Mike Pennisi [:jugglinmike]]

Attachment: 905260-recipients-padding.patch

Hi Julien,

From comment 0:

> * we should remove the padding for placeholders; when we add a big big recipient (that takes all the available space), then the placeholder is at the next line, and the cursor does not seem to be at the right place. It's really aligned with the start of the text in the above recipient, but since placeholders don't have borders, it doesn't look good.

If we remove the padding, then recipients will suddenly shift when they become "assimilated". This could be especially disorienting when adding a recipient that (prior to being accepted) just fits on the final line. After accepting this recipient, it will receive additional padding and suddenly render on a new line.

I think the real problem here is that the placeholders don't have borders, so that when they are "assimilated", they shift by one pixel in both dimensions. This patch renders "contenteditable" recipients with a transparent border so that this no longer occurs.

Does this approach make sense to you?

Comment 7

[Julien Wajsberg [:julienw]]

Attachment: screenshot with the patch

Victoria, could you please have a look to this screenshot ? Especially I'd like to know if the vertical space (1) between the header and the recipient looks good to you, same for the vertical space (2) between the last recipient and the recipient panel border.

Currently (2) is the same as the specified space between 2 lines. But for me, the fact that (1) and (2) are different feels weird. And I don't think we should lower (1) either. Therefore I'd like your advice on this.

Comment 8

[Julien Wajsberg [:julienw]]

(In reply to Mike Pennisi [:jugglinmike] from comment #6)
> Created attachment 793726 [details] [diff] [review]
> 905260-recipients-padding.patch
> 
> Hi Julien,
> 
> From comment 0:
> 
> > * we should remove the padding for placeholders; when we add a big big recipient (that takes all the available space), then the placeholder is at the next line, and the cursor does not seem to be at the right place. It's really aligned with the start of the text in the above recipient, but since placeholders don't have borders, it doesn't look good.
> 
> If we remove the padding, then recipients will suddenly shift when they
> become "assimilated". This could be especially disorienting when adding a
> recipient that (prior to being accepted) just fits on the final line. After
> accepting this recipient, it will receive additional padding and suddenly
> render on a new line.
> 
> I think the real problem here is that the placeholders don't have borders,
> so that when they are "assimilated", they shift by one pixel in both
> dimensions. This patch renders "contenteditable" recipients with a
> transparent border so that this no longer occurs.

Yep this is a good change, thanks.

However, it still looks weird when the placeholder is the first in a line, and there are other recipients above. I understand your argument though, and I have a counter argument ;)

Could we, for placeholders, nullify the left-padding, but double the right-padding ? This way the whole placeholder would have the same width as a assimilated recipient, and would correctly go to the next line if necessary.

What do you think ?

Comment 9

[Julien Wajsberg [:julienw]]

(In reply to Julien Wajsberg [:julienw] from comment #8)

> Could we, for placeholders, nullify the left-padding, but double the
> right-padding ? This way the whole placeholder would have the same width as
> a assimilated recipient, and would correctly go to the next line if
> necessary.

As discussed on IRC, when doing this, when assimilated the recipient would "jump" to the right. I just tried on gmail, and they're actually doing just this. I think this is also less important than you think because usually users will choose an existing contact from the suggestion list rather than typing a whole number.

Another solution that was suggested by Jeremie Patonnier, is to have a pale border for placeholders.

Flagging NI on Victoria for guidance on this issue.

Comment 10

[Mike Pennisi [:jugglinmike]]

Also note that the vertical spacing above the first line (1) is necessary to provide a visual affordance of the recipients list in single-line mode. If we reduce that spacing during multi-line mode, we will have to re-position the "To:" label when switching modes, which seems undesirable.

Comment 11

[Victoria Gerchinhoren [:vicky]]

Hi Julien, I am attaching the original specs for this. As you can see, you'll never have this panel that big, the recipients will be placed behind the header leaving a small portion visible giving a hint of more information sitting there. So when the user adds recipients, the list would move up (behind the header). Therefore, the advise you're asking for is a bit obsolete. Is there a strong reason why this is not following the specs? Please let me know. 

Thanks!

Comment 12

[Victoria Gerchinhoren [:vicky]]

Attachment: Original design, the recipients area does not grow when adding recipients

Comment 13

[Julien Wajsberg [:julienw]]

Victoria, these screenshots are when we slide down the panel. I agree you gave a specification for when the panel is slid up, but we have no specification for this when the panel is slid down.

Comment 14

[Victoria Gerchinhoren [:vicky]]

Hi Julien et all,

I understand what you say about 2 different sizes on top and bottom, but the spacing you show in this screenshot looks good for a few reasons:

1. Since we gave room to have a hint of what it is hidden behind the header, that bigger spacing before the header is needed to make a part of the hidden bubbles visualble and It's better not to reposition the "to" field and the "+" after pulling down that area.
2. If we do not move the to space and the +, moving the recipients bubbles upper will missalign them from this elements.
3. The user needs space to tap into the bubbles.

So I would put first all the above fundaments before changing those spaces for the sake of modularity.

Thanks!
Victoria

 (In reply to Julien Wajsberg [:julienw] from comment #7)
> Created attachment 793979 [details]
> screenshot with the patch
> 
> Victoria, could you please have a look to this screenshot ? Especially I'd
> like to know if the vertical space (1) between the header and the recipient
> looks good to you, same for the vertical space (2) between the last
> recipient and the recipient panel border.
> 
> Currently (2) is the same as the specified space between 2 lines. But for
> me, the fact that (1) and (2) are different feels weird. And I don't think
> we should lower (1) either. Therefore I'd like your advice on this.

Comment 15

[Julien Wajsberg [:julienw]]

If it looks good for you, it looks good for me :) Thanks !

There is another question for you in comment 9 ;-)

To summarize:
See attachment 790331 [details]: when on a new line, the cursor for the currently-editing placeholder is aligned with the text above, which looks not good to me. I'd rather see it aligned with the left border instead.

However, this is not possible to do this technically. Therefore, there are 2 possible solutions:

* solution 1: we remove the padding-left for placeholders, but we double the padding-right (+). The consequence to this is that when the user types a number and then press enter, the number will "jump" to the right.
I just tried this on gmail, and they're actually doing just this. I think this is also probably less important than what we might think because usually users will choose an existing contact from the suggestions list rather than typing a whole number.

(+) doubling the padding-right ensures that the number will go on the next line if necessary, just like an "assimilated" recipient.

* solution 2: we have a border for placeholders, just like "assimilated" recipients. Of course, the border would be a lot paler and lighter, and we would still have a white background. For this solution.

So... we need to choose between those solutions... or just keep what we have now, but I think either of these solutions would be better.

For solution 2, we'll need the color you propose for the border.

Thanks Victoria !

Comment 16

[Victoria Gerchinhoren [:vicky]]

Hi Julien,
I think the best is Solution Nbr 1, i've seen it many times and that "jump" is not uncomfortable, so let's move forward with it.

Thanks!

Comment 17

[Julien Wajsberg [:julienw]]

So Mike, we have our way forward now :)

Comment 18

[Mike Pennisi [:jugglinmike]]

Yessir! I hope to get to this later today

Comment 19

[Julien Wajsberg [:julienw]]

Mike, do you think you could wrap-up this today ? I think there is not much to do to land it, right ?

Comment 20

[Mike Pennisi [:jugglinmike]]

Attachment: 905260-recipients-padding-v2.patch

Hey Julien,

You're right--there was very little to update based on Victoria's feedback :)

Comment 21

[Julien Wajsberg [:julienw]]

Comment on attachment 798542 [details] [diff] [review]
905260-recipients-padding-v2.patch

r=me

thanks !

Comment 22

[Mike Pennisi [:jugglinmike]]

master: https://github.com/mozilla-b2g/gaia/commit/bca41fe990f748d23035c1c4588cff73c21b8ad4

Thanks Julien and Victoria!

Comment 23

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/99c3a234ddb2