Update gallery view styling/layout

[jkeck]

Closes #2312

Chrome (before)

Chrome (after)

---

Firefox (before)

Firefox (after)

Note: Firefox does not support display: box yet, so the text is truncated w/o an ellipsis

---

Safari (before)

Safari (after)

[codecov-io]

Codecov Report

Merging #2359 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2359   +/-   ##
=======================================
  Coverage   95.92%   95.92%           
=======================================
  Files          98       98           
  Lines        1275     1275           
=======================================
  Hits         1223     1223           
  Misses         52       52

Impacted Files	Coverage Δ
src/components/GalleryView.js	85.71% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a00105...2529b95. Read the comment docs.

[ggeisler]

Nice. I know the PR is still draft and this wasn't included in #2312 but I wonder if we should reduce the line-height of the captions?

@jvine What do you think? The current line-height matches the Caption specs in #2093 but I think in this context using a headline line-height (e.g., 1.33em) might work better? It would help tighten up the whole layout while still be very readable, I think.

[jkeck]

Yeah, in an earlier pairing session the line-height of the captions was called out as needing some tweaking. Happy to change it (and the related heights) as needed.

[jkeck]

I set the line-height for the caption to be 1.33rem and the height of the caption area to be a fixed height of 3rem, but it looks like the descenders are getting clipped a bit.

Also, the original ticket says something about adding a word break, but wasn't sure exactly what we wanted there. Is there a specific word break strategy that you all have in mind?

[ggeisler]

Actually my suggestion was 1.33em not rem, which does seem to make a difference (slightly more compact line height).

[jkeck]

Okay, so should the fixed height be 2rem then? (line-height: 1.33em and height: 2rem)?

Forgive my ignorance 😬

[ggeisler]

That seems to work, so I think so...

[ggeisler]

Also @jkeck, about this question:

Also, the original ticket says something about adding a word break, but wasn't sure exactly what we wanted there. Is there a specific word break strategy that you all have in mind?

I think @aeschylus wrote that and I'm not sure what he had in mind specifically, but I'm not sure we need to specify anything. The default seems to be what we want. The main alternative would be to add word-break: break-all; but I don't think that produces the result we want (does allow more letters per line in some cases, but at the cost of weird word breaks).

[jkeck]

Agreed, I played w/ some of the various options, and none of them seemed to be better than the default.

[jkeck]

Okay @ggeisler / @jvine, I pushed it up. Let me know if that looks good and I'll make it ready for review.

[ggeisler]

Hmm, the captions look good but now the rows seem to have way too much whitespace between them. The whitespace increases as the browser width increases (or hits different breakpoints, I think):

Seems like the row height should be static across breakpoints, since the image height and caption height is fixed?

[jkeck]

Okay, so what height would you like me to set then? The height stays fixed for me as I resize my screen, so I'm not able to really reproduce this to know what to change.

[ggeisler]

Wait, I just adjusted it again to play with the height and it snapped back to how it was before, where the height is static and doesn't change with the breakpoints. So I guess what I was looking at was not accurate. So I don't think any change is needed, sorry.

[jvine]

1. We need word-break: break-word to handle long words - otherwise they will break the spacing. First image has word-break: break-word, second does not:
   

2. As for the line-height and overall height...
   a) I feel like 1.33em is too close for readability/accessibility for such small text. The guideline would be 1.5em, which is just slightly roomier without being loose.
   b) I think it's potentially brittle if we use different measures for the line height (em) and overall height (rem) -- so if we use 1.5em, the height should be 3em.

Before (1.33em and 2rem)

After (1.5em and 3em)

It's a tiny difference but it's still a bit closer than 1.25rem and it's officially compliant.

[ggeisler]

I guess it's important to test on more than one fixture! I used NGA and didn't see any difference with word-break: break-word so thought that was the default but with the other object it's clear that it does make a difference...