984404 - [FTE] Wrong step indicator

Status: RESOLVED FIXED

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

Description

[Fernando Campo (:fcampo)]

When we go through all the steps on the First Time Experience, the step indicator shows incorrect positions:
- The last step [About B2G OS (2)] is signalled on the step indicator as the previous-to-last
- Jump from [Import Contacts] to [About B2G OS (1)] is quite bigger than the rest of the step

Comment 1

[Fernando Campo (:fcampo)]

Attachment: Import section

Check the position of the marker before the jump

Comment 2

[Fernando Campo (:fcampo)]

Attachment: About section

Now the indicator has moved way more than one step space

Comment 3

[Fernando Campo (:fcampo)]

Attachment: Last step

There's still space for another step, but we are on the last one

Comment 4

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Attachment: Pull Request #17783 - [FTU] Update navigation progress bar logic

Some refactoring was required to properly account for skipping steps in the FTU. Let me know if it looks good (added tests as well).

Comment 5

[Fernando Campo (:fcampo)]

Comment on attachment 8399097 [details] [review]
Pull Request #17783 - [FTU] Update navigation progress bar logic

Code looks good Mihai, thanks for taking care of it. Just some comments on github, we can merge after fixing those (merely readability and improve tests).

I tested it on device and works good, but I need to test if on a Fugu to see that it doesn't break any DSDS behaviour (I don't think it will, judging from the code, but I'd rather not risk it).

Also, I'm r+ this because is a quicker patch than waiting for the full refactor from bug 916826 (hopefully we'll fix the current navigation mess in there)

Comment 6

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Comment on attachment 8399097 [details] [review]
Pull Request #17783 - [FTU] Update navigation progress bar logic

Thanks for the review Fernando! I updated the tests in the PR, let me know if it is good to merge.

Comment 7

[Fernando Campo (:fcampo)]

Comment on attachment 8399097 [details] [review]
Pull Request #17783 - [FTU] Update navigation progress bar logic

Hi Mihai, Even if code seems ok at first, I have to r- it after testing it on device. Basically, if you try without a SIM card inserted, we don't show the cellular data screen (ok), but the step indicator clearly jumps one position (not ok).

The idea is that the length of the step indicator varies depending on the total number of steps, so we don't have this kind of jumps (see bug 826623 comment 6).

Comment 8

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Comment on attachment 8399097 [details] [review]
Pull Request #17783 - [FTU] Update navigation progress bar logic

Thanks for the input Fernando, I had the impression that the respective step jump should be intentional... The PR is now updated to meet the requirements (did a refactor of my previous approach, this one should be cleaner).

Let me know if it is good to go.

Comment 9

[Fernando Campo (:fcampo)]

Comment on attachment 8399097 [details] [review]
Pull Request #17783 - [FTU] Update navigation progress bar logic

Thanks Mihai,

code looks good, and works fine on the device.

Comment 10

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Thanks for the review, Fernando!

Landed on master:
https://github.com/mozilla-b2g/gaia/commit/6638a589bb0ba7e7e8f6acd3e07f668cfb4cfc11