qmd-lab / closeread

https://closeread.dev
MIT License
130 stars 5 forks source link

Grid-based mobile layout #24

Closed jimjam-slam closed 3 months ago

jimjam-slam commented 7 months ago

As of commit dc1e1fb I've had a crack at a grid-based mobile layout. This first version is pretty naive: it changes the grid from 1fr 2fr (or whatever the proportions are) to a single-track 1fr column, and it positions both the narrative and sticky content on that one column. Then it sets z-index: 1 on the narrative content to ensure it's on top.

In practice, I imagine we'd have a slightly more complex (and perhaps more configurable) grid that includes gutters and maybe a few subdivisions so that you can still align overlaid narrative content to the left or right. Or perhaps the latter makes sense for 'tablet' sizing, with mobile going right down the middle.

One problem I'm seeing with this first test is that our figures don't seem to be shrinking down from their native width:

image

I've added a test down the bottom of the doc to compare against. Normally .quarto-figure > figure sets width: 100% and the img itself has max-width: 100%. But for some reason this isn't working now.

I was worried that maybe the poem-related and that this JS was forcing something to be wide. But it doesn't seem to be poem-related (I still see the problem if I remove those from the test). And when I click up through the DOM from the offending img, only it and the figure have a width set on them (all the others are greyed out, indicating they're set based on parent or children). So I'm a bit stumped.

jimjam-slam commented 7 months ago

Found the culprit here! It's the GraphViz diagram. It isn't wrapped in .quarto-figure and specifically has max-width: none set on it, so it stays at 672px wide, forcing the whole column out.

Have isolated and reproduced this issue at https://github.com/jimjam-slam/test-quarto-graphviz-sizing and filed it as https://github.com/quarto-dev/quarto-cli/issues/9249 😊 If it is by design, we can override it by specifying width: 100% !important on the figure if need be, but let's see if the Quarto folks change anything.

jimjam-slam commented 7 months ago

Ahh, I was wrong! I've added the workaround I described for GraphViz sizing in commit 80da14f. It works when the poems are removed, but when the poems are present they're pushing the width out. I need to put some JS debugging statements back in... I think the same problem I had vertically before with poems is now happening horizontally, where it reserves layout based on its pre-scaled width 😕

jimjam-slam commented 7 months ago

Adding a max-width to the poem mitigates but doesn't entirely fix the problem. There's still a bit too much room, and the horizontal positioning looks like it's happening based on the pre-overflow width rather than the actual width—I'll need to dip back into the JS to check that.

Also, although I've set max-width: 100svh, it should really only be that if the container is 100svh (as it currently is on mobile). On desktop, it would reflect the proportions of the grid (which could be complicated if we allow the user to configure that). Maybe a CSS variable is in order to control both—or using JS to manually calculate it. Yuck.

jimjam-slam commented 6 months ago

@andrewpbray Here's what I'm thinking in terms of grid-based layout configs.

It's really five basic layouts, with only one being possible at mobile width but up to all five available at desktop width.

sketch grid layouts

The mechanism I'm thinking is that you either define the basic layout via classes or via YAML:

---
closeread:
  layout-tablet: overlay-right
  layout-desktop: sidebar-right
  # (no layout-mobile options)
---
:::{.cr-section .cr-tablet-overlayright}
::: 

We could define the grid layouts for those classes in SCSS (and perhaps attach those classes in Lua to ones that have no other layout using the YAML options).

There are two ways you could do this:

  1. Write grid tracks for all the possible layouts in one system, and then simply set the properties defining which tracks are actually used according to the class.
  2. Write separate grid tracks for each class. Only the grid tracks that are actually used for any given section are available, and users can override the class directly if they want to customise (we could make the precise widths CSS or SCSS variables). I think this is my preference, but I'm happy to kick it around!

What do you think?

jimjam-slam commented 6 months ago

In the mean time, I still haven't fixed this horizontal space problem — I'm so stuck that I even tried rendering the poem as an SVG (which is its own can of worms and also nukes accessibility).

