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

[feature] Prop for setting 2nd param of springPositions.set() #92

Open
jfrancos opened this issue Dec 17, 2022 · 7 comments
Open

[feature] Prop for setting 2nd param of springPositions.set() #92

jfrancos opened this issue Dec 17, 2022 · 7 comments
Labels
feature request investigating Not quite sure if it's valid, yet

Comments

@jfrancos
Copy link

jfrancos commented Dec 17, 2022

Describe the feature
I'd like to be able to set the 2nd param of springPositions.set(), so I can adjust animations on the fly.

Explain it's value / reasoning
I'd like to remove the animation effect in the case where a user is continuously sliding the nub.

Additional context
It's a very small adjustment to the code, which I've done in a fork. I've tried it out, and it does what I want. Obvs happy to submit a PR, but thought I'd open an issue for it first (I haven't contributed much to open source projects so I'm not sure what is customary here?)

@jfrancos jfrancos added feature request investigating Not quite sure if it's valid, yet labels Dec 17, 2022
@jfrancos
Copy link
Author

jfrancos commented Dec 17, 2022

This is what I'm then doing in my own code, to create the effect I'm looking for:

<script>
...
  const handleTouchEnd = () => {
    setSpringOptions = undefined;
    removeEventListener("touchend", handleTouchEnd);
    addEventListener("touchmove", handleTouchMove);
  };

  const handleTouchMove = () => {
    setSpringOptions = { hard: true };
    removeEventListener("touchmove", handleTouchMove);
    addEventListener("touchend", handleTouchEnd);
  };

  onMount(() => addEventListener("touchmove", handleTouchMove));
  onDestroy(() => {
    removeEventListener("touchmove", handleTouchMove);
    removeEventListener("touchend", handleTouchEnd);
  });
...
</script>

@simeydotme
Copy link
Owner

thanks @jfrancos ..

I kind of understand your desired effect, not sure fully how to approach this to be honest... is it not OK for you to dampen spring uniformly? What's the need for two effects in different contexts?

(p.s thanks for the two issues, I will get to the other one as soon a I can, I have a branch for it but I found an issue which would take me much more to test and investigate, 🙏 )

@jfrancos
Copy link
Author

Hi @simeydotme,

The desire for changing the effect is wanting it to snap to each value, while a user is sliding it through the different values. With the animation in effect, it kind of glides through the values as the user is sliding their finger, rather than stopping at each one.

But, I'd still like the animated effect when the handle is moving over longer distances.

As for how to approach it ... this very tiny change to src/RangeSlider.svelte does the trick. If you're okay with it, I can open a PR.

@simeydotme
Copy link
Owner

simeydotme commented Dec 20, 2022

yeh, I can see the change and how it works... but my gripe is how it's encouraging this;

const handleTouchEnd = () => {
    setSpringOptions = undefined;
    removeEventListener("touchend", handleTouchEnd);
    addEventListener("touchmove", handleTouchMove);
  };

  const handleTouchMove = () => {
    setSpringOptions = { hard: true };
    removeEventListener("touchmove", handleTouchMove);
    addEventListener("touchend", handleTouchEnd);
  };

  onMount(() => addEventListener("touchmove", handleTouchMove));
  onDestroy(() => {
    removeEventListener("touchmove", handleTouchMove);
    removeEventListener("touchend", handleTouchEnd);
  });

which I believe to be a code smell.
(and in fact, you should probably consider using these events instead!)


I feel like it should probably be a boolean option where I can optimise the toggling of the spring internally... but then I feel like why not just make it default behaviour? Is there any value to having the handle be springy while being dragged? The only downside is it's an aesthetically change

@jfrancos
Copy link
Author

jfrancos commented Dec 23, 2022

(and in fact, you should probably consider using these events instead!)

That would be nice, except that those custom events don't reveal what the underlying event is, so I don't think there'd be a way to differentiate between moving between two adjacent positions via press vs drag.

That said, if you want to add this behavior as an option, that would be really cool and I could delete a few lines of code! :)

Is there any value to having the handle be springy while being dragged?

I personally think it feels unnatural for it to be springy while dragged, but I see how others may want this behavior.

@jfrancos
Copy link
Author

jfrancos commented Dec 23, 2022

Also fwiw, I've updated my code to

const handleTouchEnd = () => {
  setSpringOptions = undefined;
};

const handleTouchMove = () => {
  setSpringOptions = { hard: true };
};

...
<div
  on:touchmove={handleTouchMove}
  on:touchend={handleTouchEnd}
>
  <RangeSlider ... />
</div>

@simeydotme
Copy link
Owner

thanks for the discussion @jfrancos , al very valuable :)
I will try to approach this in the coming weeks, I might have to push a minor version and caveat it has aesthetic changes.

@simeydotme simeydotme added this to the Svelte 4+ , Typescript milestone Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request investigating Not quite sure if it's valid, yet
Projects
None yet
Development

No branches or pull requests

2 participants