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

WIP transition works, except blue desktop #11

Closed
wants to merge 6 commits into from

Conversation

jimjam-slam
Copy link
Collaborator

First test of actual Scrollama transitions.

This works, with some caveats:

  1. The blue desktop one doesn't currently work because both the wrapper

    and the contents have .cr-sticky.

  2. The double counting of blue desktop is throwing off the count of subsequent sticky elements.

I think both these problems can be solved if the Lua script removes the class from the when it moves it to the wrapper

. But,

  1. Events sometimes don't trigger properly at narrower widths. I think this is related to the visibility of the displayed elements (although I'm not sure why that would be the case). Hopefully building a proper mobile layout will ameliorate this problem!

Thst said, it broadly works as expected! 🥳

@jimjam-slam
Copy link
Collaborator Author

jimjam-slam commented Aug 4, 2023

Also I've used explicit .cr-sticky and .cr-step here; I've gotta go back over our discussion to remember what we agreed on!

@andrewpbray
Copy link
Contributor

Hey, that's awesome!

It's pretty neat how, on the markdown side, all it really takes is tacking a class onto an element, div or span. I can't wait to play with this stuff!

I just made a PR into this branch that should hopefully remove the cr-sticky class from inner Inline elements so as not to throw things off with the count in the sticky stack.

@andrewpbray
Copy link
Contributor

I've had a chance to read through the scrollama js code and I think I have a sense of how it's built (that console logging is super helpful!).

It strikes me that there are (at least) two different transitions here that could be helpful: fade-in and fade-out-and-fade-in. As I understand it, you've implemented the former: any sticky element that scrolls into the viewer has the cr-scrolledby class added, which sets the opacity to 1. fade-out-then-in would do the same thing, but also remove the class from the previous element.

In terms of API, this could be implemented by using .cr-fade-in and .cr-fade-out-and-fade-in. I'm just spitballing, but from there would the approach be to add something like:

   scroller
     .setup({
       step: ".cr-fade-out-and-fade-in",
       offset: 0.4
     })
     .onStepEnter((response) => {
       if (response.direction == "down") {

         let allStickies = Array.from(
            document.querySelectorAll(".cr-sticky, [data-cr=\"sticky\"]"));
         let currentSticky = allStickies[response.index - 1];
         let otherStickies = allStickies.slice(0, response.index - 1).concat(allStickies.slice(response.index))

         currentSticky.classList.add("cr-scrolledby");
         otherStickies.forEach(e => e.classList.remove("cr-scrolledby"));
      }
     })
       // plus the rewind version

As I think about it, I'm pretty sure this won't work. The dilemma is illustrated by this line:

currentSticky = allStickies[response.index - 1]

allStickies is an array built from the set of cr-sticky elements. response.index is the index from the set of cr-step elements. We're safe to index the former with the latter if there are the same number of elements in each set. If we partition the cr-step set into cr-fade-in and cr-fade-out-and-fade-in, then I think it'll break.

Looking at the scrollama docs, we also have access to response.element and step: can accept an array of selectors, so we could alternatively:

  1. Keep one call to scrollama
  2. In the setup, use step: [".cr-fade-in", ".cr-fade-out-and-fade-in"] (or whatever the right syntax is)
  3. In onStepEnter, add conditional logic that changes which items in the cr-sticky array get classes based on the class of response.element

Do you think this is a good way to approach it - differentiating transitions types based on class and the implementation approach outlined directly above? If so, I can take a crack at implementing it.

@jimjam-slam
Copy link
Collaborator Author

That's right! The only complication I would mention is that I wrote the effect of .cr-scrolledby (specifically the opacity: 1) to only kick in on the last element to have that class, rather than all of them. The main reason I did that was that since the sticky elements are all superimposed, if several of them are visible, and they have different sizes, you get them overlapping rather than just seeing one at any given time.

I think having both .cr-fade-in and .cr-fade-out-and-fade-in would make a lot of sense in a revealjs fragment ish situation where you may want to have three elements side-by-side (or stacked vertically) and have one fade in at each step! I hadn't even considered that situation—I'd only been thinking about sticky elements that replace each other. I like your scope better!

Your reasoning around making the scroller step selector complex (ie. adding several classes as steps) and using conditional logic in the callbacks is sound to me 😊 Happy to see you go at it!

I'll post another comment for the other change we might need below to keep my thoughts straight 😅

@jimjam-slam
Copy link
Collaborator Author

If I understand the need for different transition classes correctly above, we'll also need additional styles available to lay out things like row- or column-based lists to have things fade in sequentially.

One way we could do that is to separate out the two jobs that .cr-sticky is doing:

  1. to mark the content as needing to be rearranged in the BOM (by the Lua script)
  2. to target the element to receive a scroll status lass for transitioning (in the JavaScript).

For example, .cr-sticky could just be used by the Lua and SCSS to stack elements on the side. Then, cr-transition or some such could be used either on the sticky element itself, or on a child, to mark the thing to be faded in (or out). This way, you could make a div sticky but have it be a list of things that are going to fade in at different steps.

The only downside of this is that it complicates our API a bit. When reveal has things like this, it only has the fragment itself—the step side is self-evident (it's a click). For us, we need the steps and the content.

This might mean that explicitly linking steps and content with an attribute (eg. cr-step="2" and `.cr-sticky cr-transition="2") makes more sense than it did before. What do you think?

@andrewpbray
Copy link
Contributor

I think I understand most of your thoughts, but I don't quite get the use-case you're envisioning:

I think having both .cr-fade-in and .cr-fade-out-and-fade-in would make a lot of sense in a revealjs fragment ish situation where you may want to have three elements side-by-side (or stacked vertically) and have one fade in at each step! I hadn't even considered that situation—I'd only been thinking about sticky elements that replace each other. I like your scope better!

I think of our current fade-in setup one where, as a step comes into view, the next sticky element in the stack transitions from an opacity of 0 to 1. That leads to an effect where each new sticky element appears on top of the previous stack, like this blue square appearing on top of the pink square:

Screen Shot 2023-08-07 at 5 16 03 PM

I was envisioning the fade-out-and-fade-in analog at this part in the example doc to be just the blue square; the pink square would have faded out as the blue one faded in. So it'd still be one element replacing one another. Could you say more about what you mean by three elements side by side?

@jimjam-slam
Copy link
Collaborator Author

Ooooh. Do you mean that you were expecting the fade out of the previous element to have been completed before the next fade in starts (rather than a cross-fade effect)? If that's the case, we can just add a delay to the fade in in CSS and call it a day! I had assumed you meant something more like an incremental list in Quarto, where it's A visible -> A and B visible -> A, B and C visible.

If you mean that the pink square is continuing to remain visible after the blue has faded in, we might have a CSS/JS bug there. I coded the opacity: 1 transition to only target the last element with .scrolledby, not all of them. So when the next element fades in, the previous one should no longer be the last one with that class and should revert to opacity: 0.

@andrewpbray
Copy link
Contributor

Ah, perhaps a bug! Indeed the pink square remains visible. Here is what it looks like once I've scrolled to the bottom of the second div:

Screen Shot 2023-08-07 at 6 55 42 PM

Ok, I see now what you mean now by the incremental list. That's like a series of revealjs fragments (that don't use r-stack): the object is present in the layout (i.e. it takes up space) but is not visible. More parallels with revealjs!

@jimjam-slam
Copy link
Collaborator Author

jimjam-slam commented Aug 8, 2023

Ahh, gotcha! If you let me know what browser and OS you're on, I can take a look—I might need to make sure we're not relying on anything too exotic as far as the transition CSS goes.

@andrewpbray
Copy link
Contributor

I'm on:

MacOS Monterey 12.6.6
Chrome Version 115.0.5790.170 (Official Build) (x86_64)

@jimjam-slam
Copy link
Collaborator Author

Closing this branch in favour of the cr-sidebar-transitionandfocus, (which includes the changes in cr-sidebar-enablescrollama, but I'll likely wipe most of them out on the JS side refactoring)

@jimjam-slam jimjam-slam deleted the cr-sidebar-enablescrollama branch December 29, 2023 23:14
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.

None yet

2 participants