qmd-lab / closeread

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

Add image zooming feature #47

Closed andrewpbray closed 4 months ago

andrewpbray commented 4 months ago

@jimjam-slam Here's a first draft of image zooming functionality 🖼️ .

A couple items of note in this PR.

  1. This uses scale-by and pan-to pandoc attributes that map directly to scale and translate style attributes. The nice thing about the direct mapping is that you can use in the pandoc attributes any units that are fine in the css. It's so close, I wonder: should we just use the pandoc names of scale and translate? Or is that so similar as to be confusing what is closeread and what is html? Is there any worry that the use of transform will cause Safari problems like you had on the poems?

  2. I've bundled the new transformImage function into an updateStickies function.

  3. I've commented out the ojs variable code that's throwing errors on this branch (but fixed on another I believe!)

  4. I've increased the padding somewhat on the narrative blocks to, inspired by the style of nyt closeread.

  5. I've changed the scrollama triggering conditions so that updateStickies is called only on Enter, not Exit. I'm imagining a situation where you have a step that reveals an image and then 5 more steps that just talk about it. I wouldn't want the image to disappear after you've moved away from the step. We can introduce a .clearAll class or something that could be added to step if we want to clear out everything. Can you think of any ways in which we'll really miss that Exit behavior?

  6. I've altered the filter a bit so that every sticky has a "sticky" class and every step a "step" class. Might useful for css styling - a bit easier than using the attributes to select - and also makes the html a bit easier to read at a glance.

jimjam-slam commented 4 months ago

Hot damn @andrewpbray this just works!!!

jimjam-slam commented 4 months ago

should we just use the pandoc names of scale and translate? Or is that so similar as to be confusing what is closeread and what is html?

I think scale-by and pan-to are fine! I think the names' primary job is to tell people what they do plainly, and the educational value of lining it up with CSS is pretty minimal. Plus, 'translate' can be a bit of a technical term.

Is there any worry that the use of transform will cause Safari problems like you had on the poems?

I think the Safari transform problem is limited to text content that gets prematurely rasterised — it should be fine with an actual raster (although you'd likely want a nice big raster to use!). We might want to do some tests for SVG plots though.

I've commented out the ojs variable code that's throwing errors on this branch (but fixed on another I believe!)

No worries!

I've increased the padding somewhat on the narrative blocks to, inspired by the style of nyt closeread.

Yeah, I like 45 svh much more than 20 svh!

I've changed the scrollama triggering conditions so that updateStickies is called only on Enter, not Exit. I'm imagining a situation where you have a step that reveals an image and then 5 more steps that just talk about it. I wouldn't want the image to disappear after you've moved away from the step.

That makes sense! We were doing some gymnastics before to think about exactly rh situation you describe (reveal on one step, then talk about it for several more), so if this simplifies that, I like it.

But if we have, say:

  1. enter red sticky
  2. discuss
  3. discuss
  4. discuss
  5. enter blue sticky

What happens if someone scrolls down past blue sticky, then back up? If you scroll back up through 4/3/2, aren't you still looking at blue until 1 enters again? Are we backtracking to find the last focus-on, or do users just need to repeat focus-on for 2/3/4? (I think the latter is a good solution, I just want to make sure I understand)

I've altered the filter a bit so that every sticky has a "sticky" class and every step a "step" class. Might useful for css styling - a bit easier than using the attributes to select - and also makes the html a bit easier to read at a glance.

Yeah, that's fair. Classes are also more performant, and if we can add those classes with Lua, why not?

I'm happy with this! The work you've done on the JS has really improved the design of the package and the readability of the code

jimjam-slam commented 4 months ago

I reckon it'd be good to throw this one in /gallery/demos too!

andrewpbray commented 4 months ago

I'm going to go ahead and merge and pick up the very good conversation about the on-entrance / on-exit design in an issue: https://github.com/qmd-lab/closeread/issues/54.