Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OJS globe Demo #40

Merged
merged 20 commits into from
Jul 30, 2024
Merged

OJS globe Demo #40

merged 20 commits into from
Jul 30, 2024

Conversation

jimjam-slam
Copy link
Collaborator

First demo: animating a globe with some labelled cities in OJS!

This is currently animating over time rather than with scroll. It's also not appearing properly (is the trigger working properly?) when Close Read is turned on (but you can see it if you revert to the regular HTML format).

James Goldie added 5 commits June 28, 2024 18:41
This is currently animating over time rather than with scroll. It's also not appearing properly (is the trigger working properly?) when Close Read is turned on.
@andrewpbray
Copy link
Contributor

Minor question: now that we have qmds not at the root of the repo (and it's not a prjoect), they're not able to find the closeread extension. How are you running those?

@andrewpbray
Copy link
Contributor

(this is gonna be super cool tho - great idea for an example!)

@jimjam-slam
Copy link
Collaborator Author

On my laptop I've just copied it down and put a hit ignore on the folder so it's not version controlled, but for the Sverto docs website I copy the _extensions folder down as part of the website deploy action! I assume at some point once we're happy with Close Read we'll organise things into a website :)

@jimjam-slam
Copy link
Collaborator Author

@andrewpbray I missed a couple of things when moving the OJS variable commits over, so I just merged the entire of rename-steps to date into this branch. Now we have a working scrolling globe demo! You just need to copy CR into the doc directory to render it.

The one fly in the soup is that scroll events aren't firing at all on CR narrative blocks that contain OJS outputs or multiple OJS code blocks. You can see this if you take the block where I calculate the angle scale and angle and split it into two code blocks, or if you have a console output anywhere in the code block.

I originally thought this might be #41 shuffling our DOM around again, but inspecting the DOM output, I'm not so sure! The events aren't firing from Scrollama at all.

That said, very happy to have a useful OJS demo!

@andrewpbray
Copy link
Contributor

Wow, this is great!

I'm catching a small blip in the animation when I scroll through and it happens right at the switch to a new step. If you track the value of crTriggerIndex + crScrollProgress, it starts at 0 + 0 and goes smoothly to 0 + .99. Next it flips to 1 + 1, then to 1 + .00 where it goes smoothly to 1 + .99 then flips again.

I've tinkered a bit with the scrollama, trying to get the index to update at the same instant the progress resets, but no luck. If you add console.log() to look at response.index and response.progress, those are tracking our OJS variables, so the problem seems to be with order of firings of scrollama; it updates the index without updating the progress.

Any ideas?

@andrewpbray
Copy link
Contributor

As an aside, this was not the implementation that I expected, but it is quite clever. I was imagining that we'd use a narrative block-spanning step so that the animation would all take place as part of the same step progress. Something like:


::::{.cr-layout}

:::{focus-on="map"}
We want our globe to rotate with the scroll progress — between -180 and 180.  

Instead of trying to do the maths to scale it ourselves, we can make a scale with d3.

There are six narrative blocks that we want to scale over, but I'd like the scrolling to start a little late and end a little early — by the time the last block has just started.

So between 0.5 (because the scroll starts with the first narrative block of the document) and 5.1. If the numbers go outside this range, we'll _clamp_ them so that the scrolling doesn't continue.
:::

And then you could just stick to tracking crScrollProgress.

Will this work from a scrollama perspective, tracking the progress of a very tall element through the threshold? Thoughts on the tradeoffs of these two methods?

One thing that strikes me with the current method is that if you insert other steps / stickies before the ojs map, you'd have to adjust the scale for the new indices.