Even more frustrating is that I can't reproduce the original scaling problem in Safari with a simpler test case—the text looks absolutely fine blown up in a simple document:

image

I might need to walk away from this one and have another swing in a few days!

jimjam-slam commented 6 months ago

A quick video to demo the inconsistency of the scaling problem in Safari (it doesn't look too bad because I've shrunk the video down for GitHub's attachment sizes, but at actual screen size it's a lot worse):

https://github.com/qmd-lab/closeread/assets/6520659/8e959a40-c3f3-41fe-b7d1-0266bb913bd4

jimjam-slam commented 6 months ago

I've pushed one more change that uses a variable poem font size (based on screen width and height). It still breaks at some unusual landscape aspect ratios, but at more common ones it behaves much more (and lays out properly more often even when scrollbars do pop in).

I'd love to get to the bottom of the Safari bug and ditch a lot of this extra complexity, but I'm also mindful of time 🤔

andrewpbray commented 6 months ago

Some great ideas and puzzling bugs here - looking forward to chatting!

jimjam-slam commented 4 months ago

Okay, returning to the original intent of this PR: letting users specify a layout!

@andrewpbray Here's what I'm thinking in terms of grid-based layout configs.

It's really five basic layouts, with only one being possible at mobile width but up to all five available at desktop width.

sketch grid layouts

The mechanism I'm thinking is that you either define the basic layout via classes or via YAML:

---
closeread:
  layout-tablet: overlay-right
  layout-desktop: sidebar-right
  # (no layout-mobile options)
---

I think the Lua function needs to place the document YAML options as classes on body, and then attributes or classes used on cr-section flow through to them on the HTML. So the CSS would be something like:

// this is less specific (0-2-1), so it applies by default...
body.cr-overlay-right-desktop .cr-layout {
  // layout here
}

// this is more specific (0-3-1), so it overrides any body option
body.closeread .cr-layout.cr-overlay-right-desktop {
  // layout here
}

(CSS specificity calculator is useful for confirming this ahead of time!)

I think making the section-specific styles more specific requires an additional class on the body (here it's .closeread that isn't used for the body defaults.

Since centre overlay is the only one supported by mobile, I would make that the default and then have the other options function on breakpoints going up from there (so you're only ever specifying layout or desktop layout).

Bootstrap breakpoints also function going up from a minimum width by default too, so that works (we could also use those breakpoints directly — ie. instead of -tablet and -desktop, it's -sm, -md, -lg, xl, xxl). So you might end up with something like:

@include media-breakpoint-up(lg) {
  .cr-overlay-right-lg {
    // overlay right code goes here
  }
}

// next breakpoint up goes here, so
// if you provide tablet and desktop ones,
// the desktop one overrides the tablet one at that width (i think)
jimjam-slam commented 4 months ago

Okay, the above isn't entirely working — for the overlay classes, I don't know how to calculate the space between narrative edge and sticky edge.

Maybe we just use separate grid tracks for the sidebar layouts, and all the overlay ones work as the mobile layout (overlay-center) and the sticky stack do: overlaid on one grid column, and then we margin/pad them to get the right effect.

So then for overlay layouts it becomes:

While for sidebar layouts it becomes:

This is basically how the sidebar works currently!

jimjam-slam commented 4 months ago

I also just found out that you're supposed to set container-type on the parent when using container units and queries! That might help! 😅

andrewpbray commented 4 months ago

Taking a look at this now... will have feedback in a couple hours...

:snail:

jimjam-slam commented 4 months ago

It could be that container-type is only used for container queries, not container units. I need to do a bit of reading on it myself!

andrewpbray commented 4 months ago

While tinkering around with styling, I realized nytimes keeps their sidebar at a fixed width. Try widening and narrowing the window here:

https://www.nytimes.com/interactive/2022/03/06/books/auden-musee-des-beaux-arts.html

jimjam-slam commented 3 months ago

Closing in favour of #55! I'll move your comment over @andrewpbray 😊