Closed Bug 911681 Opened 11 years ago Closed 11 years ago

[FTU] replace tutorial images for large device

Categories

(Firefox OS Graveyard :: Gaia::First Time Experience, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::First Time Experience
Platform:
Other
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-b2g:koi+, b2g-v1.2 fixed)

Status:
RESOLVED FIXED
Project Flags:
blocking-b2g koi+
Tracking Flags:
Tracking Status
b2g-v1.2 --- fixed

People

(Reporter: gasolin, Assigned: eragonj)

References

Details

(Whiteboard: [Flatfish only] [l10n])

Attachments

(6 files)

[Tablet] FTU(Tutorial)_20130911.pdf
1.43 MB, application/pdf
Details
FTU_20130911.zip
5.36 MB, application/zip
Details
FTU_20130912.zip
4.90 MB, application/zip
Details
Pointer to Github pull request 12213.html
290 bytes, text/html
borjasalguero
: review+
alive
: review+
davehunt
: review+
squib
: review+
etienne
: feedback+
Details
FTUTutorial_20130925.zip
3.24 MB, application/zip
Details
FTU_20130926.zip
5.05 MB, application/zip
Details 
current tutorial images are only for portrait, need to provide landscape images for tablet device.
FTU page 4 should be scroll up to homescreen instead of press home button in tablet device. 

(engineer need to use some ScreenLayout detection technique to handle this)


carrie, please help referring this issue to proper UX colleague, thanks.
Blocks: flatfish
blocking-b2g: --- → koi?
Depends on: 911673
Flags: needinfo?(cawang)
blocking-b2g: koi? → koi+
Hi Neo, please help to clarify the issue. Thanks!

(In reply to Fred Lin [:gasolin] from comment #1)
> FTU page 4 should be scroll up to homescreen instead of press home button in
> tablet device. 
> 
> (engineer need to use some ScreenLayout detection technique to handle this)
> 
> 
> carrie, please help referring this issue to proper UX colleague, thanks.
Flags: needinfo?(cawang)
Hi Neo, please help to clarify the issue. Thanks!

(In reply to Fred Lin [:gasolin] from comment #1)
> FTU page 4 should be scroll up to homescreen instead of press home button in
> tablet device. 
> 
> (engineer need to use some ScreenLayout detection technique to handle this)
> 
> 
> carrie, please help referring this issue to proper UX colleague, thanks.
Flags: needinfo?(nhsieh)
After I discuss this topic with SW & VD. According to schedule issue, now we only can have two FTU screens in 1.2 tablet. But we don't know is it OK for PM ?  Or we need to push this feature to 1.3 ?
Flags: needinfo?(nhsieh) → needinfo?(styang)
For 1.2, I'm ok to have the necessary but not all pages to indicate users. But we should have all pages implemented in v1.3.
Flags: needinfo?(styang)
Assignee: nobody → ejchen
@juwei, please attach FTU images here, thanks!
Flags: needinfo?(jhuang)
Hi Fred,
Wireframes has attached, images will deliver later.
Flags: needinfo?(jhuang)
Attached file FTU_20130911.zip — 
Images attached.
juwei, thanks!


ej, please rise you hand if anything need to be clarified.
@Juwei, 

Thanks for your great help !


@Fred,

Yes, for sure. I am working on the redesign of this change and will ping you if with problems. Thanks.
Attached file FTU_20130912.zip — 
Hi Fred & EJ,
New layout version uploaded.
@Juwei, 

Thanks again for the update ! The tutorial is almost done and will show you a demo later.
Hi Etienne, 

I just finished this ftu update. I am still working on tests. After finished, they are ready to go. Maybe you can check the code first for me. Thanks :D
Attachment #804392 - Flags: feedback?(etienne)
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

Hi Etienne, 

I just finished my tests and it works well for all unit tests. 

Please help me review codes. Thanks
Attachment #804392 - Flags: feedback?(etienne) → review?(etienne)
Hi Alive,

I just checked ftu_launcher.js and I want to know is there any way that Tutorial.js can tell launcher that we are now at the last step and we can take off the stopimmediatepropagtion() to let users can swipeup to exti FTU ?

Thanks.
Flags: needinfo?(alive)
(In reply to ejchen from comment #15)
> Hi Alive,
> 
> I just checked ftu_launcher.js and I want to know is there any way that
> Tutorial.js can tell launcher that we are now at the last step and we can
> take off the stopimmediatepropagtion() to let users can swipeup to exti FTU ?
> 
> Thanks.

This is a hack. What we need to do is co-operate with UX to resolve https://bugzilla.mozilla.org/show_bug.cgi?id=911673
"Does each go to home transition replace the original app closing transition?"
If so we don't need hack as comment 14.
Flags: needinfo?(alive)
OK I think I know what you want but now it's impossible to do so, except IAC.
Okay, I will check IAC later. 

Thanks for your great help !
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

This looks good but it should be reviewed by a FTU peer, I'm not well versed in this part of the code.
Attachment #804392 - Flags: review?(francisco.jordano)
Attachment #804392 - Flags: review?(etienne)
Attachment #804392 - Flags: feedback+
Hi all, 

I just made an update using IAC to communicate between system app and communications app. It works well if your gecko build is built later than 9/12 (you can check IAC here : https://bugzilla.mozilla.org/show_bug.cgi?id=876397).
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

Hi Alive,

Can you help me review the IAC part in system app ? Thanks :P
Attachment #804392 - Flags: review?(alive)
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

Basically r+ but see comments.
Attachment #804392 - Flags: review?(alive) → review+
Hi huwei I saw 2 issues in FTU images:

1. dialer/sms/usage/FM app are hidden in tablet, does those icons confuse user?
2. default icons in bottom dock are not reflect current layout (current default tablet dock are calendar, email, browser, gallery, music)

https://github.com/mozilla-b2g/gaia/blob/master/distribution_tablet/homescreens.json
Flags: needinfo?(jhuang)
sorry juwei, forgive my typo :p
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

Original behavior is let ftu to do window.close() and after that it won't exist anymore. In this change ftu would stay there if use press home or dispatch home event by gesture, and then ftu would stay opened. This consumes memory and make user confusing.

Please do kill FTU when (evt.type == home + _bypassHomeEvent == true).
Attachment #804392 - Flags: review+
Alive, 

I just updated the code, 

please check it again if you are free. Thanks
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

Hi Francisco and Alive,

Please help me review my change. Thanks for your great help.
Attachment #804392 - Flags: review?(alive)
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

r=me
Attachment #804392 - Flags: review?(alive) → review+
Hi Fred!
I've updated the images, please refer to attachment, thanks !
Flags: needinfo?(jhuang)
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

Reassigning review to Borja.

BTW, could you rebase?

Thanks a lot for your work!
F.
Attachment #804392 - Flags: review?(francisco.jordano) → review?(fbsc)
Francisco, 

thanks for your reassigning ! and I just rebased the code.


Borja,

I just left some comments in github please read it when you have time. Thanks for your kindly review by the way :D
Attached file FTU_20130926.zip — 
Hi Fred,
FTU assets updated, please refer to attachment, thanks!!
Hi Juwei, 

thanks for your help ! I just updated the tutorial images.
Hi Alive, 

After asking Gene some details about IAC, he told me that there is only one window.navigator.mozSetMessageHandler can be used with an app. 

Because currently both FTU and music (lockscreen has to show music info) use this function, for future development, I added a script called iac_handler.js to let make us call related exposed API (by checking incoming keywords) when receiving IAC keywords. 

This may be a small change, so if possible, you can leave some recommendations about this change for me. Thanks :D
Flags: needinfo?(alive)
That sounds sane. Let me know when you need feedback on the implementation.
Flags: needinfo?(alive)
@Alive, 

I am glad it's ok to you. 

I have implemented it and you can check the source code here : 

https://github.com/EragonJ/gaia/blob/3e35ceff7e7298a12d8ca7c39dc4e0c3445f787a/apps/system/js/iac_handler.js
(In reply to EJ Chen [:eragonj] from comment #36)
> @Alive, 
> 
> I am glad it's ok to you. 
> 
> I have implemented it and you can check the source code here : 
> 
> https://github.com/EragonJ/gaia/blob/
> 3e35ceff7e7298a12d8ca7c39dc4e0c3445f787a/apps/system/js/iac_handler.js

I cannot comment on your repo so please have a PR and ask feedback if you really want.
*Please try not to call other's method directly*
Thanks Alive, 

I think dispatching event would be a better way to implement this. 

In current design, it will fetch out keywords from manifest and check the keyword for you. All you have to do is register related event (prefix with iac-*) and put your logic in your js. I think this approach can be a shared js later if someone wants to do the same thing with IAC mechanism. 

What do you think, Alive ?

https://github.com/mozilla-b2g/gaia/pull/12213
Flags: needinfo?(alive)
Hi Borja,

please help me review the code again when you have time. I think we are almost done ! THanks
Flags: needinfo?(alive) → needinfo?(borja.bugzilla)
Whiteboard: [Flatfish only]
Hi Francisco, 

It seems that Borja just got relocated to UK and may be a little bit busy. Should I forward the reviewing process to someone else or wait for him (because He is really a busy guy xD) ?
Flags: needinfo?(francisco.jordano)
Hi!

You can forward the reviews to myself or :fcampo

Cheers!
F.
Flags: needinfo?(francisco.jordano)
Flags: needinfo?(borja.bugzilla)
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

Thanks Francisco. I really appreciate it :)

I will quickly explain details about this patch. It is a patch that can help tablet customize its own FTU steps. ( different from mobile and in my current design, it can be used in the future if we want to support more different devices) 

Because of some UX design, I use the new IAC (Inter App Communication API, you can check the document on wiki here : https://wiki.mozilla.org/WebAPI/Inter_App_Communication_Alt_proposal) to communicate between system app and communication app. 

IAC part just got review+ from Alive and it works well in mobile and tablet. You can try the patch when navigating FTU with latest gecko (or commits later than 2013/09/13, I remembered it is the date Gecko landed the basic version of IAC support)

Hope these information helps, thanksssss !
Attachment #804392 - Flags: review?(borja.bugzilla) → review?(fernando.campo)
Wow, that's a big piece of pull request :D

I'll read the IAP doc and try to handle the review ASAP, but right now I have a few koi+ on my table, so this one probably will take a while. Please bear with me.
(In reply to Fernando Campo (:fcampo) from comment #43)
> Wow, that's a big piece of pull request :D
> 
> I'll read the IAP doc and try to handle the review ASAP, but right now I
> have a few koi+ on my table, so this one probably will take a while. Please
> bear with me.

Thanks for your reviewing, Fernando. I know it is not a small patch so that I have to tell you more details about the implementations to help you review the patch on the fly. You can work on your koi+ bug first then give me a review when you have more spare time.

Thanks again for your big help, I can work on the other bugs now ;D
Hi Juwei, 

@borja has some questions about UX problems here : https://github.com/mozilla-b2g/gaia/pull/12213#issuecomment-26426291

Can you reply him about his questions here in bugzilla ? Thanks for your great help.
Flags: needinfo?(jhuang)
Hi Broja,
Thanks for your concern !! In the perspective of UX, the reason we don't align the same design is because the button will be getting too wide and doesn't look like a button. So we design a proper width for the button to make it looks tappable & comfortable.
Thanks:)
Flags: needinfo?(jhuang)
Whiteboard: [Flatfish only] → [Flatfish only] [l10n]
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

It seems Borja is really back and we are almost done with this bug !! 

I just reassigned reviewer to him and you can focus on your work, thanks ;)
Attachment #804392 - Flags: review?(fernando.campo) → review?(borja.bugzilla)
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

Awesome work here! Now tutorial and all fixes in the FTU have improved the responsiveness of the app. Tablet version rocks! R+ for sure. :)
Attachment #804392 - Flags: review?(borja.bugzilla) → review+
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

Hi Davehunt, 

thanks for helping me setup gaia-ui-tests environment !! In this patch, because 2 ~3 selectors got changed, I updated 2 related ui tests to make Travis happy. 

Please help me review related python files. Thanksss :)
Attachment #804392 - Flags: review?(dave.hunt)
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

Test changes look good, thanks!
Attachment #804392 - Flags: review?(dave.hunt) → review+
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

@Jim, your music app has been modified and I think I may need your help about this change in order not to break your music app. 

Currently, it seems that FTU and music will need system's help for IAC support. But in each app, we can only have one mozSetMessageHandler, that's why I made iac_handler.js for use in system app to make everyone use.

Can you help me review related changes ? thanks.
Attachment #804392 - Flags: review?(squibblyflabbetydoo)
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

Some of the IAC stuff here seems wrong, namely that we only store one IAC port, even though there can now be two of them: one for ftu and one for media. I haven't run with this to be sure, but this is probably wrong. I think it would technically work, since FTU will be done by the time the user can open the music app, but we should fix this now so that no one gets bitten by this issue in the future.
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

r-'ing for the above comment and because I'd like to see some unit tests that test multiple IAC connections.

That said, I think this will actually simplify one of my patches (bug 891024), so thanks! :)
Attachment #804392 - Flags: review?(squibblyflabbetydoo) → review-
@Jim, thanks for your quick review. This really gives me a nice hint about what to do next. I will use feedback later to ask for your advices.  :)
Attachment #804392 - Flags: review- → review?(squibblyflabbetydoo)
@Jim, 

