Closed
Bug 1023819
Opened 10 years ago
Closed 10 years ago
[Follow up] Get "Update search app to align with new homescreen" to pass UX review for FL
Categories
(Firefox OS Graveyard :: Gaia::Search, defect)
Tracking
(feature-b2g:2.0, b2g-v2.0 fixed, b2g-v2.1 fixed)
People
(Reporter: epang, Assigned: daleharvey)
References
Details
(Whiteboard: [systemsfe])
User Story
Interaction Spec: https://mozilla.app.box.com/files/0/f/1955330942 Visual Specs https://mozilla.box.com/s/ho1brfbe9f4sw5uv3ga4 Image assets (clear icon): https://mozilla.box.com/s/2h15e0xrtr4uluyj5w2q
Attachments
(2 files)
46 bytes,
text/x-github-pull-request
|
kgrandon
:
review+
epang
:
ui-review-
djabber
:
feedback+
bajaj
:
approval-gaia-v2.0+
|
Details | Review |
717.23 KB,
application/zip
|
Details |
This is a follow up to bug 1009401 containing issues with the current implementation of the search app Visual 1. RB search labels should have 3 lines of text (and larger vertical spacing between icons - extra 20px as seen in spec). 2. Search Result icons are a little small compared to the homescreen (they should be 64x64px) 3. Search Result icons are blurry 4. Go button is missing in RB 5. Search suggestions colour should be #00aacc (text) and #666666 (bullets). 6. Entering RB Search motion - Based on comment 3 is it the animation that isn’t running good? Can you let me know, cause if that’s the case we can try something else out (maybe something similar to entering edit mode on homescreen - cause that seems to work really smoothly) 7. some search results icons seem to be missing Interaction: 1. The spec states that if the user taps the close button, the search field should be cleared and the user should be returned to the home screen. The next time the user accesses the search app, it should be in the empty state (as the search field was cleared). In the build, however, the previous text remains and is shown highlighted. 2. Conversely, if the user opens the search app and the text field has not been cleared, it should show the previous text AND results. The build, however, only shown the text highlighted, but no results. According to the spec, the text should not be highlighted and the search results should be shown. I've attached Please let us know if you have any questions, thanks Dale!
Reporter | ||
Comment 1•10 years ago
|
||
I've also attached the specs to the user story. THanks!
Reporter | ||
Comment 2•10 years ago
|
||
Please flag Francis and I for Ui-Review when ready, thanks!
Comment 3•10 years ago
|
||
I'm updating the title here to indicate that this is bug is tracking the need to get the search app to pass UX review.
Summary: [Follow up] Update search app to align with new homescreen → [Follow up] Get "Update search app to align with new homescreen" to pass UX review for FL
Comment 4•10 years ago
|
||
Stephany - Can you add the feature-b2g 2.0 flag here? I want to make sure we've got a bug flagged here tracking the need to get UX review to pass post landing.
Flags: needinfo?(swilkes)
Updated•10 years ago
|
QA Whiteboard: [VH-FL-blocking+][VH-FC-blocking+]
Updated•10 years ago
|
feature-b2g: --- → 2.0
Flags: needinfo?(swilkes)
Updated•10 years ago
|
Whiteboard: [systemsfe]
Updated•10 years ago
|
Target Milestone: --- → 2.0 S4 (20june)
Updated•10 years ago
|
blocking-b2g: --- → 2.0?
Comment 5•10 years ago
|
||
Marked as 2.0? blocker as it's a feature for 2.0.
Updated•10 years ago
|
blocking-b2g: 2.0? → ---
Comment 6•10 years ago
|
||
In response to bug #1021418, the Go button has been replaced with a clear button to follow established patterns. This has had some impact on the IxD spec: > > 4. Go button is missing in RB > This should be replaced with "Clear button is missing in RB" > > Interaction: > 1. The spec states that if the user taps the close button, the search field > should be cleared and the user should be returned to the home screen. The > next time the user accesses the search app, it should be in the empty state > (as the search field was cleared). In the build, however, the previous text > remains and is shown highlighted. The close button now no longer clears the search field - it just closes the app but maintains its state.
Reporter | ||
Comment 7•10 years ago
|
||
(In reply to Francis Djabri [:djabber] from comment #6) > In response to bug #1021418, the Go button has been replaced with a clear > button to follow established patterns. This has had some impact on the IxD > spec: > > > > > 4. Go button is missing in RB > > > This should be replaced with "Clear button is missing in RB" > > > > > Interaction: > > 1. The spec states that if the user taps the close button, the search field > > should be cleared and the user should be returned to the home screen. The > > next time the user accesses the search app, it should be in the empty state > > (as the search field was cleared). In the build, however, the previous text > > remains and is shown highlighted. > > The close button now no longer clears the search field - it just closes the > app but maintains its state. I've update the specs on box, I've also added the clear icon here: https://mozilla.box.com/s/2h15e0xrtr4uluyj5w2q Dale, do you think we can add the clear button is this bug? If so we can probably close bug 1021418.
User Story: (updated)
Flags: needinfo?(dale)
Assignee | ||
Comment 8•10 years ago
|
||
Ill figure out when implementing them if they are worth being seperate patches or not and if so just close this out
Flags: needinfo?(dale)
Assignee | ||
Comment 9•10 years ago
|
||
Ok, I will likely split this into a few bugs, implemented the fixes to the input behaviour + persistence and the clear button but not the grid changes open questions 1. Is there a spec with the go button, clear button and close button, I expect we dont want all 3 shown at the same time? 2. Why does the grid have extra spacing for 3 labels? we use the same rendering code for the homescreen and the search app, the behaviour will be the same
Flags: needinfo?(epang)
Reporter | ||
Comment 10•10 years ago
|
||
(In reply to Dale Harvey (:daleharvey) from comment #9) > Ok, I will likely split this into a few bugs, implemented the fixes to the > input behaviour + persistence and the clear button but not the grid changes > > open questions > > 1. Is there a spec with the go button, clear button and close button, I > expect we dont want all 3 shown at the same time? > > 2. Why does the grid have extra spacing for 3 labels? we use the same > rendering code for the homescreen and the search app, the behaviour will be > the same Hi Dale, no prob about splitting into a few bugs - just let us know the bug numbers when ready. Regarding your questions: 1. The clear button takes the place of the go button. We're removed the go button - the user can 'go' by using the keyboard. So only the close and clear buttons are now shown. 2. For the spacing, we wanted to follow what the current version of search is doing. Many of the e.me search labels tend to be longer so it makes sense to have extra vertical space for them especially for different localizations. Is this really difficult? Thanks! Eric
Flags: needinfo?(epang) → needinfo?(dale)
Assignee | ||
Comment 11•10 years ago
|
||
Split into seperate bugs: https://bugzilla.mozilla.org/show_bug.cgi?id=1026157 - The open animation was problematic first time I implemented it, it didnt perform great, we cant use css3 transforms since its the height that needs animated https://bugzilla.mozilla.org/show_bug.cgi?id=1026159 - The grid UX changes, including the 3 lines for labels The clear button, changes in how the search terms persist when pressing the close button and suggestions colour changes are implemented in this bug
Flags: needinfo?(dale)
Assignee | ||
Comment 12•10 years ago
|
||
Implements the clear button, fixes the persistence of the search term and minor visual fixes
Attachment #8440948 -
Flags: ui-review?(epang)
Attachment #8440948 -
Flags: review?(kgrandon)
Attachment #8440948 -
Flags: feedback?(fdjabri)
Comment 13•10 years ago
|
||
Comment on attachment 8440948 [details] [review] https://github.com/mozilla-b2g/gaia/pull/20584 Code looks good to me, thanks!
Attachment #8440948 -
Flags: review?(kgrandon) → review+
Comment 14•10 years ago
|
||
Thanks Dale. This looks good to me but there's one interaction that still doesn't match the spec - namely, if the user selects one of the e.me search suggestions, the search field should update with the selected term and the keyboard should remain on display. Right now in the latest patch, the keyboard is dismissed if the user taps a search suggestion I've raised Bug #1026716 so we can close this bug off.
Updated•10 years ago
|
Attachment #8440948 -
Flags: feedback?(fdjabri) → feedback+
Comment 15•10 years ago
|
||
Assignee | ||
Comment 16•10 years ago
|
||
Thanks Francis, thats a seperate bug so will take that one and landed there changes as is, itd easier to follow up bugs like there https://github.com/mozilla-b2g/gaia/commit/e4c440ba3996988fb0b749c7034d7990e19601e4
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 17•10 years ago
|
||
Comment on attachment 8440948 [details] [review] https://github.com/mozilla-b2g/gaia/pull/20584 Hey Dale, Most things look good, but there's a few problems I'm still seeing When I flash with: DEVICE_DEBUG=1 GAIA_DEV_PIXELS_PER_PX=1.5 make reset-gaia I'm seeing a clear button that is scaled too large See attached screen http://cl.ly/image/1e3v0r2O1Z1o Also, is anything we can do with the lower quality icons? Or is this an search provider problem, if it is and we can't do anything then it's fine. http://cl.ly/image/24372m2A0y05 Sorry for the delay in review I was having problems flashing. Since this has already landed will need bugs need to be opened for the icons and potentially lower quality icons? Thanks!
Attachment #8440948 -
Flags: ui-review?(epang) → ui-review-
Flags: needinfo?(dale)
Comment 19•10 years ago
|
||
Comment on attachment 8440948 [details] [review] https://github.com/mozilla-b2g/gaia/pull/20584 This is needed for the vertical homescreen. We've done our best effort at testing this and believe it is safe for uplift.
Attachment #8440948 -
Flags: approval-gaia-v2.0?(bbajaj)
Assignee | ||
Comment 20•10 years ago
|
||
Eric, the icons come from the provider, I not sure what we can do about it but definitely doesnt come under this bug, thanks for catching the icon size, I posted a follow up to make sure that works https://github.com/mozilla-b2g/gaia/commit/8a239771e739f113241f20aa9566387adc824e0d
Flags: needinfo?(dale)
Reporter | ||
Comment 21•10 years ago
|
||
(In reply to Dale Harvey (:daleharvey) from comment #20) > Eric, the icons come from the provider, I not sure what we can do about it > but definitely doesnt come under this bug, thanks for catching the icon > size, I posted a follow up to make sure that works > > https://github.com/mozilla-b2g/gaia/commit/ > 8a239771e739f113241f20aa9566387adc824e0d np, thanks Dale :)
Updated•10 years ago
|
Attachment #8440948 -
Flags: approval-gaia-v2.0?(bbajaj) → approval-gaia-v2.0+
Comment 23•10 years ago
|
||
2.0: https://github.com/mozilla-b2g/gaia/commit/ff88c60655d123b76ddc30894ab94e3126473062 https://github.com/mozilla-b2g/gaia/commit/c45f2a787e81828f8181e897d8e9a4970b2c9581
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
Updated•10 years ago
|
Flags: in-moztrap-
Comment 24•10 years ago
|
||
Landed a follow-up to fix the className to follow our platform standards, should've caught it in the review: https://github.com/mozilla-b2g/gaia/commit/e485e031b249bcb135d78fa49b9a4d3ef160e801 Also going to see if we can run this through our CSSLinter.
Assignee | ||
Comment 25•10 years ago
|
||
That last follow up broke the integration tests (and the functionality, yay working integration tests) filed https://bugzilla.mozilla.org/show_bug.cgi?id=1041319 and landed the fix to get the tree green
You need to log in
before you can comment on or make changes to this bug.
Description
•