Closed Bug 924764 Opened 6 years ago Closed 6 years ago
Collection design needs updating
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.
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 on attachment 814817 [details] Patch - redirect to github PR Excellent work!
Attachment #814817 - Flags: review?(crdlc) → review+
Attachment #814817 - Flags: review?(amirn)
merged in master e45fa382af8341f7db70a8d5bf7dff7f5067a87d https://github.com/mozilla-b2g/gaia/commit/e45fa382af8341f7db70a8d5bf7dff7f5067a87d
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
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?
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+
thanks Ben. landed in master https://github.com/mozilla-b2g/gaia/commit/25ad6d49773ca738bf0fb50dfb3709e96811be81
Status: REOPENED → RESOLVED
Closed: 6 years ago → 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.