924764 - Collection design needs updating

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Everything.me, defect)

Description

[Evyatar 'Tron' Amitay (everything.me)]

Current collection design causes UX confusion and should be changed:
1) Remove the "Phone" suffix form the collection name
2) Darken the header overlay more, to make the device background less visible
3) Change the animation of the collection to match that of opening the marketplace - the header should slide in from the top, and the content should slide up from the bottom.

Comment 1

[Evyatar 'Tron' Amitay (everything.me)]

4) Change the font of the collection name to match headers across the OS (as per https://bugzilla.mozilla.org/show_bug.cgi?id=923491#c14)

Comment 2

[Evyatar 'Tron' Amitay (everything.me)]

Attachment: Patch - redirect to github PR

Comment 3

[Cristian Rodriguez (:crdlc)]

Comment on attachment 814817 [details]
Patch - redirect to github PR

Excellent work!

Comment 4

[Evyatar 'Tron' Amitay (everything.me)]

merged in master
e45fa382af8341f7db70a8d5bf7dff7f5067a87d

https://github.com/mozilla-b2g/gaia/commit/e45fa382af8341f7db70a8d5bf7dff7f5067a87d

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

I'm sorry, but this caused a fairly frequent failure in the gaia_ui_tests, so I backed it out:

  https://github.com/mozilla-b2g/gaia/commit/f5c2e987fcc859e371fedb44006ca94fc0ebcb28

Please see bug 925342 for details on which test and what the error was.

Comment 6

[Amir Nissim (:amirn)]

Attachment: redirect to PR 12805.html

Comment 7

[Amir Nissim (:amirn)]

Comment on attachment 816327 [details]
redirect to PR 12805.html

only difference from previous patch is in test files:
tests/python/gaia-ui-tests/gaiatest/apps/homescreen/regions/search_panel.py
https://github.com/mozilla-b2g/gaia/pull/12805/files#diff-4

and 
tests/python/gaia-ui-tests/gaiatest/tests/functional/everythingme/test_everythingme_launch_packaged_app.py
https://github.com/mozilla-b2g/gaia/pull/12805/files#diff-5

Comment 8

[Cristian Rodriguez (:crdlc)]

Comment on attachment 816327 [details]
redirect to PR 12805.html

Please Evyatar review it because right now I don't have time to do this, sorry

Comment 9

[Evyatar 'Tron' Amitay (everything.me)]

Comment on attachment 816327 [details]
redirect to PR 12805.html

on my end it looks good, we simply haven't updated the automation tests (we only handled the unit tests).

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 816327 [details]
redirect to PR 12805.html

I just tried running the updated PR against the gaia_ui_test that was failing using these steps:

  cd gaia
  npm install
  ./node_modules/.bin/travisaction gaia_ui_tests install
  export GAIATEST_ACKNOWLEDGED_RISKS=true
  export GAIATEST_SKIP_WARNING=true
  gaiatest --app=b2gdesktop --binary=b2g/b2g-bin --profile=profile --restart tests/python/gaia-ui-tests/gaiatest/tests/manifest.ini /srv/gaia-master/tests/python/gaia-ui-tests/gaiatest/tests/functional/everythingme/test_everythingme_launch_packaged_app.py

And I still get the error.

Are you saying that the gaiatest needs to be changed to account for differences in the app?

Comment 11

[Amir Nissim (:amirn)]

(In reply to Ben Kelly [:bkelly] from comment #10)
> Comment on attachment 816327 [details]
> redirect to PR 12805.html
> 
> I just tried running the updated PR against the gaia_ui_test that was
> failing using these steps:
> 
>   cd gaia
>   npm install
>   ./node_modules/.bin/travisaction gaia_ui_tests install
>   export GAIATEST_ACKNOWLEDGED_RISKS=true
>   export GAIATEST_SKIP_WARNING=true
>   gaiatest --app=b2gdesktop --binary=b2g/b2g-bin --profile=profile --restart
> tests/python/gaia-ui-tests/gaiatest/tests/manifest.ini
> /srv/gaia-master/tests/python/gaia-ui-tests/gaiatest/tests/functional/
> everythingme/test_everythingme_launch_packaged_app.py
> 
> And I still get the error.
> 
> Are you saying that the gaiatest needs to be changed to account for
> differences in the app?

That's weird. I am running the test on machine according to your instructiond and it passes.
Travis is all green for this PR as well https://travis-ci.org/mozilla-b2g/gaia/builds/12481928

Yes, we had to change the test to accommodate for changes in the code:
https://github.com/mozilla-b2g/gaia/pull/12805/files#diff-4
https://github.com/mozilla-b2g/gaia/pull/12805/files#diff-5

Am I missing something?

Comment 12

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 816327 [details]
redirect to PR 12805.html

(In reply to Amir Nissim (Everything.me) from comment #11)
> Am I missing something?

I guess it would help if I actually tested your new code.  I copied the commands I was running previously without adjusting the path to where I had your PR checked out.  Oops!

With the correct paths I got 11 consecutive successes, so f+ from me.

Thanks! Sorry for my confusion.

Comment 13

[Amir Nissim (:amirn)]

thanks Ben.

landed in master
https://github.com/mozilla-b2g/gaia/commit/25ad6d49773ca738bf0fb50dfb3709e96811be81