Closed Bug 1041765 Opened 6 years ago Closed 6 years ago

[Messages] Thread view redesign

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(feature-b2g:2.1, tracking-b2g:backlog, b2g-v2.1 fixed)

VERIFIED FIXED
2.1 S3 (29aug)
feature-b2g 2.1
tracking-b2g backlog
Tracking Status
b2g-v2.1 --- fixed

People

(Reporter: julienw, Assigned: azasypkin)

References

Details

(Whiteboard: [p=2])

Attachments

(4 files, 1 obsolete file)

See bug 1030925 for the general spec.

In this bug we'll handle the refresh for the Thread view, excluding displaying the contact pictures.

Fang, we need a clear spec for this.
Flags: needinfo?(zfang)
Jenny, we need some UX advice on how we handle the "resend message" action with the planned redesign. In the current version, we have the "error" icon that is actionable. But this icon seems to be removed in the planned redesign.

Thanks !
Flags: needinfo?(jelee)
ni the right Fang :)
Flags: needinfo?(zfang) → needinfo?(fshih)
Just checked with Fang, the icon is still there and I intend to keep it actionable. There should be no problem, let's wait and see the new layout!
Flags: needinfo?(jelee)
Attached file 2.1_MessagingThread_VisualSpec.zip (obsolete) —
Attached the messaging thread visual spec. Thanks!
Flags: needinfo?(fshih)
Hey Fang,

Thanks for the spec! After a quick look I have several concerns regarding to the thread view spec:

* Previously we had some space (4 rem) at the left side of every message so that when user enters "Delete messages" mode that space is used to show checkbox for every message without affecting message layout. New spec got rid of this space, where should we show checkboxes now?

* I noticed that for message in "sending" mode we use different text color, but in case of sending MMS, should we somehow change styles for subject and multimedia content (image thumbnails, video and audio placeholders)?

* Using box-shadow together with border-radius for message bubbles will likely affect thread view performance, can we have some fallback plan just in case we notice that problems and don't find any solution from our side?