PS: as I play around with putting multiple paragraphs within a step, it's not quite the behavior I was expecting: the CSS adds padding to any child element of .sidebar-col

  .sidebar-col {
    grid-column: 1;
    
    /* extended wrapper around content to allow early scroll detection */
    > * {
      padding-block-start: 20svh;
      padding-block-end: 20svh;
      vertical-align: middle;

I'm thinking we should specifically target step elements with this padding. What do you think? And what's the best way to target? Assign a class in the filter? Just use the [data-focus-on] attribute?

@jimjam-slam
Copy link
Collaborator Author

@andrewpbray I think we'd probably want some sort of different identifier for a "compound" narrative block — otherwise it's difficult to tell whether somebody wants one or just wants a regular narrative block comprised of a couple of paragraphs.

That said, in 941f382 I have fixed the original problem with the animation catching! scroller-init.js now resets scroll progress to either 0 or 1 when a block trigger event comes in. This fixes problems with the timing of those events.

There is a bit more fiddly math than I'd like dealing with the null value of the scroll trigger, though 🤔

Merge branch 'main' into feature-demos

# Conflicts:
#	_extensions/closeread/scroller-init.js
#	demo-block-types.qmd
@jimjam-slam jimjam-slam mentioned this pull request Jul 16, 2024
//| label: angle-scale
//| echo: true
//| code-fold: false
angleScale = d3.scaleLinear()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised this works! Is the d3 library accessible without the need to use require()?

https://quarto.org/docs/interactive/ojs/libraries.html#d3

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll double check that with the Quarto team! Observable Plot uses d3, so maybe that's why it's always available...

@andrewpbray andrewpbray changed the title Demos OJS globe Demo Jul 23, 2024
@andrewpbray andrewpbray marked this pull request as ready for review July 25, 2024 16:06
response.progress);
ojsScrollerDirection?.define("crScrollerDirection",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there's some differences here between using scroll and scroller in the name. What about using the name of whatever we settle on: "trigger" or "step"? (we can discuss this in our meeting!)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm, yeah! I currently have:

  • crTriggerIndex
  • crStickyName
  • crScrollProgress
  • crScrollDirection

But I agree that we should standardise it around triggers and steps, which are the names we've been using elsewhere. So how about instead:

  • crTriggerIndex
  • crStickyName
  • crTriggerProgress
  • crDirection or crTriggerDirection

For crStickyName, it's important to note that it is the name of the sticky element, not the trigger (the trigger generally being the one you'd want to use to drive animation). I actually don't think this one is super necessary, unless you intended for it to be the ID of the trigger instead! If that's the case and we re-wired it, crTriggerId would be good too... or we could remove trigger altogether and just have crIndex, crId, crProgress and crDirection.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this proposal looks good! I think flagging Trigger vs Sticky is helpful. I know while workign on the js that having those well named helps in thinking through what you're doing.

My plan would be to so a search-and-replace of every incidence of step in our codebase with trigger. Also to change the name of sidebar-col to narrative-col (I guess caption-col is also an option?). The result would be HTML that looks like...

< div class="cr-layout" >
  < div class="narrative-col" >
    < div class="narrative-block" >
      < some block content >
    < /div >
     < div class="narrative-block trigger" possibly-some-data-attributes >
      < some block content >
    < /div >
  < /div >
  < div class="sticky-col" >
    < div class="sticky-stack" >
      < div class="sticky" >
        < some sticky block content >
      < /div >
      < div class="sticky" >
        < some sticky block content >
      < /div >
    < /div >
  < /div >
< /div>

Questions for you:

  1. Are you good on the codebase changes to use trigger and narrative- (I'm open to alternatives but like these fine)
  2. Do you think we should double class .narrative-block (or .caption?) and .trigger on the same div or should the trigger live in a separate div inside the block?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love to have this added to main, both for the demo and the fixed ojs var code. I'm happy to make these last renaming changes, just let me know!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Questions for you:

  1. Are you good on the codebase changes to use trigger and narrative- (I'm open to alternatives but like these fine)

Sounds good to me!

  1. Do you think we should double class .narrative-block (or .caption?) and .trigger on the same div or should the trigger live in a separate div inside the block?

Mmm, a double class might be handy. It'd also make the CSS more readable, as we wouldn't have to do > * to select the narrative blocks.

I might need to ask you to make the changes, sorry! I have a bit of cleaning up to do before we hop on the plane 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No prob, I'm on it!

docs/gallery/demos/ojs-map/index.qmd Show resolved Hide resolved
@andrewpbray
Copy link
Contributor

This was still looking a bit glitchy on my end up I had my versions messed up, so sorry for the delay. This looks great!

That's a good point about the need for more information to disambiguate a progressing trigger that spans multiple pandoc blocks. I'd say we call this PR excellent and leave that block-spanner for another PR!

@jimjam-slam
Copy link
Collaborator Author

No worries! Have just commented up above on the names of the variables, but once we resolve that it's good to go 😊

merge in main

# Conflicts:
#	_extensions/closeread/scroller-init.js
#	demo-block-types.qmd
@andrewpbray andrewpbray merged commit e19e831 into main Jul 30, 2024
1 check passed
@andrewpbray andrewpbray deleted the feature-demos branch July 30, 2024 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants