`<Pace>` does not respect `getPace` prop when `ms={undefined|null}`

[anulman]

Hello! Me again 😅

I was wondering if there's a story behind your use of 'ms' in props over in Pace.tsx.

Specifically, I'm trying to add a "hold button to speed up"-style UX to my windup, and when in this mode would like to use a pace of 4ms per character; meanwhile when I'm not in this mode I'm hoping to use the defaultGetPace implementation. Intended usage looks something like:

<windups.WindupChildren>
  <windups.Pace ms={isInFastMode ? 4 : null} getPace={windups.defaultGetPace}>
    this is a lot of text that a user might fast-mode through
  </windups.Pace>
</windups.WindupChildren>

Is this another contrib I can help with?

[anulman]

Update: I tried an alternative solution, roughly speaking:

const getPace = React.useCallback((char: string) => {
  const defaultPace = windups.defaultGetPace(char, undefined);
  return isInFastMode ? (defaultPace / 50) : defaultPace;
}, [isInFastMode]);

// ... later
<windups.WindupChildren>
  <windups.Pace getPace={getPace}>
    this is a lot of text that a user might fast-mode through
  </windups.Pace>
</windups.WindupChildren>

This unfortunately doesn't work—I'm guessing the original callback prop is getting cached somewhere and never swaps in the new one?—but the diff to support ms={null} is super tidy. ~~I'll publish a PR shortly!~~

Edit: lol, guess not. I'm having the same "persisted prop" issue when toggling between ms={isInFastMode ? 4 : null}! Back to the stacktracing...

[sgwilym]

Hello! Me again 😅

Always nice to see a familiar face.

The use-case you're describing is one I definitely see the need for and think windups should support.

But... I think it might not be possible with this version of Windups. It's about how Windups deal with equivalence.

You're bumping into a version of this problem. Here's the relevant part:

🐸 WindupChildren is only able to compare the text content and shape of the children you pass it.

🐸 What it can't do is know if you changed the values of any components or their props inside that.

This is a caveat I had to accept to make WindupChildren's API possible. It stems from the identity of the children prop changing on every render in React.

Now if you did what the frog suggests and change the key of the Pace element, it would pick up on the change. Unfortunately it would also restart the windup, as it's regarded as a completely new windup.

Now this is where the unreleased version 2 that's been sitting on the shelf completely unloved by me comes in: it has a RewindupChildren component that can diff two windups, and resume from their earliest common point. I think it could handle this use-case. Let me know if that interests you.

By the way: thank you for contributing to this repo. I'm heads down with a funded OSS project that leaves me very little time for anything else, and it annoys me that I have all this new windups stuff lying around simply because I can't together the time to write docs for it.

[anulman]

Now this is where the unreleased version 2 that's been sitting on the shelf completely unloved by me comes in: it has a RewindupChildren component that can diff two windups, and resume from their earliest common point. I think it could handle this use-case. Let me know if that interests you.

😂 Lol, I hear you; I've had many fun projects linger on shelves too. Yes this is definitely an incentive to migrate to Rewindups! How do you suggest I hop on?

And no worries about the time / availability thing. I am very incentivized to get this working, so if you can even point me to the v2 code I'd be happy to fiddle around in there and help you get the docs release-ready.

[anulman]

Hey Sam! Sending you a gentle + compassionate nudge on this thread—I appreciate that your focus is elsewhere! I tried to look through the branches published on Github and can't find the "prerelease" source anywhere.

If you could please git push the changes to the repo, I'd be delighted to port the pause/resume work into it, self-serve my way into <RewindupChildren>, and otherwise help however possible (within reason though lol) once you're ready to get it ready to publish.

[sgwilym]

Thanks for your patience. I'll get round to pushing up a branch, in the meantime there is a release on NPM you can try:

yarn add [email protected]

import { useRewindupString, RewindupChildren } from 'windups'

[anulman]

Alas, unless that version supports the new isPaused prop I can't really use it in prod! The isPaused work is more critical to the site experience we're going for 😭

No worries mate, I can keep waiting. You did such a great job with this lib already, this is just a minor UX quibble from some of my testers 😄