Closed
Bug 840069
Opened 12 years ago
Closed 12 years ago
[MMS][User Story] Message preview including size
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect, P1)
Tracking
(blocking-b2g:leo+, b2g18 fixed)
Tracking | Status | |
---|---|---|
b2g18 | --- | fixed |
People
(Reporter: pdol, Assigned: greg)
References
Details
(Keywords: feature, Whiteboard: [LOE:M])
Attachments
(6 files, 1 obsolete file)
UCID: Messages-040
User Story:
As a user, I can preview a composed MMS message and see the size of the message (in kB) prior to sending to ensure the message is properly formatted and is a known size.
Updated•12 years ago
|
Depends on: b2g-mms-dom-api
Updated•12 years ago
|
Blocks: mms-userstories
Updated•12 years ago
|
Assignee: nobody → boaz
Updated•12 years ago
|
Assignee: boaz → cassie
Whiteboard: u=user c=messaging s=v1.1-sprint-3 → [LOE:M] u=user c=messaging s=v1.1-sprint-3
I recommend that there should be a carrier configurable preference which allows for the carrier to set MMS billing either on a per-message basis or per-KB.
When the pref is set to per-KB we will give the user the ability to re-size images before attaching them to messages. We will also display along side each attachment the file size so that the user knows how much data is being used for the message.
When the pref is set to a per-message basis, we automatically re-size image to the extents of the message limits and do not display the file sizes since they are not relevant in this billing scenario to the user.
All this is outlined in the MMS UX doc.
Comment 2•12 years ago
|
||
Per partner and release-driver discussions, marking blocking- until all MMS functionality in bug 849867 is complete, allowing it all to be uplifted at once to avoid SMS bustage.
blocking-b2g: leo+ → -
Comment 3•12 years ago
|
||
Got a good start on this, after muddling around with possible tests for existing functionality (bailed on that, since it was so DOM centric) and how to best achieve the placeholder text on a div[contentEditable].
Code so far: https://github.com/danheberden/gaia/commit/c3b117c430d015d6ef7f2996e009ba0eaa54d040
a) Abstracted the message composition into its own object to only expose sensible things and not perform direct DOM manipulation by other areas of Messaging.
b) Internalized .placeholder to use the `placeholder` attr of the element for i10n
c) Updated existing code to still work with ThreadUI.compose object
Not done, though - there's some things like `updateCounter` that perform messaging logic AND UI stuff. ThreadUI should just be UI, obviously, so I'll push this into the compose object. I just wanted to get *something* pushed tonight. Todo:
a) move sendMessage to utilize the form submission only
b) clean up the `updateHeight`, `sendEnabled` etc to not be so DOM dependent for testing
c) keep sending of the message non DOM dependent
Where I'm stuck is how to manage media inside of this element. There are a few use-cases to worry about:
a) user pastes media into input
b) user removes already included media
c) order of media inserted for MMS concatenation
My idea is this: have `ThreadUI.compose.getMessage()` return an array to be consumed by `sendMessage`. The array will be something like ['someText', Attachment, 'someMoreText']. The Attachment object will be the media attached, thumbnail, size, etc to display to the user. It will also have a DOM represenation, like, <div class="attachment"></div> that will be used to display the thumbnail, kB size, etc. If this element is deleted by the user using the backspace key, the message array from getMessage() will be updated accordingly. When an attachment is placed in the text area, the DOM representation will be added.
This way, the MMS api can consume an Attachment however it wants/needs to. Feel free to PM me on IRC for discussion.
Updated•12 years ago
|
Flags: in-moztrap?
Comment 4•12 years ago
|
||
leo+ as this is a part of MMS and part of v1.1 to be included in leo+ queries. No_UPLIFT for now before the whole MMS is completed
blocking-b2g: - → leo+
Whiteboard: [LOE:M] → [LOE:M] [NO_UPLIFT]
Comment 5•12 years ago
|
||
Hi! I finally got my last remaining issues figured out, so this is ready for an intense look. Here's what it looks like on a unagi: http://danheberden.com/share/90a3.png
Everything is in ThreadUI.compose:Object, which has things like append, insert, prepend. I also made Attachment, which creates attachments.
Attachment, however, needs to be worked on a bit to support proper types - like getting the preview from a video EXIF, etc. (Though, I'd like to see a Video constructor before that happens).
One thing, though, after rebasing the hell out of this: I don't understand why every single element has an ID. It makes CSS harder with specificity weight, it makes the code bound to the DOM, and it makes for a lot more work. You can see how I handled the DOM in ThreadUI.compose and ThreadUI.compose.init. Feel free to hit me up on IRC if you'd like to discuss this with me (maybe i'm missing something or unaware of a particular problem?).
The work is all at https://github.com/danheberden/gaia/commits/840069_message_preview but i'll try to figure out how to add a .patch file.
Comment 6•12 years ago
|
||
Attachment #733182 -
Flags: review+
Updated•12 years ago
|
Comment 8•12 years ago
|
||
Hi Dan,
Could you also send a pull request to mozilla-b2g gaia? I will take a deep look later but it looks fine at first. Thanks for the great work!
Updated•12 years ago
|
Attachment #733182 -
Flags: review?(fbsc)
Updated•12 years ago
|
Attachment #733182 -
Flags: review?(felash)
Comment 9•12 years ago
|
||
Hi Dan,
For landing a patch to Gaia you have to ask Julier or me to get a review of your code (that's why I've updated the flags). On the other hand, is this implementation based on any visual design from our UX Team?
Assignee | ||
Comment 10•12 years ago
|
||
Attachment #741379 -
Flags: review?
Updated•12 years ago
|
Attachment #733182 -
Attachment is obsolete: true
Attachment #733182 -
Flags: review?(felash)
Attachment #733182 -
Flags: review?(fbsc)
Comment 11•12 years ago
|
||
Comment on attachment 741379 [details]
pull request on github
taking the review
Attachment #741379 -
Flags: review? → review?(felash)
Updated•12 years ago
|
Whiteboard: [LOE:M] [NO_UPLIFT] → [LOE:M]
Updated•12 years ago
|
Target Milestone: --- → 1.1 CS (11may)
Updated•12 years ago
|
Assignee: waldron.rick → greg
Comment 12•12 years ago
|
||
reviewed on the PR
Comment 13•12 years ago
|
||
Is a new patch needed here?
Comment 14•12 years ago
|
||
new patches are constantly done on github's PR :)
(that's partly why PR are wrong, and partly why they're awesome).
Comment 15•12 years ago
|
||
Yeah, realized that last week :P
How far is this from landing then?
Comment 16•12 years ago
|
||
Comment on attachment 741379 [details]
pull request on github
Adding me as well here for helping Julien as well (being in the same timezone is a good point here! ;) ).
Attachment #741379 -
Flags: review?(fbsc)
Comment 17•12 years ago
|
||
Comment on attachment 741379 [details]
pull request on github
borja, I'll let you finish that review, I think the patch is not far from being finished.
Attachment #741379 -
Flags: review?(felash)
Comment 18•12 years ago
|
||
We are quite close to land this! :)
Comment 19•12 years ago
|
||
Comment 20•12 years ago
|
||
Updated•12 years ago
|
Attachment #747063 -
Attachment description: Issue 1 → Issue 1 - 'Blinking' text
Comment 21•12 years ago
|
||
Comment 22•12 years ago
|
||
Comment 23•12 years ago
|
||
Comment 24•12 years ago
|
||
Comment on attachment 741379 [details]
pull request on github
As we agreed, due to this code is risky, we are going to merge in a 'unstable' branch for moving forward. There are pending issues/regressions to be fixed, so we have to create a bug per each. Pending issues:
- **Tests are not working**:
[sms] compose_test.js Message Composition Placeholder "before each" hook:
TypeError: this._mozMobileMessage is undefined
- Composer input:
+ Black & blinking text (Issue 1 attachment)
+ 'setInputHeight' is not working as expected
+ Layout is broken, and is breaking the App layout as well (not only the composer) (Issue 2 attachment)
- Composer, recipients container:
+ Height has a 'pull-down' effect while typing. CSS is not working as expected. (Issue 3 attachment)
+ Header is pushed up, so the layout of the app is broken
- Counter
+ Counter style completely broken (Issue 4 attachment)
+ maxSegmentsInfo is not working at all (it was a Tef+ https://bugzilla.mozilla.org/show_bug.cgi?id=865411 that now is reopened)
(Issue 5 attachment)
- Other comments to be addressed:
+ Remove the bug https://bugzilla.mozilla.org/show_bug.cgi?id=869159 if is not in the code right now.
+ https://github.com/mozilla-b2g/gaia/pull/9385/files#r4141001. Removing logs.
This is R+ **only** for merging in the dev-branch. All these pending issues should be addressed for considering this US done.
Attachment #741379 -
Flags: review?(fbsc) → review+
Comment 25•12 years ago
|
||
Created https://bugzilla.mozilla.org/show_bug.cgi?id=870057 for the Composer input bugs
Updated•12 years ago
|
Whiteboard: [LOE:M] → [LOE:M], landed on dev-branch
Updated•12 years ago
|
Whiteboard: [LOE:M], landed on dev-branch → [LOE:M], landed on dev-branch [NO_UPLIFT]
Comment 26•12 years ago
|
||
Greg, FYI there has been some work from dan and I to get units passing again on https://github.com/gnarf37/gaia/compare/840069-squash - can you pick up this new squashed commit as your HEAD on the pull request?
Comment 27•12 years ago
|
||
After testing the branch there are several more issues pending:
- Fixing the unit tests
- Send message & enable send is not working. STR:
+ Create a recipient '123'
+ Go to input text
+ Tap on 'ENTER' several times
EXPECTED: Send button is not enabled
CURRENTLY: Send button is enabled, and when tapping on it nothing happens.
Tomorrow morning I will take a look about the bugs that we need to create for fixing all the issues.
Comment 28•12 years ago
|
||
please include the commit hash and url here :)
Whiteboard: [LOE:M], landed on dev-branch [NO_UPLIFT] → [LOE:M][landed on dev-branch][NO_UPLIFT]
Comment 29•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Whiteboard: [LOE:M][landed on dev-branch][NO_UPLIFT] → [LOE:M][NO_UPLIFT]
Updated•12 years ago
|
Whiteboard: [LOE:M][NO_UPLIFT] → [LOE:M]
Updated•11 years ago
|
Attachment mime type: text/plain → text/x-github-pull-request
You need to log in
before you can comment on or make changes to this bug.
Description
•