-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Don't call OnScrolled if offset did not change #2646
Conversation
I meant this to be a draft PR sorry. I cannot figure out yet why the visual tests fail just because we are not triggering on duplicate OnScroll calls |
Have you managed to find the source for the error? |
I'm getting there. Lots of widgets just worked off this false assumption. |
Finally got to the bottom of it :) *) Calling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think there might be something wrong here. When adding a few more tabs to the DocTabs in fyne_demo, the scroller jumps back to the start after a while of dragging it to the right.
I would go about replicating it like this:
- Go to the DocTabs section in fyne_demo.
- Press the "+" button to add a few tabs and thus get a scroller.
- Grab hold of the scroller with the mouse and drag it to the right.
- About 2/3 on the way to the right, the scroller jumps back to the start again.
This is a good catch thanks @Jacalz, however the issue is on develop too, so not part of the changes here. |
Are you sure? I tested on develop and couldn't quite replicate it. I'll have to try again, I guess. |
Quite sure - it seems to glitch when it's about to show the selected tab when scrolling. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are entirely correct. Sorry for the fuss.
Not a problem thanks @Jacalz. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good twice :)
Fixes #1868
Checklist: