Closed Bug 862311 Opened 7 years ago Closed 7 years ago

[MMS][User Story] Display attachment icon in thread

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:leo+, b2g18 fixed)

RESOLVED FIXED
1.1 CS (11may)
blocking-b2g leo+
Tracking Status
b2g18 --- fixed

People

(Reporter: airpingu, Assigned: jugglinmike)

References

()

Details

(Keywords: feature)

Attachments

(2 files, 7 obsolete files)

Per discussion with UX guys. In the current thread view, we only display the text body for an SMS. For the MMS, we need to display more MMS-specific info:

  - an attachment icon (if this MMS has some attachments)
  - the subject text next to the icon (if this MMS has a subject)
Flags: needinfo?(ffos-product)
This bugs need both Gecko and Gaia work:

Gecko: need to expose a new 'type' attribute and assign MMS' subject to the 'body' attribute.

Gaia: need to have corresponding UIs to interpret the new attributes as mentioned above.
Blocks: 862320
Is this type different from content-type of blob? Can we just use that type?
No, "type" is for "sms"/"mms".
ffos-product - please consider this alongside bug 862320
I will defer UX to Ayman/Rafael for MMS.  Does this match the specs?
Flags: needinfo?(hello)
Flags: needinfo?(ffos-product)
Flags: needinfo?(aymanmaat)
According to the last wireframes released by Ayman
HTML5_SMS-MMSUserStorySpecifications_20130404_V4.0.pdf 
included in https://www.dropbox.com/s/kfjcejxacdoyb52/HTML5_SMS-MMSUserStorySpecifications_20130404_V4.0.pdf (pag 12,Message thread listing)

If the last message exchanged (sent or recieved) has an attachment, an icon will be showed in the thread list (due to space constraints there is no need to include any text from the massage as it is the attachment that is important)

And regarding the MMS subject, will NOT be included.
removing RFI to me as answer is covered in Maria's response in #Comment 6
Flags: needinfo?(aymanmaat)
blocking-b2g: leo? → leo+
Summary: [MMS][User Story] Display attachment icon and show subject next to the icon if exist. → [MMS][User Story] Display attachment icon in thread
Flags: in-moztrap?
Assignee: nobody → mike
Hi Victoria,

I'm looking for the attachment icon (the paper clip image in the wireframes). Is it already in the Gaia repository somewhere? If not, do you know where I can find it?
Flags: needinfo?(vpg)
Attached patch WIP (obsolete) — Splinter Review
I've implemented the necessary functionality with this patch. As described in this bug's metadata, any solution depends on Bug 863241. Note also that the requisite art assets are not available.

This particular solution also re-factors Thread List HTML generation. Doing so makes the code-base more readable and more consistent, but it introduces an additional dependency on Bug 865287, which is currently under review.
Keywords: feature
OS: Mac OS X → Gonk (Firefox OS)
Priority: -- → P1
Hardware: x86 → ARM
Whiteboard: [status: needs assets, needs gecko change to land]
Duplicate of this bug: 862320
Why do we show an attachment icon as soon as the message is a MMS? Can't we have MMS that are simple text with no attachment?
The latest design documentation states that only the attachment icon should be rendered:

> 02 module indicating last message either sent or received has an attachment
> - Due to space constraints there is no need to include any text from the
> massage as it is the attachment that is important

I can't speak to the design, but I can say that there are currently technical limitations that prevent this:

When rendering the Thread List, only thread data is available. This includes some data about the latest message, namely the message body and time stamp. MMS text content is not available.

Displaying MMS text content in the Thread List would require approval from UX and non-trivial technical work in Gecko: extending the MMS API to include the complete message object of the latest message in the thread. This will in turn require SMIL parsing, which has non-negligible performance implications.
(In reply to Mounir Lamouri (:mounir) from comment #11)
> Why do we show an attachment icon as soon as the message is a MMS? Can't we
> have MMS that are simple text with no attachment?

Hi Mounir

My understanding for v1.1 was that an MMS is only an MMS if it has a file attached to it. I have pinged Daniel and Peter with a question around group messaging the answer of which should influence my response to your question. 

RFI to Daniel, Peter and Kev to confirm V1.1 requirements around sending an MMS that does *does not* have a file attached.
(In reply to Mike Pennisi [:jugglinmike] from comment #12)
> The latest design documentation states that only the attachment icon should
> be rendered:
> 
> > 02 module indicating last message either sent or received has an attachment
> > - Due to space constraints there is no need to include any text from the
> > massage as it is the attachment that is important
> 
> I can't speak to the design, but I can say that there are currently
> technical limitations that prevent this:
> 
> When rendering the Thread List, only thread data is available. This includes
> some data about the latest message, namely the message body and time stamp.
> MMS text content is not available.
> 
> Displaying MMS text content in the Thread List would require approval from
> UX and non-trivial technical work in Gecko: extending the MMS API to include
> the complete message object of the latest message in the thread. This will
> in turn require SMIL parsing, which has non-negligible performance
> implications.

Hey, maybe I am missing something in what you have written. We are not specifying the displaying of MMS text content in the Thread List as we were told be the Taipei team during the Madrid work week that this was not technically possible. Therefore we only show the paper clip icon to indicate that the message is an MMS. Are you saying that we cannot detect that a message is an MMS and therefore cannot show the paperclip icon? Please can you clarify. Thanks.
(In reply to ayman maat :maat from comment #14)
> Hey, maybe I am missing something in what you have written. We are not
> specifying the displaying of MMS text content in the Thread List as we were
> told be the Taipei team during the Madrid work week that this was not
> technically possible. Therefore we only show the paper clip icon to indicate
> that the message is an MMS.

This is correct.

> Are you saying that we cannot detect that a message is an MMS and therefore
> cannot show the paperclip icon? Please can you clarify. Thanks.

Although this is not what I was trying to say in comment 12, it is technically correct as of this moment. Work is already underway to address this issue; progress is being tracked in bug 863241 (which this bug depends on).

The work-in-progress patch I previously attached assumes that this behavior is implemented.
(In reply to ayman maat :maat from comment #13)
> RFI to Daniel, Peter and Kev to confirm V1.1 requirements around sending an
> MMS that does *does not* have a file attached.

We can totally make sure that we never send an MMS if we could send an SMS instead. I'm not sure how good/bad that would be but we indeed could (and maybe we do). However, I doubt it would be a good idea to try to save something as an SMS when it was actually an MMS.
In other words, if I received an MMS from a friend, the thread list will show an attachment icon, I will look at the MMS and just see text but no attachment. That would be a terrible user experience and will be seen as a bug. The user will likely think that there is an attachment but we do not show it.
Hi(In reply to Mike Pennisi [:jugglinmike] from comment #8)
> Hi Victoria,
> 
> I'm looking for the attachment icon (the paper clip image in the
> wireframes). Is it already in the Gaia repository somewhere? If not, do you
> know where I can find it?

Hi Mike, 
I am attaching the icon here, the assets live in this folder:
https://www.dropbox.com/sh/rdbkf2o2w2h1udu/TNnUzdT2Rq

Cheers,
V
Flags: needinfo?(vpg)
Target Milestone: --- → 1.1 CS (11may)
Attached patch Work in progress v.2 (obsolete) — Splinter Review
This updated patch incorporates the attachment icon asset provided by Victoria. Note that while it is still blocked by bug 863241, it should now be trivial to merge once that bug has landed.

Corey: I wanted to make this functionality visible when running on the desktop. Although we can just "lie" about a given thread's `lastMessageType` property, I was worried that would make the experience of demoing with the mock data too confusing (where opening a thread with the attachment icon would not actually contain an MMS). I took some time to mock an MMS message--does this seem reasonable to you?
Attachment #743847 - Attachment is obsolete: true
Flags: needinfo?(gnarf37)
Attached patch Work in progress v.3 (obsolete) — Splinter Review
I neglected to include the new art asset in my previous patch. The comments on that attachment are still relevant to this one. Sorry for the noise!
Attachment #744412 - Attachment is obsolete: true
Seems sane to me!
Flags: needinfo?(gnarf37)
No longer blocks: 862320
Attached patch Attachment icon (obsolete) — Splinter Review
This patch fully implements the feature described for this bug. The UI specification has recently changed, so this code is not ready for master. I am attaching it for tracking purposes and am not requesting a review.
Attachment #744432 - Attachment is obsolete: true
Attached image MMS banner asset (obsolete) —
Hi,

This the solution I have designed to indicate it is an MMS since an MMS can or cannot contain attachments (it can be only text). Having an attachment icon to differentiate MMS from SMS makes no sense simply because it might not contain any file inside. 

Also, the importance of indicating it is an MMS in the composing scenario and replying one also is due to the high cost of that kind of message in the main markets this OS is going to be launched. 

Thanks!
We're going to land this work with the "paperclip" attachment icon and implement the more logical "MMS banner" UI with a separate bug/patch.

Pull request on GitHub.com: https://github.com/mozilla-b2g/gaia/pull/9626
Attachment #747147 - Attachment is obsolete: true
Attachment #747149 - Attachment is obsolete: true
Attachment #747151 - Attachment is obsolete: true
Attachment #747158 - Flags: review?(fbsc)
Whiteboard: [status: needs assets, needs gecko change to land]
(In reply to Victoria Gerchinhoren from comment #25)

moved to bug 870137 as a followup.
Whiteboard: [NO_UPLIFT]
This patch version incorporates feedback I received during reviews from Corey and Borja.

Please note that, as documented in this bug's dependency list, the UI changes introduced by this patch will only be visible on the latest version of Gecko in `master` (not b28-18).
Attachment #747158 - Attachment is obsolete: true
Attachment #747158 - Flags: review?(fbsc)
Attachment #747240 - Flags: review?(fbsc)
Blocks: 868679
Attachment #747240 - Flags: review?(fbsc) → review+
https://github.com/gnarf37/gaia/commit/1b84796eabac3d20e1ccf956fddb73d8b20a9555
https://github.com/mozilla-b2g/gaia/commit/d6a0f0dc2f0db7a7fc605494f744ca372648029d

Thanks Mike for your great job!
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(hello)
Resolution: --- → FIXED
Whiteboard: [NO_UPLIFT]
Blocks: 837994
Depends on: 840055
Depends on: 859296
Depends on: 845097
No longer depends on: 845097
v1-train: 63665c6
Flags: in-moztrap? → in-moztrap+
You need to log in before you can comment on or make changes to this bug.