-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fixing Orthogonal Nested ScrollViews #9118
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
base: master
Are you sure you want to change the base?
Conversation
…cted. A movement on the inner ScrollView will not be blocked by the outer ScrollView.
|
Here is a test program I used to interactively test the behavior of nested scroll views. """
Test file for demonstrating nested ScrollViews with orthogonal scrolling.
This test shows how the updated ScrollView implementation allows proper
nested scrolling when the scroll directions are perpendicular to each other.
Layout:
- Top level: Horizontal BoxLayout
- Left side: Vertical ScrollView containing multiple horizontal ScrollViews
- Right side: Horizontal ScrollView containing multiple vertical ScrollViews
"""
from kivy.app import App
from kivy.uix.boxlayout import BoxLayout
from kivy.uix.gridlayout import GridLayout
from kivy.uix.button import Button
from kivy.uix.label import Label
from kivy.core.window import Window
from kivy.metrics import dp
from kivy.uix.scrollview import ScrollView
class NestedScrollViewTest(App):
def build(self):
# Main horizontal layout
main_layout = BoxLayout(orientation='horizontal', spacing=10, padding=10)
# Left side: Vertical ScrollView with horizontal ScrollViews inside
left_vertical_sv = ScrollView(
do_scroll_x=False, # Only vertical scrolling
do_scroll_y=True,
scroll_type=['bars', 'content'],
bar_width=dp(8),
bar_color=[0.3, 0.6, 1.0, 0.8]
)
# Container for horizontal ScrollViews
left_container = BoxLayout(
orientation='vertical',
spacing='20dp',
size_hint_y=None
)
left_container.bind(minimum_height=left_container.setter('height'))
# Add multiple horizontal ScrollViews
for i in range(8):
# Create a horizontal ScrollView
horizontal_sv = ScrollView(
do_scroll_x=True, # Only horizontal scrolling
do_scroll_y=False,
scroll_type=['bars', 'content'],
size_hint_y=None,
height=dp(120),
bar_width=dp(6),
bar_color=[1.0, 0.5, 0.3, 0.8]
)
# Create content for horizontal ScrollView
horizontal_content = BoxLayout(
orientation='horizontal',
spacing=dp(10),
size_hint_x=None
)
horizontal_content.bind(minimum_width=horizontal_content.setter('width'))
# Add a label to identify this row
label = Label(
text=f'Row {i+1} - Horizontal Scroll',
size_hint_x=None,
width=dp(200),
height=dp(30),
color=[1, 1, 1, 1],
bold=True
)
horizontal_content.add_widget(label)
# Add buttons to make content wider than the ScrollView
for j in range(15):
btn = Button(
text=f'Btn {j+1}',
size_hint_x=None,
width=dp(100),
height=dp(80),
background_color=[0.2 + (i * 0.1) % 0.8, 0.3, 0.7, 1]
)
horizontal_content.add_widget(btn)
horizontal_sv.add_widget(horizontal_content)
left_container.add_widget(horizontal_sv)
left_vertical_sv.add_widget(left_container)
# Right side: Horizontal ScrollView with vertical ScrollViews inside
right_horizontal_sv = ScrollView(
do_scroll_x=True, # Only horizontal scrolling
do_scroll_y=False,
scroll_type=['bars', 'content'],
bar_width=dp(8),
bar_color=[0.3, 1.0, 0.6, 0.8]
)
# Container for vertical ScrollViews
right_container = BoxLayout(
orientation='horizontal',
spacing='20dp',
size_hint_x=None
)
right_container.bind(minimum_width=right_container.setter('width'))
# Add multiple vertical ScrollViews
for i in range(6):
# Create a vertical ScrollView
vertical_sv = ScrollView(
do_scroll_x=False, # Only vertical scrolling
do_scroll_y=True,
scroll_type=['bars', 'content'],
size_hint_x=None,
width=dp(200),
bar_width=dp(6),
bar_color=[1.0, 0.3, 0.5, 0.8]
)
# Create content for vertical ScrollView
vertical_content = GridLayout(
cols=1,
spacing=dp(10),
size_hint_y=None,
height=0
)
vertical_content.bind(minimum_height=vertical_content.setter('height'))
# Add a label to identify this column
label = Label(
text=f'Column {i+1}\nVertical Scroll',
size_hint_y=None,
height=dp(40),
color=[1, 1, 1, 1],
bold=True
)
vertical_content.add_widget(label)
# Add buttons to make content taller than the ScrollView
for j in range(20):
btn = Button(
text=f'Item {j+1}',
size_hint_y=None,
height=dp(60),
background_color=[0.7, 0.3, 0.2 + (i * 0.1) % 0.8, 1]
)
vertical_content.add_widget(btn)
vertical_sv.add_widget(vertical_content)
right_container.add_widget(vertical_sv)
right_horizontal_sv.add_widget(right_container)
# Add both sides to main layout
main_layout.add_widget(left_vertical_sv)
main_layout.add_widget(right_horizontal_sv)
return main_layout
if __name__ == '__main__':
NestedScrollViewTest().run() |
|
I'm converting this to a draft, I need to make changes to the mouse wheel handling. |
|
Mouse wheel handing has been added. |
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.
Tested, LGTM!
|
I've found an issue when multiple scrollviews are visible - as in the testcase above. The touch grabs were not being properly released. I've got a fix and will update the PR in a day or so. Thanks for your patience. |
…ing. There was an interaction with grabbed touches. A number of "guards" have been established to ensure proper touch grab management.
|
The issue with grabs has been fixed. Here is a brief summary. I added some additional comments to the code that relates to these changes.
|
…er-touch metadata.
|
I'm marking this as a draft again. I found some new corner cases causing incorrect behavior. I am making progress on a new implementation that uses a special widget for nesting ScrollViews, NestedScrollVIewManager that will result in a more maintainable solution. I'll leave this as a draft for now. If I prefer the new implementation I will close this PR. If not I will come back and work to fix these corner cases. In my proposed implementation for nested scrollviews they will be under a NestedScrollViewManager. It will look something like this: My objective is to deliver a solution that is more maintainable both for ScrollView and the NestedScrollViewManager. |
@ElliotGarbus Even with an alternative implementation, I believe this PR is necessary to address issues that make implementing this orthogonal sliding behavior (even without nested scrollviews) complicated: |
|
@DexerBR I agree on the need. My current status: I have simplified the ScrollView, refactored the long methods with helper functions and added significant implementation comments. I will be starting on the NestedScrollViewManager later today. When I have something working I'll share it on discord - and then create a new PR. I'll look forward to your feedback. If for some reason NestedScrollViewManager seems like the wrong direction, I'll come back to this PR. As I see it today, adding the NestedScrollViewManger will allow new capabilities and improve the maintainability of the code. |
All right 👍 Considering this hypothetical layout, how would the NestedScrollViewManager behave? FloatLayout:
BoxLayout:
ScrollView:
BoxLayout:
Button:
ScrollView:
BoxLayout:
BoxLayout:
Button:
ScrollView:
BoxLayout:
Button:
BoxLayout:
ScrollView:
Button:
BoxLayout:
Button:
ScrollView:
BoxLayout: |
|
To use the Here are my initial thoughts on the implementation, I'm confident these details will change. The The This information is used to control the behavior of the
The Having knowledge of the inner vs outer scrollview will simplify the behavior and make the code more maintainable. Happy to answer any questions. |

The modifications allow orthoganal nested ScrollViews to work as expected.
Previously, if a
ScrollViewwas nested inside anotherScrollView, theinner scrollview would consume all touches, preventing the outer scrollview
from scrolling, even when the movement was orthogonal to the inner's scroll
axis.
Now, when two
ScrollViewwidgets are nested with perpendicular scrollingdirections (e.g., inner vertical, outer horizontal), touches are routed
according to movement direction:
ScrollViewhandles scrolling.ScrollView,allowing it to scroll.
The direction of the touch is determined only at the beginning of the on touch move, changing orientation(from vertical to horizontal for example) requires releasing the touch and starting a new touch. Only one
ScrollViewis active at a time.There is a similar change made to support the mouse scroll wheel.
If this
ScrollViewcan't scroll in the incoming wheel direction, or there's nothing to scroll on that axis, the event is not consumed, False is returned so the outerScrollViewcan handle the scroll.This behavior is automatic and requires no configuration changes.
It makes nested scrolling more intuitive.
The direction of the touch is determined only at the beginning of the on touch move, changing orientation(from vertical to horizontal for example) requires releasing the touch and starting a new touch. Only one
ScrollViewis active at a time.This PR fixed the following issues:
#5617
#6889
#4052
#2986
#8926
Maintainer merge checklist
Component: xxxlabel.api-deprecationorapi-breaklabel.release-highlightlabel to be highlighted in release notes.versionadded,versionchangedas needed.