Closed Bug 882086 Opened 11 years ago Closed 11 years ago

[MMS] [UX] Message thread. Spacing between the last bubble and input field should be 1.5 rem

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(blocking-b2g:-)

RESOLVED FIXED
blocking-b2g -

People

(Reporter: vicky, Assigned: jugglinmike)

References

Details

Attachments

(3 files, 1 obsolete file)

When showing the end of a long thread (longer than the space in the thread's visible canvas) the idle position should be the last portion of the thread. In this case the space between the last bubble and the input field should be 1.5 rem. Same situation when the keyboard is displayed.

See attachments for illustration
Blocks: 872514
QA Contact: mike
Assignee: nobody → mike
QA Contact: mike
Hi Julien,

As I noted in the commit message, this patch also resolves Bug 875362
Attachment #767968 - Flags: review?(felash)
my understanding is that this simple patch should be leo+. Probably bug 875362 is more important than ths one though, but since both are fixed in the same time I'm requesting leo in both bugs.
blocking-b2g: --- → leo?
it's polish, so minusing. it's ok to fix in the other bug and close this one when landed though ;)
blocking-b2g: leo? → -
Comment on attachment 767968 [details] [diff] [review]
Correct spacing after final message bubble

Review of attachment 767968 [details] [diff] [review]:
-----------------------------------------------------------------

so, we could land it like this, the code is ok to me (even if I would have liked to find a solution to have less similar code in both cases in that function, but I can't find anything sensible now).

However, I've noticed that something else is not right here and it makes sense to make it work now. That's polish, that means that if we don't do it now, we won't probably have it in 1.1 ;)

see the MMS wireframe [1] page 53, 3rd screenshot, annotation 01, we can see we need to keep a small space that shows the threadlist is still behind, so that the user can tap on it to hide the keyboard. I'll needinfo Victoria to know how many pixels this small space should be, but I think you can very easily do that in that bug. Are you ok with this Mike ?

[1] https://mozilla.box.com/s/wzgsb3lkqglv0dnfdgzs/1/864518456/7994808358/1

::: apps/sms/js/thread_ui.js
@@ +662,5 @@
>          newHeight;
>        composeForm.style.height = newHeight + 'px';
>  
> +      // Set the bottom border of the container so the Compose field does not
> +      // occlude the messages. `padding-bottom` is not used because it it is

nit: "it it"

@@ +689,5 @@
>      // We calculate the height of the Compose form which contains the input
>      composeForm.style.height = newHeight + 'px';
>  
> +    // Set the bottom border of the container so the Compose field does not
> +    // occlude the messages. `padding-bottom` is not used because it it is

nit: "it it"
Victoria, see previous comment, could you tell us how many pixels that space should be please ?
Flags: needinfo?(vpg)
Hi Julien,
I guess 3 rem would be ok (30 px).

Thanks!
Flags: needinfo?(vpg)
Hi Julien,

Thanks for the feedback. I've included a fix for the bug you identified in comment 5 (with the value Victoria recommended in comment 7). As before, this patch also resolves Bug 875362
Attachment #767968 - Attachment is obsolete: true
Attachment #767968 - Flags: review?(felash)
Attachment #768969 - Flags: review?(felash)
Comment on attachment 768969 [details] [diff] [review]
Correct spacing after final message bubble

Review of attachment 768969 [details] [diff] [review]:
-----------------------------------------------------------------

This is not working as expected.

Here are the 2 issues I saw, the second one was existing in the previous patch, I just haven't seen it:
* the sliver should not be used in the "new thread" view.
* when tapping in the sliver, the keyboard correctly hides. However, when tapping the input again, the keyboard shows up, and the input is not resized. I checked that it was not working in the previous patch either. But it was working before :)

Also, could you move that patch to Bug 875362 that is leo+ ?
Attachment #768969 - Flags: review?(felash) → review-
Sure thing, Julien. I've included the latest version of this patch in Bug 875362. That patch will resolve this bug (the only way I could prevent that would involve intentionally setting the last message's margin to an incorrect value, which seems counter-productive).

Should we mark this bug as "Duplicate", or just wait until Bug 875362 lands and mark it as "Works for Me" ?
Flags: needinfo?(felash)
I'd say, let's mark it resolved/fixed once it lands, so that it's verified by QA.
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: needinfo?(felash)
Resolution: --- → FIXED
oops, I didn't mean to do this now.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
was fixed by bug 875362.
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: