[MMS] Display the subject of an MMS, if present

RESOLVED FIXED in 1.3 Sprint 5 - 11/22

Status

Firefox OS
Gaia::SMS
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: julienw, Assigned: evhan55)

Tracking

unspecified
1.3 Sprint 5 - 11/22
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:1.3+)

Details

(Whiteboard: MMS_TEF, [u=commsapps-user c=messaging p=0] )

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
We have to show the subject in the MMS App when receiving a MMS.
(Reporter)

Updated

4 years ago
See Also: → bug 885680
(Reporter)

Comment 1

4 years ago
NI Ayman for the Wireframes.
Flags: needinfo?(aymanmaat)
(Assignee)

Updated

4 years ago
Assignee: nobody → evelyn
(Assignee)

Comment 2

4 years ago
(In reply to Julien Wajsberg [:julienw] (in Paris-Web conference thursday and friday) from comment #0)
> We have to show the subject in the MMS App when receiving a MMS.

Just to clarify, this bug is also supposed to handle displaying subjects for MMS's that are being sent, as well as received, right?

I am not talking about composing subjects, but I am talking about displaying subjects on MMS's I have composed before.

If so, I'd like to change the title of this bug.
(In reply to Evelyn Eastmond [:evhan55] from comment #2)
> (In reply to Julien Wajsberg [:julienw] (in Paris-Web conference thursday
> and friday) from comment #0)
> > We have to show the subject in the MMS App when receiving a MMS.
> 
> Just to clarify, this bug is also supposed to handle displaying subjects for
> MMS's that are being sent, as well as received, right?

Yes, per these "wireframes" https://bugzilla.mozilla.org/show_bug.cgi?id=919966#c5

Any message should display a subject if the subject property of a message object has a value that is not an empty string, null or undefined
(Assignee)

Comment 4

4 years ago
(In reply to Rick Waldron from comment #3)
> (In reply to Evelyn Eastmond [:evhan55] from comment #2)
> > (In reply to Julien Wajsberg [:julienw] (in Paris-Web conference thursday
> > and friday) from comment #0)
> > > We have to show the subject in the MMS App when receiving a MMS.
> > 
> > Just to clarify, this bug is also supposed to handle displaying subjects for
> > MMS's that are being sent, as well as received, right?
> 
> Yes, per these "wireframes"
> https://bugzilla.mozilla.org/show_bug.cgi?id=919966#c5
> 
> Any message should display a subject if the subject property of a message
> object has a value that is not an empty string, null or undefined

Actually, for MMS's from my Samsung Galaxy S4 that I have been testing with, MMS's with no subject get a subject of 'no subject'.  So I should test for that too?  And who knows what other carriers do :(

Ok, I will change the name of this bug to accurately reflect that it's for the rendering of both sent and received MMS's.
(Assignee)

Updated

4 years ago
Summary: [MMS] Display the subject of a received MMS, if present → [MMS] Display the subject of an MMS, if present
(In reply to Evelyn Eastmond [:evhan55] from comment #4)
> Actually, for MMS's from my Samsung Galaxy S4 that I have been testing with,
> MMS's with no subject get a subject of 'no subject'.  So I should test for
> that too?  

HA! Yes, absolutely.

Comment 6

4 years ago
(In reply to Evelyn Eastmond [:evhan55] from comment #4)
> (In reply to Rick Waldron from comment #3)
> > (In reply to Evelyn Eastmond [:evhan55] from comment #2)
> > > (In reply to Julien Wajsberg [:julienw] (in Paris-Web conference thursday
> > > and friday) from comment #0)
> > > > We have to show the subject in the MMS App when receiving a MMS.
> > > 
> > > Just to clarify, this bug is also supposed to handle displaying subjects for
> > > MMS's that are being sent, as well as received, right?
> > 
> > Yes, per these "wireframes"
> > https://bugzilla.mozilla.org/show_bug.cgi?id=919966#c5
> > 
> > Any message should display a subject if the subject property of a message
> > object has a value that is not an empty string, null or undefined
> 
> Actually, for MMS's from my Samsung Galaxy S4 that I have been testing with,
> MMS's with no subject get a subject of 'no subject'.  So I should test for
> that too?  And who knows what other carriers do :(
> 
> Ok, I will change the name of this bug to accurately reflect that it's for
> the rendering of both sent and received MMS's.

Hi Evelyn, Rick

When an MMS is received with no subject can you ensure that the text 'no subject' is not output into the message thread. I have been quit deliberate in my specification of this because real estate is at a premium on our devices therefore any unnecessary content on the screen should be avoided in order to maximise the space for the information contained in the message thread (the messages themselves and subjects where a subject is added). 'no message' is considered data rather than information.

feel free to ni? me if you wish to discuss futher
Flags: needinfo?(aymanmaat)
(Assignee)

Comment 7

4 years ago
(In reply to ayman maat :maat from comment #6)
> (In reply to Evelyn Eastmond [:evhan55] from comment #4)
> > (In reply to Rick Waldron from comment #3)
> > > (In reply to Evelyn Eastmond [:evhan55] from comment #2)
> > > > (In reply to Julien Wajsberg [:julienw] (in Paris-Web conference thursday
> > > > and friday) from comment #0)
> > > > > We have to show the subject in the MMS App when receiving a MMS.
> > > > 
> > > > Just to clarify, this bug is also supposed to handle displaying subjects for
> > > > MMS's that are being sent, as well as received, right?
> > > 
> > > Yes, per these "wireframes"
> > > https://bugzilla.mozilla.org/show_bug.cgi?id=919966#c5
> > > 
> > > Any message should display a subject if the subject property of a message
> > > object has a value that is not an empty string, null or undefined
> > 
> > Actually, for MMS's from my Samsung Galaxy S4 that I have been testing with,
> > MMS's with no subject get a subject of 'no subject'.  So I should test for
> > that too?  And who knows what other carriers do :(
> > 
> > Ok, I will change the name of this bug to accurately reflect that it's for
> > the rendering of both sent and received MMS's.
> 
> Hi Evelyn, Rick
> 
> When an MMS is received with no subject can you ensure that the text 'no
> subject' is not output into the message thread. I have been quit deliberate
> in my specification of this because real estate is at a premium on our
> devices therefore any unnecessary content on the screen should be avoided in
> order to maximise the space for the information contained in the message
> thread (the messages themselves and subjects where a subject is added). 'no
> message' is considered data rather than information.
> 
> feel free to ni? me if you wish to discuss futher

Hi Ayman,

Yes, we will filter any meta-data/information out and only display subjects and messages that have content.  I think we are in agreement.  Sorry for the misunderstanding.

Do we know what kinds of meta-data carriers are likely to send?  'no subject' was something I noticed on a test with a specific kind of device on a specific carrier.  I am not sure if other devices/carriers would send other meta-data kind of subjects.

Thanks,
Evelyn
(Assignee)

Comment 8

4 years ago
Created attachment 815960 [details]
https://github.com/evhan55/gaia/compare/920164

Hi Rick,

My first go at this patch is almost there.
It is missing two features:

1) Proper filtering of (localized) subjects that we don't wan to display
2) Concatenation of subjects that are too long to fit on one line (this might be an NI from the UX/UI peeps)

For 1), I'd like to get your advice on how to do it better than my initial try:
https://github.com/evhan55/gaia/compare/920164#diff-74841e428dea67f7357f1893a4d26155R1203
https://github.com/evhan55/gaia/compare/920164#diff-74841e428dea67f7357f1893a4d26155R1224

Any advice appreciated, thanks!
Attachment #815960 - Flags: feedback?(waldron.rick)
(Assignee)

Comment 9

4 years ago
(In reply to Evelyn Eastmond [:evhan55] from comment #8)
> Created attachment 815960 [details]
> https://github.com/evhan55/gaia/compare/920164
> 
> Hi Rick,
> 
> My first go at this patch is almost there.
> It is missing two features:
> 
> 1) Proper filtering of (localized) subjects that we don't wan to display
> 2) Concatenation of subjects that are too long to fit on one line (this
> might be an NI from the UX/UI peeps)
> 
> For 1), I'd like to get your advice on how to do it better than my initial
> try:
> https://github.com/evhan55/gaia/compare/920164#diff-
> 74841e428dea67f7357f1893a4d26155R1203
> https://github.com/evhan55/gaia/compare/920164#diff-
> 74841e428dea67f7357f1893a4d26155R1224
> 
> Any advice appreciated, thanks!

By 'concatenation' I meant 'truncation' or shortening with trailing ellipses, my bad.

Comment 10

4 years ago
> 
> Do we know what kinds of meta-data carriers are likely to send?  'no
> subject' was something I noticed on a test with a specific kind of device on
> a specific carrier.  I am not sure if other devices/carriers would send
> other meta-data kind of subjects.
> 

I dont, but i can find out.
ni? to me to track down this information
Flags: needinfo?(aymanmaat)
(Assignee)

Comment 11

4 years ago
Hi Ayman,

We need more information regarding MMS subject text that can possibly come from carriers/devices that we don't want to display, but aren't necessarily empty.

For example, for a Samsung Galaxy S4 on Sprint in the US, sending an MMS with no subject, sends a subject line of 'no subject'.

Thank you!
Evelyn

Comment 12

4 years ago
> 2) Concatenation of subjects that are too long to fit on one line (this
> might be an NI from the UX/UI peeps)

I will look at your patch tomorrow (its friday night here in Berlin, and can hear the clatter of beer bottles calling my name) 
...could I ask in what instance you are truncating subject? We should not do this in the message thread itself. We should display the full subject.
Flags: needinfo?(evelyn)
(Assignee)

Comment 13

4 years ago
> ...could I ask in what instance you are truncating subject? We should not do
> this in the message thread itself. We should display the full subject.

Yep ok! Is there a mockup with a multi-line subject?  Some of them can be too long to fit in one line.  All the subjects in the mockups here: https://bugzilla.mozilla.org/show_bug.cgi?id=919966#c5 are single-line subjects.
Flags: needinfo?(evelyn)
Created attachment 815987 [details]
920164.png

(In reply to Evelyn Eastmond [:evhan55] from comment #8)
> Created attachment 815960 [details]
> https://github.com/evhan55/gaia/compare/920164
> 
> Hi Rick,
> 
> My first go at this patch is almost there.
> It is missing two features:
> 
> 1) Proper filtering of (localized) subjects that we don't wan to display

I don't know what the plan is.

> 2) Concatenation of subjects that are too long to fit on one line (this
> might be an NI from the UX/UI peeps)

This should work:

  white-space: nowrap;
  overflow: hidden;
  text-overflow: ellipsis;



@ayman, I've attached a screenshot of progress so far
(In reply to Evelyn Eastmond [:evhan55] from comment #13)
> > ...could I ask in what instance you are truncating subject? We should not do
> > this in the message thread itself. We should display the full subject.
> 
> Yep ok! Is there a mockup with a multi-line subject?  Some of them can be
> too long to fit in one line.  All the subjects in the mockups here:
> https://bugzilla.mozilla.org/show_bug.cgi?id=919966#c5 are single-line
> subjects.

Evelyn, Ayman—what about "tap to show" and "tap to hide" the remainder of the subject?
(Assignee)

Comment 16

4 years ago
> Evelyn, Ayman—what about "tap to show" and "tap to hide" the remainder of
> the subject?

This sounds ok to me, I'm wondering what the design would be to show users they can do that without taking up too much space.  Ellipses to cut off the message and then a disclosure triangle?  Only ellipses? A button?

I'm ok with just showing the entire subject too, although that makes most sense only if subjects have a character limit.

Comment 17

4 years ago
(In reply to Evelyn Eastmond [:evhan55] from comment #16)
> > Evelyn, Ayman—what about "tap to show" and "tap to hide" the remainder of
> > the subject?
> 
> This sounds ok to me, I'm wondering what the design would be to show users
> they can do that without taking up too much space.  Ellipses to cut off the
> message and then a disclosure triangle?  Only ellipses? A button?
> 
> I'm ok with just showing the entire subject too, although that makes most
> sense only if subjects have a character limit.

Character limit of subject is 40 characters i believe. I would prefer to show the whole subject and not have a tap to show/hide feature.
(Assignee)

Comment 18

4 years ago
(In reply to ayman maat :maat from comment #17)
> Character limit of subject is 40 characters i believe. I would prefer to
> show the whole subject and not have a tap to show/hide feature.

Okay, and what if the 40-character subject is too wide to fit on one line?
(Reporter)

Comment 19

4 years ago
Can we check if the 'no subject' string is something spec-ed ? I'm especially worried that Android send other strings when using other locales, eg french...
(Assignee)

Comment 20

4 years ago
(In reply to Julien Wajsberg [:julienw] from comment #19)
> Can we check if the 'no subject' string is something spec-ed ? I'm
> especially worried that Android send other strings when using other locales,
> eg french...

I think Ayman has been NI'd on this issue, to research this and see what strings we will need to look out for.

Comment 21

4 years ago
(In reply to Evelyn Eastmond [:evhan55] from comment #20)
> (In reply to Julien Wajsberg [:julienw] from comment #19)
> > Can we check if the 'no subject' string is something spec-ed ? I'm
> > especially worried that Android send other strings when using other locales,
> > eg french...
> 
> I think Ayman has been NI'd on this issue, to research this and see what
> strings we will need to look out for.

Yep, i am making enquiries, but in the meantime if anyone else has any example strings please feel free to post.

leaving ni? for me

Comment 22

4 years ago
(In reply to Evelyn Eastmond [:evhan55] from comment #18)
> (In reply to ayman maat :maat from comment #17)
> > Character limit of subject is 40 characters i believe. I would prefer to
> > show the whole subject and not have a tap to show/hide feature.
> 
> Okay, and what if the 40-character subject is too wide to fit on one line?

referencing: FFOS_MessageSubject_V1.3_20121009_V4.0

**for message thread**
Page 6
frame 2 'Message thread created with subject in the message'
annotation 01: "Whatever was written in the subject field in screen one is output here. If the text length is more than one line the subject text wraps into two lines. It is not truncated."

**for subject field**
Page 9
frame 'Content in subject field'
"If the content that is added to the subject field is more than one line extendthe subject field onto two lines and push the message field downwards."
Flags: needinfo?(aymanmaat)
(Assignee)

Comment 23

4 years ago
(In reply to ayman maat :maat from comment #22)

> referencing: FFOS_MessageSubject_V1.3_20121009_V4.0

Thank you, Ayman.  Is this a document I can access somewhere?  I don't see it on the Mozilla UX box.com account.
(Reporter)

Comment 24

4 years ago
It's in bug 885680. (I think it should be in at least the feature bug...)
(Assignee)

Comment 25

4 years ago
(In reply to Julien Wajsberg [:julienw] from comment #24)
> It's in bug 885680. (I think it should be in at least the feature bug...)

Thank you, I should have looked there first for all these questions I have asked.  I see it now.

Comment 26

4 years ago
(In reply to Evelyn Eastmond [:evhan55] from comment #25)
> (In reply to Julien Wajsberg [:julienw] from comment #24)
> > It's in bug 885680. (I think it should be in at least the feature bug...)
> 
> Thank you, I should have looked there first for all these questions I have
> asked.  I see it now.

your quite right though, its not on box. I thought i had uploaded it there! Sorry! 
anyway. i have uploaded it to box here: 

Firefox UX OS > 1.3 Specs > Messages app
(Assignee)

Comment 27

4 years ago
Created attachment 816700 [details]
920164-long_subject.png

What should the line-height be for long subjects that go on multiple lines?  See attached image.  Might take a while to get this CSS right for both single-line and multi-line subjects
Flags: needinfo?(aymanmaat)
(Assignee)

Comment 28

4 years ago
Created attachment 816712 [details]
920164_long-subject-2.png

Trying styles for long subject again, please review this screenshot instead.
Attachment #816700 - Attachment is obsolete: true

Comment 29

4 years ago
(In reply to Evelyn Eastmond [:evhan55] from comment #27)
> Created attachment 816700 [details]
> 920164-long_subject.png
> 
> What should the line-height be for long subjects that go on multiple lines? 
> See attached image.  Might take a while to get this CSS right for both
> single-line and multi-line subjects

This is more a question for the visual designer so i am going to ni? Victoria to advise you. Victoria, could you also give Evelyn feedback on Comment 28 as well as comment 27 please. Thanks.
Flags: needinfo?(aymanmaat) → needinfo?(vpg)
PLease refer to this guide for the line height: http://www.mozilla.org/en-US/styleguide/products/firefox-os/typeface/

Also, please do not forget the margin at the end of the bubble should be 1.5 rem so the text should not extend that far.

Thanks
Flags: needinfo?(vpg)
PLease refer to this guide for the line height: http://www.mozilla.org/en-US/styleguide/products/firefox-os/typeface/

Also, please do not forget the margin at the end of the bubble should be 1.5 rem so the text should not extend that far.

Thanks
(Assignee)

Comment 32

4 years ago
(In reply to Victoria Gerchinhoren from comment #31)
> PLease refer to this guide for the line height:
> http://www.mozilla.org/en-US/styleguide/products/firefox-os/typeface/

Thank you.  I don't see any specs there regarding line-height, unless I am missing something?  I tried inspecting the paragraphs of Fira Sans text, but they are actually images and not pure HTML.

> Also, please do not forget the margin at the end of the bubble should be 1.5
> rem so the text should not extend that far.

Thanks, I didn't realize it needed to be on both sides.  I will flag you when there is a new implementation to review.
(Assignee)

Comment 33

4 years ago
> Also, please do not forget the margin at the end of the bubble should be 1.5
> rem so the text should not extend that far.

Looking at this mockup https://bug919966.bugzilla.mozilla.org/attachment.cgi?id=814143

I don't see the 1.5rem margin on the right hand-side.  Do you know what size the right-side margin (on incoming messages) should be?
Flags: needinfo?(vpg)
(Assignee)

Comment 34

4 years ago
(In reply to Evelyn Eastmond [:evhan55] from comment #33)
> > Also, please do not forget the margin at the end of the bubble should be 1.5
> > rem so the text should not extend that far.
> 
> Looking at this mockup
> https://bug919966.bugzilla.mozilla.org/attachment.cgi?id=814143
> 
> I don't see the 1.5rem margin on the right hand-side.  Do you know what size
> the right-side margin (on incoming messages) should be?

Hi Victoria, I see now, it's 1.5rem on the right and 4.0rem on the left.  I'll try to cancel the NI.
(Assignee)

Updated

4 years ago
Flags: needinfo?(vpg)
(Assignee)

Updated

4 years ago
Flags: needinfo?(aymanmaat)
Thanks for that Evelyn ;)
(Reporter)

Comment 36

4 years ago
Evelyn, are you still expecting something from Ayman here ?
(Assignee)

Comment 37

4 years ago
(In reply to Julien Wajsberg [:julienw] from comment #36)
> Evelyn, are you still expecting something from Ayman here ?

Yes, Ayman was going to research possible empty subject strings that we need to filter out and not display.  For example, some carriers send along a 'no subject' message.  It would be good to get more insight on that.

Comment 38

4 years ago
(In reply to Evelyn Eastmond [:evhan55] from comment #37)
> (In reply to Julien Wajsberg [:julienw] from comment #36)
> > Evelyn, are you still expecting something from Ayman here ?
> 
> Yes, Ayman was going to research possible empty subject strings that we need
> to filter out and not display.  For example, some carriers send along a 'no
> subject' message.  It would be good to get more insight on that.

Sorry I am late replying. I am currently lead to believe that the presence of 'no subject' is actually device dependent not carrier dependant, so there is no need to filter.

ni? me if you require more input
Flags: needinfo?(aymanmaat)
(Assignee)

Comment 39

4 years ago
(In reply to ayman maat :maat from comment #38)

> Sorry I am late replying. I am currently lead to believe that the presence
> of 'no subject' is actually device dependent not carrier dependant, so there
> is no need to filter.

Just to clarify, you are ok with 'no subject' being displayed as the subject?
Flags: needinfo?(aymanmaat)

Comment 40

4 years ago
(In reply to Evelyn Eastmond [:evhan55] from comment #39)
> (In reply to ayman maat :maat from comment #38)
> 
> > Sorry I am late replying. I am currently lead to believe that the presence
> > of 'no subject' is actually device dependent not carrier dependant, so there
> > is no need to filter.
> 
> Just to clarify, you are ok with 'no subject' being displayed as the subject?

Hi Evelyn no, please do not display anything when there is no subject accompanying a message. 

what i was meaning when i said 'there is no need to filter' is that there are no strings to filter. The appearance of the string 'no subject' is down to the device, not the carrier, I am lead to believe. Therefore there should be no variation of strings sent to indicate that the message has no subject.
Flags: needinfo?(aymanmaat)
(Reporter)

Comment 41

4 years ago
That means that if (hypothetically) a third-party device was sending the "no subject" string as a subject, we would display it. Am I understanding rightly?
Flags: needinfo?(aymanmaat)

Comment 42

4 years ago
(In reply to Julien Wajsberg [:julienw] from comment #41)
> That means that if (hypothetically) a third-party device was sending the "no
> subject" string as a subject, we would display it. Am I understanding
> rightly?

I would say so as we as the receiving device would not be able to distinguish between "no subject" being a string that the sending 'system' has added and a string the sender has added.
Flags: needinfo?(aymanmaat)
(Reporter)

Comment 43

4 years ago
Great, thanks, all's clear now ;)
blocking a committed 1.3 user story. 1.3+
blocking-b2g: 1.3? → 1.3+
(Assignee)

Comment 45

4 years ago
Ok, thanks for the clarification!
Target Milestone: --- → 1.3 Sprint 4 - 11/8
Target Milestone: 1.3 Sprint 4 - 11/8 → 1.3 Sprint 5 - 11/22
(Assignee)

Comment 46

4 years ago
Comment on attachment 815960 [details]
https://github.com/evhan55/gaia/compare/920164

Submitting for feedback.

Questions:
1) I have filtered out 'system subjects' on MMS's that have subjects applied by the system.  Not sure if my list is exhaustive?
2) What kind of test would suffice for this patch?
Attachment #815960 - Flags: feedback?(felash)
Attachment #815960 - Flags: feedback?(aymanmaat)
(Reporter)

Comment 47

4 years ago
Evelyn, I don't exactly get what are these "system subjects", could you please explain?  Thanks!

Unit tests are probably enough :)
(Assignee)

Comment 48

4 years ago
(In reply to Julien Wajsberg [:julienw] from comment #47)
> Evelyn, I don't exactly get what are these "system subjects", could you
> please explain?  Thanks!

I had noticed them in the MMS's with warnings in the Tom OHare SMS desktop mock thread.
Looking through the code-base now, I'm not sure where those subjects are coming from. They are just being mocked and not actually generated by the system like I mistakenly believed.  I will remove them.  When you test the code, you will see what I mean in the Tom OHare test thread.

> Unit tests are probably enough :)

Ok

I'll remove the filtering and add unit tests and resubmit for review.
@julienw, for context, the "Tom O'Hare" thread has messages whose sender value is "123456". I think those subjects might be made up?
(Reporter)

Comment 50

4 years ago
Yep, that's what I was gonna say: you can change the subjects in the desktop mocks to put more sensible ones if you want :)

Comment 51

4 years ago
Comment on attachment 815960 [details]
https://github.com/evhan55/gaia/compare/920164

removing flag to me as i think Julien has the questions asked on this covered. 
flag me again if there is something more you need from me.
Attachment #815960 - Flags: feedback?(aymanmaat) → feedback-
(Reporter)

Updated

4 years ago
Attachment #815960 - Flags: feedback-
(Reporter)

Comment 52

4 years ago
Comment on attachment 815960 [details]
https://github.com/evhan55/gaia/compare/920164

Does not look bad :)

I wonder if we really need these `.article-list[data-type="list"]` prefixes in the CSS...

Long-term, we should move towards more componentized CSS, eg a message-bubble.css that would hold the css for a message bubble, so better start coding a more OO css already now, and I'm sure you know more than me about this :)
Attachment #815960 - Flags: feedback?(felash) → feedback+
(Assignee)

Comment 53

4 years ago
(In reply to Julien Wajsberg [:julienw] from comment #52)
> Comment on attachment 815960 [details]
> https://github.com/evhan55/gaia/compare/920164
> 
> Does not look bad :)
> 
> I wonder if we really need these `.article-list[data-type="list"]` prefixes
> in the CSS...
> 
> Long-term, we should move towards more componentized CSS, eg a
> message-bubble.css that would hold the css for a message bubble, so better
> start coding a more OO css already now, and I'm sure you know more than me
> about this :)

It looks like a lot of the CSS in style_unstable changed recently so I'm not sure how to think about it app-wide or system-wide yet
(Assignee)

Comment 54

4 years ago
Created attachment 833311 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/13765

- `thread_ui.js`
    - added subject CSS class if message type is mms
    - added subject to the message DOM template if mms and has subject
- `index.html`
    - added subject div and template item
- `sms.css`
    - styling for subject div
- `thread_ui_test.js`
    - added unit test to check against template interpolation with subject field for mms
Attachment #833311 - Flags: review?(felash)
(Assignee)

Comment 55

4 years ago
Comment on attachment 833311 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/13765

Resubmitting for review based on Julien's feedback
(Assignee)

Comment 56

4 years ago
Comment on attachment 833311 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/13765

Resubmitting for review based on Julien's feedback.

Julien, I could not test this on the device because of the introduction of a new bug, which I had to file:

https://bugzilla.mozilla.org/show_bug.cgi?id=940576

MMS's won't display until that bug is fixed. Should I take that bug?
(Reporter)

Updated

4 years ago
Blocks: 941030
(Reporter)

Comment 57

4 years ago
Comment on attachment 833311 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/13765

r=me with the latest comments implemented
Please ask me a review again if you don't implement them all :)

Of course please have a green travis before merging!
Thanks :)
Attachment #833311 - Flags: review?(felash) → review+
(Assignee)

Comment 58

4 years ago
(In reply to Julien Wajsberg [:julienw] from comment #57)
> Comment on attachment 833311 [details] [review]
> https://github.com/mozilla-b2g/gaia/pull/13765
> 
> r=me with the latest comments implemented
> Please ask me a review again if you don't implement them all :)
> 
> Of course please have a green travis before merging!
> Thanks :)

Should Rick do the merge if/when I implement all your requested changes?
(Reporter)

Comment 59

4 years ago
just add the keyword "checkin-needed" in the bug, and somebody will eventually merge. (this could be Rick or me of course, if we happen to be available)
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/gaia/commit/e7f4ac493dbf7f3c92553d78b6a5c11215b16bae
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.