1203677 - Implement registerStep for FTU navigation

Status: RESOLVED FIXED

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

Description

[Sam Foster [:sfoster] (he/him)]

As a stepping stone to bug 916826, and to start to facilitate a more extensible Navigation flow, I'm proposing we implement a method of explicitly registering the steps. This should let us implement better flow control for the navigation steps with minimal changes on the surface.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] sfoster:ftu-navigation-bug-1203677 > mozilla-b2g:master

Comment 2

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8659448 [details] [review]
[gaia] sfoster:ftu-navigation-bug-1203677 > mozilla-b2g:master

Key points:

* Steps are now in a 0-based array. The existing step data is registered during init()

* Should be no real functional difference in how Navigation works at this stage

* registerStep takes beforeStep / afterStep options, this is only used in the test for now. I'm thinking of using this facility for late customization, we may also want to use it if we move some of the flow logic into the step data (kind of visitor pattern, discussed in bug 916826)


* I think the 'totalSteps' is some legacy code? I've re-named it unskippedStepsCount but if you look you'll see its not actually used. I didn't remove it yet as it seems like we might want to use it again, but can do so.

There's lots of things that I ran into that could be improved, but I want to take this a step(!) at a time to avoid a hairball of a commit that regresses everything and never ends up landing. That said, let me know if you see anything obviously wrong/redundant/misleading.

Comment 3

[Fernando Campo (:fcampo)]

Comment on attachment 8659448 [details] [review]
[gaia] sfoster:ftu-navigation-bug-1203677 > mozilla-b2g:master

Tried it a few times on flame, working nicely. Just optional nits for properties privacy.

so...does this mean that we have finally a plan for the refactoring? :)

Comment 4

[Sam Foster [:sfoster] (he/him)]

(In reply to Fernando Campo (:fcampo) from comment #3)
> Tried it a few times on flame, working nicely. Just optional nits for
> properties privacy.

\o/. Yeah I'll make a getter for stepCount. Still not sure about unskippedStepsCount, I guess I'll remove it for now we can always put it back. I'm not sure its worth making getters for the current/previousStepIndex properties - we need to set those in the tests so we'd just trade a little protection for some test hackery. 

> 
> so...does this mean that we have finally a plan for the refactoring? :)

I dont have a grand plan, just trying to get it into shape one commit at a time. I'll use the Late Customization feature to try and get something like a module pattern started and begin to define possible extension points. For the actual step sequence, building a state table as we register each step seems like our best bet? I'll try and prototype something.

Comment 5

[Sam Foster [:sfoster] (he/him)]

On master: https://github.com/mozilla-b2g/gaia/commit/93f7ade5d5b1030264ab0ace3a88f125e9e660c7