976535 - [FTE] Refactor tutorial

Status: RESOLVED FIXED

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

Description

[Borja Salguero [:borjasalguero]]

Currently tutorial is handled by JS, and we can simplify this with CSS only. Removing this navigation here is the first step to reduce the complexity as well here https://bugzilla.mozilla.org/show_bug.cgi?id=916826.

Comment 1

[Borja Salguero [:borjasalguero]]

Attachment: Pull request

Comment 2

[Borja Salguero [:borjasalguero]]

This is the fist step for testing a simple navigation, where we reduce the JS logic (for example for the progress bar) where it's not needed.

We are supposed to be working in a lot of new devices (new tablets, resolutions...), so dealing with CSS  & responsive design is a must here.

Comment 3

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8382128 [details] [review]
Pull request

Left some comments on Github, Borja ! Please check it ! 

Nice work, by the way. :P

Comment 4

[Borja Salguero [:borjasalguero]]

Comment on attachment 8382128 [details] [review]
Pull request

Updating the flag properly as all comments were addressed yesterday. Thanks for all the comments!

Comment 5

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8382128 [details] [review]
Pull request

Nice work Borja !! These changes are awesome !!! :P

Comment 6

[Fernando Campo (:fcampo)]

Comment on attachment 8382128 [details] [review]
Pull request

Sorry it took me long, honestly I thought I've had given the f+ lat week :S

Anyway, I like the way it's handled, and as we talked offline, I would like to apply this approach to the rest of the FTU (that needs a good refactor so badly).

I left couple of comments on github anyway, to make code a little more clear.

Great job Borja!

Comment 7

[Guillaume Marty [:gmarty]]

Hi guys.
Can we land this patch? What we need is probably a bit of merging when bug 984404 lands (today or tomorrow) and then update the tests.

Borja, do you have time to do that? If not, I can do it.

The reason I want this quickly is because I've started working on refactoring the navigation, as per bug 984404 and bug 916826.

Comment 8

[Borja Salguero [:borjasalguero]]

Hi Guillaume! Im gonna try to update this today.

On the other hand I've seen you are interested in working on the refactor of the Navigation of FTU, which is a really good idea. This bug was discussed previously with Eragon, trying to get a common idea about how the Navigation object should look like (take a look to bug https://bugzilla.mozilla.org/show_bug.cgi?id=927807). Which is your final idea about the refactor of Navigation? Please feel free to assign to you the bug https://bugzilla.mozilla.org/show_bug.cgi?id=916826, and dont hesitate to ask! Definitely this is a great win, and I'm more than happy if you can work on it (and help you if needed as well). Thanks!

Comment 9

[Guillaume Marty [:gmarty]]

Thanks, that would be great to get this PR updated and landed!

I'm pretty new to the code base, so I'd appreciate if you can give me some help to get me started in the right direction. When are you free to discuss this?

Comment 10

[Borja Salguero [:borjasalguero]]

Comment on attachment 8382128 [details] [review]
Pull request

Ready to review! Now with tests & rebased.

Comment 11

[Borja Salguero [:borjasalguero]]

(In reply to gmarty from comment #9)
> Thanks, that would be great to get this PR updated and landed!
> 
> I'm pretty new to the code base, so I'd appreciate if you can give me some
> help to get me started in the right direction. When are you free to discuss
> this?

Guillaume, I've just assigned to you the bug https://bugzilla.mozilla.org/show_bug.cgi?id=916826, so we can talk directly there. Please use 'needinfo' flag when you need my review,feedback or help there!

On the other hand, it would be great if you can describe your idea about the refactor in the bug 916826, so we will have a clear picture of the things needed to implement, and the discussion & suggestions added. Wdyt?

Thanks again and welcome to FTU! :)

Comment 12

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Comment on attachment 8382128 [details] [review]
Pull request

Nice work Borja +++++++

I left some comments on the bug and please check it when you have time !

Bypass the r? to Fernando :)

Comment 13

[Guillaume Marty [:gmarty]]

The PR looks good, Borja can you address the few comments by eragonj? Then we can merge and I can start working on the incremental refactoring we discussed last week.

Comment 14

[Fernando Campo (:fcampo)]

Just waiting to have access to a tablet to test the PR. 
Code looks good and works nice on Nightly, but I want to be sure we don't break anything

Comment 15

[Borja Salguero [:borjasalguero]]

Updating right now! :)

Comment 16

[Fernando Campo (:fcampo)]

Comment on attachment 8382128 [details] [review]
Pull request

tablet seal of approval :)

Comment 17

[Guillaume Marty [:gmarty]]

Cool! That means that we can merge it? :-)

Comment 18

[Guillaume Marty [:gmarty]]

Setting blocking for bug 949439 as per https://github.com/mozilla-b2g/gaia/pull/18550#issuecomment-41118375.

Comment 19

[Borja Salguero [:borjasalguero]]

https://github.com/mozilla-b2g/gaia/commit/ddcc24e4f37de64faf2387267de48db8a59956c8
https://github.com/borjasalguero/gaia/commit/5646b05956575b795b2568b8490f553daf61539a

R+. Travis green. Merged!