Closed
Bug 1162030
Opened 9 years ago
Closed 9 years ago
[Messages][NG] Figure out how navigation works
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: julienw, Assigned: julienw)
References
Details
(Whiteboard: [p(FxOS-S4)=2][p(FxOS-S3=3][p(2.2S13)=2][sms-sprint-2.2S14])
Attachments
(1 file)
No description provided.
Assignee | ||
Updated•9 years ago
|
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → felash
Assignee | ||
Updated•9 years ago
|
Summary: [Messages][New Gaia Architecture] use little-browser in views/index.html → [Messages][New Gaia Architecture] use gaia-navigator in views/index.html
Comment 1•9 years ago
|
||
Not in the sprint, but likely will be moving in "out-of-sprint" mode.
Whiteboard: [p=2] → [p=2][sms-sprint-2.2S14]
Assignee | ||
Updated•9 years ago
|
Summary: [Messages][New Gaia Architecture] use gaia-navigator in views/index.html → [Messages][New Gaia Architecture] figure out how navigation works
Updated•9 years ago
|
Updated•9 years ago
|
Assignee | ||
Comment 4•9 years ago
|
||
not working WIP
Assignee | ||
Updated•9 years ago
|
Blocks: sms-sprint-FxOS-S3
Whiteboard: [p=2][sms-sprint-2.2S14] → [p(FxOS-S3=3][p(2.2S13)=2][sms-sprint-2.2S14]
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8623785 [details]
github PR
Hey Steve, Oleg,
I think this is ready for a first feedback pass !
Thanks :)
Attachment #8623785 -
Flags: feedback?(schung)
Attachment #8623785 -
Flags: feedback?(azasypkin)
Comment 6•9 years ago
|
||
Comment on attachment 8623785 [details]
github PR
I think it's a good basis to iterate on. We can polish and improve Navigation progressively, especially we can simplify Navigation a lot once we split all panels and possibly move ReportView out of navigation subsystem and make it dialog instead.
It supports both NGA and old models, basically all we need :)
I haven't tested much, but in browser it works good so far!
Thanks!
Attachment #8623785 -
Flags: feedback?(azasypkin) → feedback+
Comment 8•9 years ago
|
||
Comment on attachment 8623785 [details]
github PR
Overall looks good and we can leave some known issues for follow up, thanks!
Attachment #8623785 -
Flags: feedback?(schung) → feedback+
Assignee | ||
Comment 9•9 years ago
|
||
Comment on attachment 8623785 [details]
github PR
Hey folks,
I think this is ready for a review, at last !
I didn't handle all your comments but I think we can handle those in follow-ups.
Things that come to my mind:
* the report view animation should not be a panel animation, but should be a dialog animation.
* possibly use URL and URLSearchParams in Utils.params and Utils.url
Attachment #8623785 -
Flags: review?(schung)
Attachment #8623785 -
Flags: review?(azasypkin)
Comment 10•9 years ago
|
||
Hey Julien, Sorry for the delay, I've almost reviewed your PR, but I need some more time to finish review and play with the patch on device. Will be the first thing I'll do on Thursday :) Thanks!
Status: NEW → ASSIGNED
Assignee | ||
Updated•9 years ago
|
Blocks: sms-sprint-FxOS-S4
Whiteboard: [p(FxOS-S3=3][p(2.2S13)=2][sms-sprint-2.2S14] → [p(FxOS-S4)=2][p(FxOS-S3=3][p(2.2S13)=2][sms-sprint-2.2S14]
Comment 11•9 years ago
|
||
Hey, I've left my last comments/questions at Github, mostly nits. I've briefly tested on device and it looks good so far! However I noticed some Gecko graphics issues that probably have been made more visible with the new changes. So I'd prefer to keep r? flag for now and finish testing once Gecko is fixed. Thanks!
Assignee | ||
Comment 12•9 years ago
|
||
Pushed a new version with comments fixed :) There are 2 commits because I've put the changes to navigateToComposer to a separate commit.
Assignee | ||
Comment 13•9 years ago
|
||
Pushed a version with integration tests fixed... It's actually important to wait for Drafts.request() before saving the draft in activity_handler.js. So I think it's ready for a new review. The gaia try for the new push won't have the unit tests properly run because of bug 1190180. I also found a bug I'll file separately: in the "new" activity, if we find a contact for a phone number, we lose the body if it was specified.
Assignee | ||
Comment 14•9 years ago
|
||
Pushed a new version with (I think) the last issues we saw fixed. This works well on my device (although everything feels slow -- but I don't think this is my fault :D)
Assignee | ||
Comment 15•9 years ago
|
||
Everything was feeling slow... and it was partly my fault :) Now this is faster than master ! Just pushed a new version.
Comment 16•9 years ago
|
||
Comment on attachment 8623785 [details]
github PR
Looks good to me!
Just small nit at github and two issues that we're going to handle in follow-ups:
* "Cancel" button in "Discard unsent message" dialog doesn't work;
* We load Inbox before going to the notification conversation when app is run via notification click.
Great job!
Attachment #8623785 -
Flags: review?(azasypkin) → review+
Comment 17•9 years ago
|
||
Comment on attachment 8623785 [details]
github PR
Thanks for all the great works :) It's really awesome and we can file some minor follow up later instead on blocking this for master.
Attachment #8623785 -
Flags: review?(schung) → review+
Assignee | ||
Comment 18•9 years ago
|
||
landed in master: 68d369c2a2b0cd16db028f0c2e660107c52ce113
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 19•9 years ago
|
||
(In reply to Oleg Zasypkin [:azasypkin][⏰UTC+1] from comment #16) > Comment on attachment 8623785 [details] > github PR > > Looks good to me! > > Just small nit at github Fixed the nit. > * "Cancel" button in "Discard unsent message" dialog doesn't work; This was fixed. > * We load Inbox before going to the notification conversation when app is > run via notification click. Filed bug 1192263. > * the report view animation should not be a panel animation, but should be a dialog animation. Filed Bug 1192267. > * possibly use URL and URLSearchParams in Utils.params and Utils.url Filed bug 1192266. More issues I found while working on this bug: * bug 1190275 - [Messages] Image attachments are not showing up properly when going to a subview and coming back * bug 1190276 - [Messages] In a "new" activity we don't keep the body if the phone number has a conversation already
You need to log in
before you can comment on or make changes to this bug.
Description
•