Thanks!
Assignee: nobody → azasypkin
Flags: needinfo?(fshih)
(In reply to Oleg Zasypkin [:azasypkin] from comment #5)
> Hey Fang,
> 
> Thanks for the spec! After a quick look I have several concerns regarding to
> the thread view spec:
> 
> * Previously we had some space (4 rem) at the left side of every message so
> that when user enters "Delete messages" mode that space is used to show
> checkbox for every message without affecting message layout. New spec got
> rid of this space, where should we show checkboxes now?

And I'd add that that space was here so that the checkbox can be displayed without moving everything (which is costly performance-wise). Although we can try to revisit this.
Hi Oleg,

Just attached the new spec for messaging thread view.

Spec includes:

* Messaging thread view
Adjusted the maximum width of the bubble size: 26.5 rem.

* Delete Message layout 
Thanks for pointing out the problem. I made a minor space adjustment of the maximum width of the bubble size (change to 26.5 rem). Under this size, the incoming message bubble just needs to shift to the right when the user enters deleted mode. So we'll have 4 rem space for the checkbox. The bubble stays in the same size without affecting the message layout. 

* Message in Sending Mode
- MMS subject text will gray out as well (#bfbfbf). 
- Multimedia content gray out as opacity 50%
( You can also refer to the layout spec )

* Fallback plan: take off the box-shadow for all the message bubbles. 

Let me know if I missed anything, Thanks!
Attachment #8464607 - Attachment is obsolete: true
Flags: needinfo?(fshih)
Assignee: azasypkin → nobody
blocking-b2g: --- → backlog
feature-b2g: --- → 2.1
Assignee: nobody → azasypkin
Status: NEW → ASSIGNED
Hey Fang,

Thanks for your answers, I have second round of questions for you :)

* Previously date _sticky_ header (TODAY, YESTERDAY, ...) had the same color as background + 0.85 opacity, so it looked nicer when it overlapped with thread messages eg. while scrolling. Should we keep the same opacity (with new color of course) for the date header?

* I see that date _sticky_ header now has left padding/margin (earlier it was aligned with carier header), but spec doesn't define exact value for it. Could you please let me know this padding/margin value?

* Previously message timestamps (or "Time text" from the spec) used main text color, but with 0.7 opacity, so it was different from the main text color, from the new spec we use different colors for outgoing and failed messages, so it looks nice without opacity too, but for incoming messages new spec uses the same color for main text and timestamp (#fff). Is that what we want or there is a typo in the spec?

* Page #1 of the spec defines "#39c3c2" for the incoming message bubble background, but at the Page #2 it's already "#39c3c1" :) What color is the right one?

* At Page #2 of the spec there is a left "1.5rem" padding defined for the back button, but it's controlled by the common BB styles that are shared across other apps. So we'll probably won't touch back button styles is in this patch.

* Since we removed progress spinner in favour of just text indicator (i.e. "Sending..." that I'm quite happy about :)) we probably need to refine "not-downloaded" message too. It had three different states:
** Initial state - "Download" button\link is displayed inside message, text\background colors are the same as for ordinary incoming message;
** Pending state - when user taps on "Donwload" button\link, it transitions to "Downloading..." text and progress spinner is displayed; (probably we can move Download\Downloading... to the same area where "Sending..." is displayed)
** Error state - text\background colors are the same as for any other failed message, but "Download" button\link is displayed, so that user can retry to download message.

Thanks!
Flags: needinfo?(fshih)
Ah, and one more question:

* I've noticed that attachment placeholder changed size from 120x80 to 150x85. We use these numbers while generating real thumbnails, so just wanted to confirm that the size change was intentional. If so, I'd rather handle it in the follow-up.
Flags: needinfo?(fshih)
Hey Fang, did you have a chance to look into the questions in comment 8 and comment 9?

Thanks!
Flags: needinfo?(fshih)
Hey Oleg, 
Sorry for the late reply, and thank you for reminding me again : ) 

> 
> Thanks for your answers, I have second round of questions for you :)
> 
> * Previously date _sticky_ header (TODAY, YESTERDAY, ...) had the same color
> as background + 0.85 opacity, so it looked nicer when it overlapped with
> thread messages eg. while scrolling. Should we keep the same opacity (with
> new color of course) for the date header?

Yes, That would be great!

> * I see that date _sticky_ header now has left padding/margin (earlier it
> was aligned with carier header), but spec doesn't define exact value for it.
> Could you please let me know this padding/margin value?

The margin is 1.5 rem, align with where the message text starts. 


> * Previously message timestamps (or "Time text" from the spec) used main
> text color, but with 0.7 opacity, so it was different from the main text
> color, from the new spec we use different colors for outgoing and failed
> messages, so it looks nice without opacity too, but for incoming messages
> new spec uses the same color for main text and timestamp (#fff). Is that
> what we want or there is a typo in the spec?

Oh! I think this one is my bad. I attempted to use it #fff with opacity 0.7. ( which is #d1eeee without opacity.) Maybe lets keep it the same like others, without the opacity and make the color # d1eeee. Would that be easier for you? 

> * Page #1 of the spec defines "#39c3c2" for the incoming message bubble
> background, but at the Page #2 it's already "#39c3c1" :) What color is the
> right one?

#39c3c2 is the correct color for it. 

> * At Page #2 of the spec there is a left "1.5rem" padding defined for the
> back button, but it's controlled by the common BB styles that are shared
> across other apps. So we'll probably won't touch back button styles is in
> this patch.

Yes, don't touch the back button. My 1.5rem padding defined is for message bubble and carrier text. Sorry for the comfusing. Yeah, so don't touch anything that's belone to common BB. : )

> * Since we removed progress spinner in favour of just text indicator (i.e.
> "Sending..." that I'm quite happy about :)) we probably need to refine
> "not-downloaded" message too. It had three different states:
> ** Initial state - "Download" button\link is displayed inside message,
> text\background colors are the same as for ordinary incoming message;
> ** Pending state - when user taps on "Donwload" button\link, it transitions
> to "Downloading..." text and progress spinner is displayed; (probably we can
> move Download\Downloading... to the same area where "Sending..." is
> displayed)
> ** Error state - text\background colors are the same as for any other failed
> message, but "Download" button\link is displayed, so that user can retry to
> download message.
For this part. I'll provide a mock up for you! Let me work on it, I'll attach the spec asap.
Flags: needinfo?(fshih)
(In reply to Oleg Zasypkin [:azasypkin] from comment #9)
> Ah, and one more question:
> 
> * I've noticed that attachment placeholder changed size from 120x80 to
> 150x85. We use these numbers while generating real thumbnails, so just
> wanted to confirm that the size change was intentional. If so, I'd rather
> handle it in the follow-up.

For this one! Recently Jenny and I were thinking to change all the media thumbnail to square, you can refer to bug 983172. And for that change, I'll provide a new spec for it, since some alignment and thumbnail assets are going to change. Thanks!
Depends on: 1054989
Hi Oleg,
Just attached the new attachment placeholder visual spec. Placeholder changed size from 120x80 to 100x100 (10remx10rem). Please refer to the attached file for new assets and padding/margin. Thanks!
Whiteboard: [p=2]
Target Milestone: --- → 2.1 S3 (29aug)
(In reply to Fang Shih from comment #13)
> Created attachment 8475045 [details]
> 2.1_MessagingThread_VisualSpec_attachment_0819.zip
> 
> Hi Oleg,
> Just attached the new attachment placeholder visual spec. Placeholder
> changed size from 120x80 to 100x100 (10remx10rem). Please refer to the
> attached file for new assets and padding/margin. Thanks!

Thanks! As we agreed on planning today, we'll cover attachment VR in the scope of bug 983172.
QA Whiteboard: [COM=Gaia::SMS]
Hey Oleg, 
Attached the download pending status spec. They are pretty much the same, but with different text color. You can refer to the mock up, I've applied those statuses into our new design. Thanks! : )
(In reply to Fang Shih from comment #15)
> Created attachment 8476562 [details]
> 2.1_Messaging_Download_Pending.zip
> 
> Hey Oleg, 
> Attached the download pending status spec. They are pretty much the same,
> but with different text color. You can refer to the mock up, I've applied
> those statuses into our new design. Thanks! : )

Thanks!
No longer blocks: sms-refresh-v2.1
Sorry, mistake, still block visual refresh 2.1
Depends on: sms-refresh-v2.1
No longer depends on: sms-refresh-v2.1
No longer depends on: 1054989
Depends on: 1059167
Comment on attachment 8479407 [details] [review]
GitHub pull request URL

Hey Steve,

While we're waiting for bug 1059167 could you please give your early feedback on this?

Thanks!
Attachment #8479407 - Flags: feedback?(schung)
Comment on attachment 8479407 [details] [review]
GitHub pull request URL

Some suggestions on github, but no big issue or regression there, awesome! :)
Attachment #8479407 - Flags: feedback?(schung) → feedback+
Comment on attachment 8479407 [details] [review]
GitHub pull request URL

Thanks for feedback! I've fixed comments and dependency has been already resolved, so asking for review :)
Attachment #8479407 - Flags: review?(schung)
Comment on attachment 8479407 [details] [review]
GitHub pull request URL

Looks good to me, now we have whole new thread view :)
Attachment #8479407 - Flags: review?(schung) → review+
(In reply to Steve Chung [:steveck] from comment #22)
> Comment on attachment 8479407 [details] [review]
> GitHub pull request URL
> 
> Looks good to me, now we have whole new thread view :)

Thanks for review!
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/gaia/commit/c1a089f2a059004c218b6783b166bddd8554ab1c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
New design is really nice :)
Thanks oleg for landing this new design
(In reply to kumar rishav (:rishav_) from comment #25)
> New design is really nice :)
> Thanks oleg for landing this new design

Thanks! A huge kudos to Jenny, Fang and Steve!
Verified User story is fixed. UI modified according to spec.

Gaia      2be78d83a760fa3b9638fe51c266b442d14597f1
Gecko     https://hg.mozilla.org/mozilla-central/rev/1db35d2c9a2f
BuildID   20140831160203
Version   34.0a1
ro.build.version.incremental=110
ro.build.date=Fri Jun 27 15:57:58 CST 2014
B1TC00011230
Status: RESOLVED → VERIFIED
Blocks: 1061150
Depends on: 1063970
Tested and new bug:
Flame
2.1
Gecko-039bd5d
Gaia-95e9b09

https://bugzilla.mozilla.org/show_bug.cgi?id=1064186
(In reply to Loli (:lolimartinezcr) from comment #28)
> Tested and new bug:
> Flame
> 2.1
> Gecko-039bd5d
> Gaia-95e9b09
> 
> https://bugzilla.mozilla.org/show_bug.cgi?id=1064186


https://bugzilla.mozilla.org/show_bug.cgi?id=1064181
Duplicate of this bug: 999226
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.