qmd-lab / closeread

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

Overlay and sidebar layout options #55

Closed jimjam-slam closed 3 months ago

jimjam-slam commented 3 months ago

Closing #24 in favour of this (I also cherry-picked the commits from #52 and #53, as they generalise the ` injection code. Shouldn't matter which is merged first!). Adds two options:

format:
  closeread-html:
    overlay-side: # "left", "center or "right"
    sidebar-side: # "left" or "right"

The Lua filter throws an error if both are used on the same doc. These options set a class on <body>.

These classes can be be inherited on individual .cr-layout using helper classes:

What do you think, @andrewpbray? I haven't touched colours here, just layout! I also didn't try to do tablet layouts: the layout locks for mobile devices (< 576 px, equivalent to Bootstrap's XS class), but anything wider than that uses the same layout. Just wanted to make sure I got this done before I took off! 😅

Also note I've hard coded the margins for now, but we could potentially write in additional options to pass that in from YAML. For now I've set the sidebar columns ratios to 1fr 2fr, overlay margins to 65%/5% (or 5%/65% on the left). Centre margins are auto, but perhaps we want to set those to percentages too to fix the width. Mobile narrative margins are 0.5em.

jimjam-slam commented 3 months ago

Re. sidebar widths, we may want to adjust this to accommodate your comment @andrewpbray:

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

andrewpbray commented 3 months ago

Ooo, feel like a kid in a candy shop getting to look at all this fine work! This inspires three lines of thinking:

YAML [1/3] On a very long roadtrip yesterday I got to thinking about how we could leverage logic similar to code cell options for propagating yaml options down into our document. What are your thoughts on having the users do something like:

format:
  closeread-html:
    cr-section:
      layout: {sidebar-left, overlay-center, etc}
    trigger:
      padding: 40svh
      propagate-focus: {up, down, false}
    sticky:
      margin: 10 10 10 10 

The propagate-focus key could be the lua filter behavior that we spoke of yesterday but the remainder would be ways to pass stuff to the classes .cr-section, .trigger, or .sticky (I'd imagine just offering up the most-used style attributes for those who don't want to tinker with the .css file). In the document, we could allow syntax like:

:::{.cr-section layout="overlay-right"}
:::

Which the filter would annotate with a higher level of specificity to override any global layout specified in the yaml.

andrewpbray commented 3 months ago

JS vs Lua [2/3] What's the rationale behind assigning some of these classes in the js instead of in the Lua filter? My rough model for the division of responsibilities there is that the Lua makes the HTML doc (and css) that populates the initial doc that loads. Any changes to it based on user input (i.e. scrolling) is handled by js.

Would it work to write a lua filter that would look something like:

-- set default
local section_layout = "sidebar-left"
-- overwrite section_layout if found in yaml via separate filter on meta

function assign_layouts(div)

  if div.classes:includes("cr-section") then
    -- loop over attributes to find any that are layout
    --  if found, use it to overwrite section_layout

    -- form appropriate class string from section_layout
    -- add class to div and return it

  end
end

What are your thoughts on the tradeoffs of these two methods? I know your time is very limited. I'm happy to work on a lua implementation if that's helpful.

andrewpbray commented 3 months ago

Display [3/3]

I'm running into some unexpected behavior when viewing https://closeread.netlify.app/, which published via quarto publish locally.

Sticky blocks looks like the overlay layout, as planned (albeit with some ojs errors). Minard, though, looks like it's still on sidebar-right:

Screenshot 2024-07-29 at 12 41 08 PM

Any ideas what's up?

To loop back to this topic:

Re. sidebar widths, we may want to adjust this to accommodate your comment @andrewpbray:

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

It's not a super strong preference, but I think I do prefer that fixed sidebar. Right now nytimes.css is getting there with min-width: 400px; which... I don't quite understand. It seems like it's behaving as if width: 400px, i.e. it does grow beyond 400 px when the page gets very wide.

What do you think the best way is to implement this? Which units?

jimjam-slam commented 3 months ago

JS vs Lua [2/3] What's the rationale behind assigning some of these classes in the js instead of in the Lua filter? My rough model for the division of responsibilities there is that the Lua makes the HTML doc (and css) that populates the initial doc that loads. Any changes to it based on user input (i.e. scrolling) is handled by js.

Would it work to write a lua filter that would look something like:

-- set default
local section_layout = "sidebar-left"
-- overwrite section_layout if found in yaml via separate filter on meta

function assign_layouts(div)

  if div.classes:includes("cr-section") then
    -- loop over attributes to find any that are layout
    --  if found, use it to overwrite section_layout

    -- form appropriate class string from section_layout
    -- add class to div and return it

  end
end

What are your thoughts on the tradeoffs of these two methods?

If you're up for writing Lua to assign either the default layout class (according to the YAML or a fallback default) or the specified class on each .cr-layout, I think that would simplify the CSS a lot and remove a good bit of JS. I'd love to see that!

My Lua's gotten a bit rusty, so when I approached this I went with the technique I'd used last week for the "remove header space" work: writing from the YAML to a <meta> tag, and then transferring that to the body in the JS (as much as I don't love that technique). Though now you lay it out in this comment, it doesn't seem as difficult as it felt yesterday 😂

I know your time is very limited. I'm happy to work on a lua implementation if that's helpful.

I appreciate that, and I'm sorry I can't hang around to refine this further! I was pushing this work around my plate a bit 😅

jimjam-slam commented 3 months ago

Ooo, feel like a kid in a candy shop getting to look at all this fine work! This inspires three lines of thinking:

YAML [1/3] On a very long roadtrip yesterday I got to thinking about how we could leverage logic similar to code cell options for propagating yaml options down into our document. What are your thoughts on having the users do something like:

format:
  closeread-html:
    cr-section:
      layout: {sidebar-left, overlay-center, etc}
    trigger:
      padding: 40svh
      propagate-focus: {up, down, false}
    sticky:
      margin: 10 10 10 10 

The propagate-focus key could be the lua filter behavior that we spoke of yesterday but the remainder would be ways to pass stuff to the classes .cr-section, .trigger, or .sticky (I'd imagine just offering up the most-used style attributes for those who don't want to tinker with the .css file). In the document, we could allow syntax like:

:::{.cr-section layout="overlay-right"}
:::

Which the filter would annotate with a higher level of specificity to override any global layout specified in the yaml.

I like this! If we add additional options to set widths and such, I imagine we'd write them directly onto the elements' style attributes and strip them out of the SCSS.

jimjam-slam commented 3 months ago

Display [3/3]

I'm running into some unexpected behavior when viewing https://closeread.netlify.app/, which published via quarto publish locally.

Sticky blocks looks like the overlay layout, as planned (albeit with some ojs errors). Minard, though, looks like it's still on sidebar-right:

Screenshot 2024-07-29 at 12 41 08 PM

Any ideas what's up?

Is this happening on the main branch, or on a branch deploy for this branch? Minard seemed to work well with all the layouts when I tested it locally... 🤔

To loop back to this topic:

Re. sidebar widths, we may want to adjust this to accommodate your comment @andrewpbray:

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

It's not a super strong preference, but I think I do prefer that fixed sidebar. Right now nytimes.css is getting there with min-width: 400px; which... I don't quite understand. It seems like it's behaving as if width: 400px, i.e. it does grow beyond 400 px when the page gets very wide.

What do you think the best way is to implement this? Which units?

Instead of using grid here, they're using a flexbox. The advantage of a flexbox is that you can specify the degree to which elements can grow or shrink to use up additional available space.

I haven't used it before, but we might be able to replace 1fr for the sidebar with minmax(400px, 1fr) to enforce a minimum. That said, if we do we might want to bump our mobile sizing threshold up from XS: 576px leaves just 176px for the content in the worst case. The next Bootstrap breakpoint up is 768px, which leaves 368px for the content.

But I think if someone was using an overlay layout, they'd probably want a narrower switch. So maybe we should have separate minimums for sidebar and overlay (not hard to do at all).

andrewpbray commented 3 months ago

Display

Is this happening on the main branch, or on a branch deploy for this branch? Minard seemed to work well with all the layouts when I tested it locally... 🤔

That was on the feature-layout-options branch when published from my local machine to netlify. I just published another version from that branch to my quarto pub account, but it looks the same: https://andrew.quarto.pub/closeread3/. Do you want to try publishing to quarto pub and see how it looks?

I haven't used it before, but we might be able to replace 1fr for the sidebar with minmax(400px, 1fr) to enforce a minimum.

Sounds good to me! We can tinker with it as we build out the gallery and doing more fine tune testing as the talk approaches. (speaking of talk, I'll be talking about this for the first time at JSM in about a week. That'll be a good dry run for the posit::conf.)

YAML syntax and JS vs Lua I'll go ahead and work on this implementation! 🚀

One thing about your approach of meta tag injection: I think that might be necessary since it needs to be attached to <body>. The stuff for the sections is all accessible in the AST, though, so it should be straightforward. 🤞

jimjam-slam commented 3 months ago

That sounds good!

Do you want to try publishing to quarto pub and see how it looks?

I can't make promises, I'm afraid 😔 I might be able to later in the week if I find myself with some down time, but the next couple days I'll be in the air!

andrewpbray commented 3 months ago

Here's the kernel of the approach of reading layout from the document yaml but allowing a section attribute to override it: https://github.com/qmd-lab/closeread/pull/55/commits/a6698c03bdc2951dffe79557e5ee569449bdd317.

That was quite a bit simpler than I imagined!