Closed
Bug 1087933
Opened 10 years ago
Closed 8 years ago
Provide a way to switch from thread view to composer view
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: augustin.trancart, Assigned: augustin.trancart)
References
Details
Attachments
(2 files)
As of bug 918970, we now redirect to a thread (if it exists) from the contact app. We should provide a way to switch from a thread view to the composer view. The thread participant recipient must be in the new message recipient, and if the user was writing a message, it should be kept when switching to a new message.
Assignee | ||
Comment 1•10 years ago
|
||
Here it is. Should I NI Jenny to validate the description?
Flags: needinfo?(felash)
Comment 2•10 years ago
|
||
Hey Jenny, this is a follow-up of your comment in Bug 918970 comment 55 :) This is also one of the things we discussed by mail. NI you so that you don't forget ;)
Flags: needinfo?(felash)
Hi there! I believe I'll be able to work on this in late November after 2.2 specs are delivered. Will get back to this, thanks!
Flags: needinfo?(jelee)
Assignee | ||
Comment 5•10 years ago
|
||
Hey Jenny! Did you have time to work on this? Thanks!
Flags: needinfo?(jelee)
Hello, see attached for first version of spec ;) Thanks!
Flags: needinfo?(jelee)
Comment 7•10 years ago
|
||
Just a notice to Augustin: don't pay attention to the design which uses next design of building blocks; just pay attention to the UX flow.
Assignee | ||
Comment 8•10 years ago
|
||
Flags: needinfo?(felash)
Assignee | ||
Comment 9•10 years ago
|
||
Hey Julien! I NI you because I have some questions about implementations. The patch is not ready yet. I need to write some unit tests, and maybe some marionette tests as well?
Comment 10•10 years ago
|
||
Answered on github :) Note that the "feedback" flag is especially useful for such uses ! Marionette tests could be nice, especially because we'll eventually implement 2 different objects for the thread panel and the composer panel, and we don't want that this regresses.
Flags: needinfo?(felash)
Updated•9 years ago
|
Assignee: nobody → augustin.trancart
Comment 11•9 years ago
|
||
Comment on attachment 8541259 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26996 Hey Julien, It's been a long time, but here you are! My patch's updated \o/ For testing, I only added marionette tests. They seemed more relevant to me than unit testing there, but feel free to tell me if you see a good candidate for unit testing. Thanks.
Attachment #8541259 -
Flags: review?(felash)
Updated•9 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment 12•9 years ago
|
||
Comment on attachment 8541259 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26996 As we discussed, this needs to be rebased now that bug 1162030 landed. Sorry about that ! Fortunately now this part won't change much for hopefully a long time :) I gave some directions on github, I hope this won't be too difficult now :)
Attachment #8541259 -
Flags: review?(felash)
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8541259 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26996 Hey Julien, Here is the second part, now a lot more simpler with drafts. As the patch is really different than before, I thought more relevant not to push follow ups commit, but starting over instead. Please r?
Attachment #8541259 -
Flags: review?(felash)
Comment 14•9 years ago
|
||
If that's fine to you, I'd like to wait for after the 2.5 branching before landing this. So it's quite lower priority for me during this week. I'll look at it for sure next week, but maybe this week if I have time :)
Assignee | ||
Comment 15•9 years ago
|
||
Completely fine, I didn't expect this to land in 2.5 :-)
Assignee | ||
Comment 16•9 years ago
|
||
Hey Julien, just a little reminder for you to review this, whenever you can :-)
Flags: needinfo?(felash)
Comment 17•9 years ago
|
||
yeah I know, crazy times :/ I know you're in holiday during this week and I'm sorry I was as busy :/
Flags: needinfo?(felash)
Comment 19•9 years ago
|
||
I left some comments already. I think you can already work on the integration tests, as they won't likely change even with any change I can ask on the main code. I'll give you more comments on the main code tomorrow. Sorry again for the delay !
Comment 20•9 years ago
|
||
Comment on attachment 8541259 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26996 Left comments on github :) Please ping me in case you have questions ! Thanks again for your work, this is really good ! And sorry for the delay...
Attachment #8541259 -
Flags: review?(felash)
Assignee | ||
Comment 21•9 years ago
|
||
Comment on attachment 8541259 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26996 Hey Julien, another try! Please r?
Attachment #8541259 -
Flags: review?(felash)
Comment 22•9 years ago
|
||
Hey Matej,
can you have a look at the strings in the spec attachment 8533556 [details] ?
What I'd like to double check is the new string "Add recipient". It will be in the menu that's appearing when the user taps the top right button. And the goal is to move to a prefilled "new message" panel so that the user can add more recipients to this message.
That's why I would favor the string "Add more recipients" instead of "Add recipient". What do you think ? Maybe you'll have better ideas as well :)
Flags: needinfo?(matej)
Comment 23•9 years ago
|
||
Note that the current patch uses "Add Recipients" (with a final 's' and a capital letter at 'Recipient' :) ).
Comment 24•9 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #22) > Hey Matej, > > can you have a look at the strings in the spec attachment 8533556 [details] ? > > What I'd like to double check is the new string "Add recipient". It will be > in the menu that's appearing when the user taps the top right button. And > the goal is to move to a prefilled "new message" panel so that the user can > add more recipients to this message. > > That's why I would favor the string "Add more recipients" instead of "Add > recipient". What do you think ? Maybe you'll have better ideas as well :) "Recipient" feels a bit cold and technical. Could we say something like "Add someone else" instead? I actually don't mind it being singular, though. Saying "recipients" makes it feel like it's specifically for adding more than one person, but may lead someone to wonder how they just add one more.
Flags: needinfo?(matej)
Comment 25•9 years ago
|
||
"Add someone else" is maybe too generic though. Especially "Add", I'm not sure this convey enough meaning. Another word could be "receiver", but it's really the same feeling. More ideas: * "Include more participants" * "Include someone else"
Flags: needinfo?(matej)
Comment 26•9 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #25) > "Add someone else" is maybe too generic though. Especially "Add", I'm not > sure this convey enough meaning. > > Another word could be "receiver", but it's really the same feeling. > > More ideas: > * "Include more participants" > * "Include someone else" I like the second one here. It's friendlier.
Flags: needinfo?(matej)
Comment 27•9 years ago
|
||
Comment on attachment 8541259 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26996 added few more comments. We're closer than ever ! This is really cool !
Attachment #8541259 -
Flags: review?(felash)
Comment 28•8 years ago
|
||
Comment on attachment 8541259 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26996 hi Julien, here is another iteration, please r?
Attachment #8541259 -
Flags: review?(felash)
Comment 29•8 years ago
|
||
Comment on attachment 8541259 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26996 Small nit in the test, I think we can do a cleaner job with the keyboard handling. We're nearly there !
Attachment #8541259 -
Flags: review?(felash)
Assignee | ||
Comment 30•8 years ago
|
||
Comment on attachment 8541259 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26996 Hey Julien please r? again :-)
Attachment #8541259 -
Flags: review?(felash)
Comment 31•8 years ago
|
||
Comment on attachment 8541259 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26996 Sorry Augustin, it's orange :( https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=8d832d45e4308c9319c0a56e539a4bbbfb5a2a6d&selectedJob=3389822 Moreover I wanted to mutualize things in one place, here I think it's better to use it in lib/keyboard...
Attachment #8541259 -
Flags: review?(felash)
Assignee | ||
Comment 32•8 years ago
|
||
Comment on attachment 8541259 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26996 Hey Julien, Ooops! Corrected :-) Also, I've tried to refit this as you wanted, but disclaimer: I'm not sure to have understood it properly :-) Please r?
Attachment #8541259 -
Flags: review?(felash)
Comment 33•8 years ago
|
||
Comment on attachment 8541259 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26996 I left some small nits but r=me once you fix them and you check the SMS tests are green. Thanks a lot, this is a great patch ! Hopefully it won't be the final one ;)
Attachment #8541259 -
Flags: review?(felash) → review+
Comment 34•8 years ago
|
||
Don't forget to squash as well, of course
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 36•8 years ago
|
||
master: 4f0e2a1a42a2d049b6fe8f4f095cdcdf0fd5465c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•