Bug 840057 - [MMS][User Story] Video playback from message #9523
Conversation
container.appendChild(wrapper); | ||
|
||
attachmentMap.set(overlayElement, attachment); | ||
attachmentMap.set(wrapper, attachment); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be refactored into a reusable component, or template?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it can be, but I really want to wait for the Attachment stuff for composer to land, and then there will likely be a better place to share this logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, that makes good sense to me. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically, Right now the development of compose and view receive have been happening separately, I'd like to make them both use the Attachment stuff @danheberden started and @incompl is finishing where we can. I'll probably take inventory of all of that later next week assuming there aren't a ton of bugs still open for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not so sure this should be reused yet; this is simple enough, and this will likely be called several hundred times, so we can try to stay low-level
After talking with UX team and taking into account the restrictions related with video decoding, we are going to move forward with an icon for 'audio' & 'video' attachments, instead of the wrapper + overlay. @VictoriaGerchinhoren We need your help here! ;) |
dataArray.forEach(function(attachment) { | ||
var mediaElement, textElement, url; | ||
var mediaElement, textElement, overlayElement, url; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to remove the overlayElement
right?
Bug 840057 - [MMS][User Story] Video playback from message
Bug 840057 - [MMS][User Story] Video playback from message(cherry picked from commit 3dad665)
https://bugzilla.mozilla.org/show_bug.cgi?id=840057