maybe you can take a look at the patches again. Thanks !!
Comment on attachment 804392 [details]
Pointer to Github pull request 12213.html

Overall, this looks good. I think we should move apps/system/js/iac_handler.js to shared/js/iac_handler.js, though. That code would be pretty useful for any other app trying to do something similar.
Attachment #804392 - Flags: review?(squibblyflabbetydoo) → review+
Thanks all. This patch got landed on master : 754d45c663b96a3a9fb5c92b4aa91c2909717970
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Congrats EJ, thank you for handling this !
status-b2g-v1.2: --- → affected
I was not able to uplift this bug to v1.2.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1.2, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1.2
  git cherry-pick -x -m1 754d45c663b96a3a9fb5c92b4aa91c2909717970
  <RESOLVE MERGE CONFLICTS>
  git commit
Flags: needinfo?(ejchen)
Thanks John, I just solved conflicts. 

Landed on gaia/v1.2 : 2cc53db01978dbe99a1d2cf1f3c84e313c394056
Flags: needinfo?(ejchen)
Reopening: see https://bugzilla.mozilla.org/show_bug.cgi?id=934690#c10
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
EJ will fix the 1.2 uplift in bug 934690. Thanks EJ!
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
(In reply to Michael Henretty [:mhenretty] from comment #62)
> EJ will fix the 1.2 uplift in bug 934690. Thanks EJ!

Thanks Michael, 

I will handle that on bug 934690. :)
Depends on: 934011
Depends on: 939310
No longer depends on: 939310
Per the discussion on bug 934011, this needs to be backed out of 1.2 since we are no longer shipping flatfish on 1.2 & this causing blocking regressions.

John - Can you back this out of 1.2?
Flags: needinfo?(jhford)
Jason, 

after discussing with Tim and the others Gaia developers. 

The risk of backing out bug 911681 is too high and we can mark bug 934011 as "approval-gaia-v1.2" to make it uplifted on v1.2 then I can land my bug (bug 934690) to make FTU / music app all work. 

What do you think ?
Flags: needinfo?(jsmith)
Okay - if it's too risky to backout, then let's go down the path you've suggested.
Flags: needinfo?(jsmith)
Flags: needinfo?(jhford)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: