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: Add always_enable_overscroll property on scrollview #6678

Merged
merged 1 commit into from
Jun 14, 2020

Conversation

tshirtman
Copy link
Member

@tshirtman tshirtman commented Jan 2, 2020

fix: #6665
2020-01-12 00-17-18

@tshirtman tshirtman self-assigned this Jan 2, 2020
@tshirtman tshirtman force-pushed the feature/scrollview_always_allow_overscroll branch from 5c2ccac to 367f4c1 Compare January 11, 2020 23:11
@tshirtman tshirtman changed the title [WIP] add always_enable_overscroll property on scrollview Add always_enable_overscroll property on scrollview Jan 11, 2020
@matham
Copy link
Member

matham commented Jan 11, 2020

Shouldn't this be True by default? Also, I think always_overscroll is probably better as it's more concise. Although it would be nice to be more specific than always, as that's quite vague as to what it means. Maybe even overscroll_smaller_viewport, although it's a bit wordy.

@tshirtman
Copy link
Member Author

tshirtman commented Jan 11, 2020

I figured making it True by default could be an unwanted change in behavior, and i tend to be conservative about these things, even if i think it's indeed a better default behavior, if you think it's good enough to allow people who don't want this behavior to disable it, then i'm fine with switching the default value.

the "always" seems important to me as currently we have overscroll only in the specific situation where the content is big enough to allow scrolling, this just makes the behavior unconditonnal.

Also, it did remind me that it's impossible to overscroll with the mousewheel currently, (and with this patch it's now possible… but only if the content is small enough to prevent normal scrolling 😬 😆) i think this should be changed and mouse scroll should work the same as touch scroll in that regard, any opinion?

@tshirtman tshirtman force-pushed the feature/scrollview_always_allow_overscroll branch from 367f4c1 to 2c1f9df Compare February 22, 2020 21:30
@tshirtman
Copy link
Member Author

Renamed the property, and enabled it by default, documented the fact that you should disable it to get the previous behavior back.

@tshirtman tshirtman force-pushed the feature/scrollview_always_allow_overscroll branch 2 times, most recently from 99fca71 to 0ff84b5 Compare May 22, 2020 21:08
@tshirtman tshirtman force-pushed the feature/scrollview_always_allow_overscroll branch from 0ff84b5 to e98503a Compare May 22, 2020 21:22
@matham matham added the Notes: API-break API was broken with backwards incompatibality label May 22, 2020
@matham
Copy link
Member

matham commented May 22, 2020

Looks ok if you're happy with it!?

@matham matham merged commit 99af5f1 into master Jun 14, 2020
@matham matham deleted the feature/scrollview_always_allow_overscroll branch June 14, 2020 16:55
:attr:`always_overscroll` is a
:class:`~kivy.properties.BooleanProperty` and defaults to `True`.
versionadded:: 2.0.0
The option was added and enabled by default, set to False to get the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tshirtman Shouldn't there be a new line before versionadded tag?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, maybe, i'll add it along the unittest fix, good call.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe docs won't be generated property without new line.
Also, better readability :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, and the .. before was missing as well, good thing you mentioned it.

@matham matham added this to the 2.0.0 milestone Oct 28, 2020
@matham matham changed the title Add always_enable_overscroll property on scrollview ScrollView: Add always_enable_overscroll property on scrollview Dec 9, 2020
@matham matham added the Component: Widgets kivy/uix, style.kv label Dec 9, 2020
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 Notes: API-break API was broken with backwards incompatibality
Projects
None yet
3 participants