Closed Bug 924764 Opened 6 years ago Closed 6 years ago

Collection design needs updating

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: evyatar, Assigned: evyatar)

References

Details

Attachments

(2 files)

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.
Status: NEW → ASSIGNED
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)
Attachment #814817 - Flags: review?(crdlc)
Comment on attachment 814817 [details]
Patch - redirect to github PR

Excellent work!
Attachment #814817 - Flags: review?(crdlc) → review+
Attachment #814817 - Flags: review?(amirn)
Attachment #814817 - Flags: review?(amirn) → review+
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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
Attachment #816327 - Flags: review?(crdlc)
Attachment #816327 - Flags: review?(bkelly)
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
Attachment #816327 - Flags: review?(crdlc) → review?(evyatar)
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).
Attachment #816327 - Flags: review?(evyatar) → review+
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?
Attachment #816327 - Flags: review?(bkelly) → feedback-
(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?
Flags: needinfo?(bkelly)
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.
Attachment #816327 - Flags: feedback- → feedback+
Flags: needinfo?(bkelly)
thanks Ben.

landed in master
https://github.com/mozilla-b2g/gaia/commit/25ad6d49773ca738bf0fb50dfb3709e96811be81
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Duplicate of this bug: 836336
Duplicate of this bug: 923491
You need to log in before you can comment on or make changes to this bug.