1026159 - [Rocketbar] Grid UX fixes for search app

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Search, defect)

Comment 1

[Francis Djabri [:djabber]]

This bug needs to be fixed because currently we're only seeing 1 line of text for search results. Previously we had three lines of text. These longer labels are needed for the search app because third party app results and e.me results tend to be longer. Market place results are pre-faced by "Install" and so the Market place results are pretty unusable right now. 

I spoke with Kevin and he mentioned that the reason why this hasn't been done is that we need the same rendering on Home screen as in the search app. I think we can compromise to some degree, but we need at least 2 lines showing in the search app. 

Peter, this might mean that we need 2 lines of text showing in the homescreen if we absolutely must have the same rendering in both the home scree and the search app. Is this something that we can live with?

Comment 2

[Kevin Grandon :kgrandon]

I guess from engineering we would be looking for a spec for both 3 and 4 column layouts with two lines of text.

I also wanted to check, and it turns out that the marketplace manifest name field can be up to 128 characters in length. Maybe the user is already familiar enough with installed apps though that they don't need the full title. Though this has been a bit frustrating to me in the past when I book two things, and the title isn't long enough to differentiante, E.g.:

Metro Maps - Stop 1 to Stop 2
Metro Maps - Stop 2 to Stop 1

It can be frustrating if both cut off at 'Metro Maps - Stop...', two lines might help that a bit.

Comment 3

[Francis Djabri [:djabber]]

One compromise I’d suggest is that we show 1-line text labels for the 3-column layout and 2-line text labels for the 4-column layout. It seems that with the current spacing, there should be enough space to show 2-line text labels for the 4-column layout. What do you think, Peter?

Comment 4

[Lawrence Mandel [:lmandel] (use needinfo)]

Although the current state is not great, it is not marked by UX as needed and the functionality is not blocked. At this point in the schedule this does not block.

Comment 5

[Kevin Grandon :kgrandon]

There is good information here, but we have a more specific bug filed with screenshots in bug 1036845.

Let's go ahead and use this bug as it's more specific (and a potential contributor might work on it).

Comment 6

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Pushing to backlog instead of -
This might be a user story for 2.1?

Comment 7

[Rob MacDonald [:robmac]]

Renoming - We need this fix to land for 2.0 because otherwise the search experience and app discovery experience from Market place using the search app will be unusable in many cases. It is also a regression that resulted from the new home screen - in 1.4, the search app provided up to 3 lines of text.

Comment 8

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

To note, this issue affects anything that uses the grid.  Collections + homescreen.  It's affecting the search more because of the "Install <app name>..."

Comment 9

[Sam Foster [:sfoster] (he/him)]

Attachment: Enable 2-line labels in the search results grid

Make global adjustments to grid item height (+4px)
Add override rules for the search grid items to allow line-wrapping and set height/line-height to clip after the 2nd line. As discussed with UX, removed text-shadow in this case where it is less necessary and would otherwise also get clipped. 
We could invert this if you prefer to make 2-lines the default for the grid items and special case homescreen to 1 line, but this seemed like the smaller change. 
Note I needed to use !important to override the component's stylesheet rules

Comment 10

[Sam Foster [:sfoster] (he/him)]

Attachment: Screenshot: search results with 2 line labels

Here's how it works out. This overlays the spec pretty much pixel perfect for me. Note the text shadows are gone and and > 3 line labels are neatly clipped at 2 lines.

Comment 11

[Sam Foster [:sfoster] (he/him)]

Attachment: Screenshot: Homescreen with patch applied

The way the patch is done, the *only* change that should affect the homescreen grid is the increased item height. I don't think the spec had this change yet for the 3 column home?

Comment 12

[Sam Foster [:sfoster] (he/him)]

Attachment: Screenshot: 4-col homescreen with patch applied

The same deal for 4 column homescreen. Again, the only change here should be the increased height (+4px), which just shows up as increased space below the label and between each icon row.

Comment 13

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8458370 [details]
Screenshot: Homescreen with patch applied

