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

ScrollView: Match scroll effect stop condition to start condition. #7213

Merged
merged 2 commits into from Dec 10, 2020

Conversation

matham
Copy link
Member

@matham matham commented Nov 12, 2020

Maintainer merge checklist

  • Title is descriptive/clear for inclusion in release notes.
  • Applied a Component: xxx label.
  • Applied the api-deprecation or api-break label.
  • Applied the release-highlight label to be highlighted in release notes.
  • Added to the milestone version it was merged into.
  • Unittests are included in PR.
  • Properly documented, including versionadded, versionchanged as needed.

Fixes #7207.

Currently the stop condition is such that the effect stop may be executed even if the effect start was not called. Because the start and stop were guarded under different conditions. This makes them equal.

@matham matham added the Component: Widgets kivy/uix, style.kv label Nov 12, 2020
@tshirtman
Copy link
Member

Doesn't seem to fix #7207 for me, if i drag the bar first, without touching anything before that, it crashes 100% of the time.

@tshirtman
Copy link
Member

Probably not the prettiest solution, but

--- a/kivy/effects/kinetic.py
+++ b/kivy/effects/kinetic.py
@@ -145,7 +145,7 @@ def update(self, val, t=None):
         See :meth:`start` for the arguments.
         '''
         t = t or time()
-        distance = val - self.history[-1][1]
+        distance = val - (self.history[-1][1] if self.history else 0)
         self.apply_distance(distance)
         self.history.append((t, val))
         if len(self.history) > self.max_history:
diff --git a/kivy/effects/scroll.py b/kivy/effects/scroll.py
index 7a722b8d7..f35b5c39b 100644
--- a/kivy/effects/scroll.py
+++ b/kivy/effects/scroll.py
@@ -113,7 +113,7 @@ def start(self, val, t=None):
         return super(ScrollEffect, self).start(val, t)
 
     def update(self, val, t=None):
-        self.displacement += abs(val - self.history[-1][1])
+        self.displacement += abs(val - (self.history[-1][1] if self.history else 0))
         return super(ScrollEffect, self).update(val, t)
 
     def stop(self, val, t=None):

seems to solve it for me without side effects.

@matham
Copy link
Member Author

matham commented Nov 14, 2020

I overlooked that crash because when I translated from kivymd I didn't fully match the kivymd rules. But it should now be fixed for scroll move as well.

@matham
Copy link
Member Author

matham commented Nov 14, 2020

The history fix would definitely fix it, but it doesn't fix the underlying issue.

@tshirtman
Copy link
Member

Right, but unless we have time to properly fix (and i can't promise i will) i would rather have that.

@matham
Copy link
Member Author

matham commented Nov 17, 2020

Oh, yeah, my recent commit should have fixed it completely.

@matham matham added this to the 2.1.0 milestone Dec 10, 2020
@matham matham merged commit 2debbc3 into master Dec 10, 2020
@matham matham deleted the matham-patch-2 branch December 10, 2020 03:52
js-blueprint added a commit to KMcleanBPL/kivy that referenced this pull request Jan 21, 2021
hamlet4401 pushed a commit to tytgatlieven/kivy that referenced this pull request Jul 3, 2021
…ivy#7213)

* Match effect stop condition to start condition.

* Also fix for scroll move.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Widgets kivy/uix, style.kv
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ScrollView Scroll Throws Bug IndexError
2 participants