Structure of scroller-init.js

[andrewpbray]

Hey @jimjam-slam , I've been going through scroller-init.js with the goals of a) understanding more of it now that I'm getting a sense of how JavaScript works and b) thinking for any opportunities to refactor in light of our discussions on how steps can trigger either transition or focus effect. I've been running into some specific questions about implementation that I figured would be easiest to ask here in the Discussions area.

[andrewpbray]

closeread/_extensions/closeread/scroller-init.js

Lines 47 to 48 in 85b9d78

	currentIndex = response.index + 1
	recalculateActiveSteps()

Is there a reason why current_index isn't passed in as an argument to recalculateActiveSteps()?

I'm only four chapters into the Eloquent JavaScript book, so please forgive any silly questions. 🤪

[jimjam-slam]

Not a silly question at all! current_index was initially passed, but in feac991 I changed it to being a global.

Prior to that, all access to the index flowed through there and was pure, so I was able to just pass it. I believe the reason I made the switch was either:

• that I introduced a code path that ran outside the Scrollama onStepEnter/onStepExit -> recalculateActiveSteps() code path, or
• the index was no longer pure (ie. I had to change it backward from where it was accessed).

Looking at the code now, though, I'm less confident of that! Maybe it would be better to revert to having it passed? 😅

[andrewpbray]

Makes sense! I was looking for examples of both bullet points but I wasn’t able to find any; may well have missed it though.

[andrewpbray]

closeread/_extensions/closeread/scroller-init.js

Lines 124 to 128 in 85b9d78

	// do the visibility update
	targets[0].classList.add("cr-active")
	if (targets[0].classList.contains("cr-poem")) {
	updateActivePoem(targets[0], priorSteps)
	}

Am I correct that since recalculateActiveSteps() is triggered on any event (transition or focus), if it's only a focus effect, this will also apply the default transition effect? That's probably the correct and safe approach here, but I just wanted to be sure I'm understanding this.

I guess this is necessary, too, since even if the sticky item were given .cr-active in a previous step, it would have been removed at the start of this step. In terms of execution time, is that remove-class / add-back-class ever visible?

[jimjam-slam]

recalculateActiveSteps() is triggered any time Scrollama registers a narrative block entering to or exiting from the bottom, or when the window is resized. It should trigger regardless of whether the block in question has just a transition or a focus effect (but the logic further down inside recalculateActiveSteps() should consider that more).

In terms of execution, I've generally found adding and removing classes is pretty instant! I don't think I've ever observed any race conditions around it.

One thing you'll notice with this code is that once the triggering block is identified, we recalculate the status of all the preceding blocks, rather than just deactivating the old one and activating the new one. I think that was to help minimise problems with fast scrolling (if you scroll fast enough, it's possible to skip blocks entirely, and of course with deep links you can initialise the page mid-way down!).

[andrewpbray]

Got it. I think that recalculation makes a lot of sense. Makes it much more robust and I haven’t been able to detect any performance cost.

[andrewpbray]

closeread/_extensions/closeread/scroller-init.js

Lines 181 to 183 in 85b9d78

	/* scalePoemFull:
	given an element `el`, rescales it to fill its containing .sticky-col-stack */
	function scalePoemFull(el, paddingX = 75, paddingY = 50) {

Might we want to want to have this be the default function applied to any sticky object that overflows the current sticky container?

If you take a look at this nytimes close read example, images get resized if either the horizontal or vertical dimension of the image is greater than the corresponding dimension on the container. As I think through our block types - code, code output, poems, images - it seems like the default behavior that we'd want for all of these. We could allow the user to escape this behavior by adding the .overflow class or something to the sticky element.

We could think of the main body of recalculateActiveSteps() to be:

1. Resize target sticky element unless it has class .overflow or the trigger has a zoom attribute.
2. Apply .cr-active to target sticky element.
3. Apply focus if present
   • zoom
   • highlight

[jimjam-slam]

Yeah, I think it makes sense to rescale non-poem content too (even plots and regular images that aren't necessarily being focused on)!

[jimjam-slam]

Apologies for the slow answers! I go a little waylaid by another package this weekend. Glad you're getting eyes on this code; it's already slipping away from me 😅

[andrewpbray]

No prob! I have at last cleared away all the hanger-on tasks from the spring semester, so I finally have some brainshare to dedicate here. My goal is to spend at least an hour each day this week on it just to become very familiar with how it works.

[andrewpbray]

closeread/_extensions/closeread/scroller-init.js

Lines 27 to 28 in 85b9d78

	ojsScrollerSection?.define("crScrollerSection", null);
	ojsScrollerProgress?.define("crScrollerProgress", null);

An unexpected consequence of initializing crScrollerSection at null is that when it adds to the index, it first goes to zero, then, 1, and so on. The upshot is that when you then scroll back up, subtracting one from the index each time, eventually you end up at:

Is there some advantage to intializing at null or can I switch it to 0?

[jimjam-slam]

Yes, it should start at 0, not null! I keep meaning to fix that 😅