er, ui-review, not review :)

Comment 14

[Peter La]

Comment on attachment 8458369 [details]
Screenshot: search results with 2 line labels

Hi Sam,

Thanks so much for this.

Did you see my updated spec?  After working it out with Eric, I sent an email ~5:30pm EST with the update.  I tightened up the spacing further so it is actually 10.2 rem row height instead of 10.4 rem row height that you've implemented here.  The reason for tightening the spacing is so that the Homescreen 4 column grid doesn't look so sparse while still allowing enough room for the 2 line search results.

This works out to a 102px row height on 320x480 screens, and 153px row height on Flame.

Here is a link to the final spec:
https://mozilla.box.com/s/ifl3nxduco6f0pc9a6yt

I will also attach it to this bug.

Comment 15

[Peter La]

Attachment: Homescreen_FWVGAUpdatedLayout_July17.jpg

Final layout with 10.2 rem row spacing.

Comment 16

[Peter La]

Comment on attachment 8458370 [details]
Screenshot: Homescreen with patch applied

Hey Sam,

No changes are required for the 3 column, since search results don't use the 3 column layout.  I would not submit this change.

Comment 17

[Peter La]

Needinfo'ing Kevin in case Sam is out tomorrow due to travel.

Comment 18

[Peter La]

Comment on attachment 8458371 [details]
Screenshot: 4-col homescreen with patch applied

Minusing for the same reason as the other 4 column ui-review.  See comment 14 for details.

Comment 19

[Kevin Grandon :kgrandon]

Comment on attachment 8458366 [details] [review]
Enable 2-line labels in the search results grid

Thanks for the patch. Going to clear review for now till we get the ui-review sorted out. I may end up picking this up today to see if we can get it in.

Comment 21

[Sam Foster [:sfoster] (he/him)]

Ok I see the 2px. I'll try and update and test the patch on my flight. Basically though it looks like: 

  const distanceBetweenIconsWithMinIconsPerRow = 36;
  const distanceBetweenIconsWithMaxIconsPerRow = 38;

and I think in search.css, I'll end up with: 
#search-results .icon .title {
  text-shadow: none!important;
  height: 3.88rem;
  white-space: normal!important;
  line-height: 1.94rem;
}

I may not be able to resubmit this until Monday, so Kevin if you have time for this before then be my guest. Otherwise I'll pick it up when I'm back.

Comment 22

[Kevin Grandon :kgrandon]

Ok, thanks a lot Sam. If I can get to it or provide a patch to you I will, otherwise I think we can try on Monday.

Comment 23

[Sam Foster [:sfoster] (he/him)]

Attachment: Screenshot: search results with 2 line labels

Comment 24

[Sam Foster [:sfoster] (he/him)]

Attachment: Screenshot: 3 col homescreen with patch applied

Comment 25

[Sam Foster [:sfoster] (he/him)]

Attachment: Screenshot: 4-col homescreen with patch applied

Comment 26

[Peter La]

Comment on attachment 8459705 [details]
Screenshot: search results with 2 line labels

Grid spacing is perfect between icons and the app label spacing is also perfect - no drift when overlayed on the spec.

However, all the content below the search input field is a bit too low.  I'll attach an image that outlines this in more detail shortly.

Comment 27

[Peter La]

Comment on attachment 8459707 [details]
Screenshot: 3 col homescreen with patch applied

Icons appear to be larger than before and this is causing the rest of the layout to be off a little.  Will attach a jpg showing the discrepancies.

Comment 28

[Peter La]

Comment on attachment 8459708 [details]
Screenshot: 4-col homescreen with patch applied

Almost there, but some slight differences.  Will upload a jpg to outline discrepancies.

Comment 29

[Peter La]

Attachment: Search Results discrepancies

Comment 30

[Peter La]

Attachment: 3 Column Grid discrepancies

I'm not going to try to describe every problem here as I think it would be difficult and too confusing anyway.  The first step is just to make sure the icons aren't larger than they used to be.  Right now they look to be about 129-130px when scaled up to Flame res - they should be at 126px.

Comment 31

[Peter La]

Attachment: 4 Column Grid discrepancies

Comment 32

[Peter La]

Attachment: search_results_icon_grid_320wide.png

320 wide version of search results spec for easy comparison

Comment 33

[Peter La]

Attachment: Homescreen_FWVGAUpdatedLayout_July17_320wide.png

320 wide version of homescreen grid spec for easy comparison

Comment 34

[Sam Foster [:sfoster] (he/him)]

Attachment: Screenshot: search results with 2 line labels

Thanks for the 320w spec, that helped a lot to get this lined up.

Comment 35

[Sam Foster [:sfoster] (he/him)]

Attachment: Screenshot: 3 col homescreen with patch applied

Comment 36

[Sam Foster [:sfoster] (he/him)]

Attachment: Screenshot: 4-col homescreen with patch applied

Comment 37

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8459804 [details]
Screenshot: 3 col homescreen with patch applied

Btw the icon size I think was just an artifact of me taking the screenshot while i was mid-transition. I have this lined up on top of the spec and it looks bang-on to me now.

Comment 38

[Kevin Grandon :kgrandon]

Comment on attachment 8458366 [details] [review]
Enable 2-line labels in the search results grid

Here's a pre-emptive R+ once the ui-review passes. The !important declarations really make me sad though =( Is there no way to avoid them?

Comment 39

[Sam Foster [:sfoster] (he/him)]

Kevin, I updated the PR which hadn't changed much while Peter and I went back and forth on the ui review. Other than the little spacing tweaks in the icon titles, the main changes are adjusting spacing between the listings and search box - which touches both the verticalhome and search apps. The !importants are sadly necessary: the grid component uses a scoped stylesheet whose rules we can't override with selector specificity - we have to bring out the big guns.

Comment 40

[Peter La]

Comment on attachment 8459802 [details]
Screenshot: search results with 2 line labels

Hi Sam,

The grid part is perfection.  The search results are still 0.1rem lower than in Eric's spec, but after speaking with Kevin, we can deal with this in a separate bug.  Going to ui-review+.

Great job!

Comment 41

[Peter La]

Comment on attachment 8459806 [details]
Screenshot: 4-col homescreen with patch applied

Looks good to me.

Comment 42

[Peter La]

Comment on attachment 8459804 [details]
Screenshot: 3 col homescreen with patch applied

The app labels are now 0.2 rem too high.  Everything else lines up perfectly (icon position, row height, etc.).

So close, but I'm gonna minus.

Comment 43

[Sam Foster [:sfoster] (he/him)]

Attachment: Screenshot: 3 col homescreen with patch applied

Working blind here as this now looks 2px/0.2rem too low to me, but should be the adjustment you need.

Comment 44

[Sam Foster [:sfoster] (he/him)]

Attachment: Screenshot: 3 col homescreen with patch applied

Comment 45

[Peter La]

Comment on attachment 8461260 [details]
Screenshot: 3 col homescreen with patch applied

Perfect!

Thanks so much Sam. :)

Comment 46

[Sam Foster [:sfoster] (he/him)]

Landed for 2.1: https://github.com/mozilla-b2g/gaia/commit/2765120564bcaa11721fe72ebbf08126a1e73aff

Comment 47

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

https://moztrap.mozilla.org/manage/case/14056/ created

Waiting for this to land in 2.0

Gaia      c72257b2d27135bfcd68e89dd584182797784016
Gecko     https://hg.mozilla.org/mozilla-central/rev/06ac51c2b8a8
BuildID   20140724040205
Version   34.0a1
ro.build.version.incremental=110
ro.build.date=Fri Jun 27 15:57:58 CST 2014
B1TC00011230

Comment 48

[Sam Foster [:sfoster] (he/him)]

Landed on v2.0: https://github.com/mozilla-b2g/gaia/commit/dd72f1310c28f18d9f75a121b6f4fd0fdacbf9d5