Skip to content

Conversation

@ElliotGarbus
Copy link
Contributor

@ElliotGarbus ElliotGarbus commented Aug 15, 2025

The modifications allow orthoganal nested ScrollViews to work as expected.

Previously, if a ScrollView was nested inside another ScrollView, the
inner 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 ScrollView widgets are nested with perpendicular scrolling
directions (e.g., inner vertical, outer horizontal), touches are routed
according to movement direction:

  • Same-axis movement: The inner ScrollView handles scrolling.
  • Orthogonal movement: The touch is released to the parent 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 ScrollView is active at a time.

There is a similar change made to support the mouse scroll wheel.
If this ScrollView can'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 outer ScrollView can 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 ScrollView is active at a time.

This PR fixed the following issues:
#5617
#6889
#4052
#2986
#8926

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.

…cted.

A movement on the inner ScrollView will not be blocked by the outer ScrollView.
@ElliotGarbus ElliotGarbus requested a review from kuzeyron August 15, 2025 23:52
@ElliotGarbus
Copy link
Contributor Author

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()

@ElliotGarbus
Copy link
Contributor Author

I'm converting this to a draft, I need to make changes to the mouse wheel handling.

@ElliotGarbus ElliotGarbus marked this pull request as draft August 16, 2025 13:28
@ElliotGarbus
Copy link
Contributor Author

Mouse wheel handing has been added.

@ElliotGarbus ElliotGarbus marked this pull request as ready for review August 17, 2025 00:26
Copy link
Contributor

@DexerBR DexerBR left a comment

Choose a reason for hiding this comment

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

Tested, LGTM!

@kuzeyron kuzeyron added the Component: core-widget properties, eventdispatcher, widget.py, animation label Aug 19, 2025
@ElliotGarbus ElliotGarbus marked this pull request as draft August 22, 2025 01:09
@ElliotGarbus
Copy link
Contributor Author

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.
@ElliotGarbus
Copy link
Contributor Author

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.
The core problem: There are two nested scrollviews in the right-hand side and left-hand side of a Boxlayout. Aggressively using right-hand side nested scrollview, and then very quickly moving to the other side would not result in the proper grab handing. A number of changes were made to address this issue:

  • Prevent off-viewport grabs

    • on_touch_down: returns False immediately when the touch is outside this ScrollView (collide_point check) so siblings/parent can handle it.
  • Safe redispatch gating

    • simulate_touch_down: blocks redispatch if another widget currently owns the touch (grab_current not in {None, self}) or if this ScrollView isn’t in touch.grab_list.
    • Introduced a per-touch override flag svforce (keyed by this SV’s uid) so intentional redispatch paths can bypass the guard.
  • Intentional redispatch paths use override

    • _change_touch_mode: wraps simulate_touch_down with svforce so the timeout-based redispatch to children still works.
    • on_scroll_stop (click passthrough when mode is unknown): wraps simulate_touch_down with svforce for single-tap forwarding.
  • Prompt cleanup on release

    • on_touch_up: if this ScrollView previously handled the touch (its uid key exists in touch.ud), it now dispatches on_scroll_stop, ungrabs if still holding, preserves focus behavior, and returns. This prevents lingering grabs that block siblings.

@ElliotGarbus ElliotGarbus marked this pull request as ready for review August 22, 2025 03:10
@ElliotGarbus ElliotGarbus marked this pull request as draft September 3, 2025 13:51
@ElliotGarbus
Copy link
Contributor Author

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:

NestedScrollViewManager:
    ScrollView:  # the outer scrollview
        ...attributes and layout
        ...
        ScrollView:  # one or more scrollviews deeper in the widget tree

My objective is to deliver a solution that is more maintainable both for ScrollView and the NestedScrollViewManager.

@DexerBR
Copy link
Contributor

DexerBR commented Sep 3, 2025

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:

NestedScrollViewManager:
    ScrollView:  # the outer scrollview
        ...attributes and layout
        ...
        ScrollView:  # one or more scrollviews deeper in the widget tree

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:

@ElliotGarbus
Copy link
Contributor Author

@DexerBR I agree on the need.
In my implementation, the ScrollView widget will be simplified as it no longer directly supports nesting. Adding the NestedScrollViewManager becomes a bigger PR with a greater impact. I hope to be done in less than 2 weeks.

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.

@DexerBR
Copy link
Contributor

DexerBR commented Sep 3, 2025

@DexerBR I agree on the need. In my implementation, the ScrollView widget will be simplified as it no longer directly supports nesting. Adding the NestedScrollViewManager becomes a bigger PR with a greater impact. I hope to be done in less than 2 weeks.

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:

@ElliotGarbus
Copy link
Contributor Author

ElliotGarbus commented Sep 3, 2025

To use the NestedScrollViewManager, we would modify the widget tree. The NestedScrollViewManager, is a RelativeLayout that captures and routes touches/events to it's child scrollviews. My objective is to create a system that enables new capabilities and is also more maintainable.

FloatLayout:
    BoxLayout:
        NestedScrollViewManager:
            ScrollView:  # this is the outer SV
                BoxLayout:
                    Button:
                    ScrollView:  # an inner SV
                        BoxLayout:
                        BoxLayout:
                    Button:
                    ScrollView:  # an inner SV
                        BoxLayout:
                            Button:
                            BoxLayout:
                                ScrollView:  # I'll ignore this... I'm not contemplating triple nesting...
                                Button:
        BoxLayout:
        Button:
    ScrollView:  # this is not a nested scrollview
        BoxLayout:

Here are my initial thoughts on the implementation, I'm confident these details will change.

The NestedScrollViewManager will grab the touch and ungrab the touch as required, and issue on_scroll_start, move and start events as required to the appropriate underlying scrollview (outer or inner).

The NestedScrollViewManager and the ScrollView will use touch.ud (user data dictionary) to maintain per-touch state across the touch lifecycle (down -> move -> up). This is critical for:
1. Nested ScrollView coordination (avoiding conflicts)
2. Gesture detection (scroll vs click )
3. State persistence across method calls

This information is used to control the behavior of the ScrollView and message routing based on the state of the touch.

touch.ud Keys for Nested Scroll Behavior

Key Purpose
mode Indicates whether the touch has been classified pending, scrolling, replay.
nested_managed Marks that this touch is being managed by a NestedScrollViewManager. Prevents confusion with normal ScrollView touches.
nested_role Tracks the current ownership/interpretation of the touch within nested scrolling (e.g. "inner", "outer", "inner-rejected").
scroll_owner Identifies the specific scrollview widget currently handling the scroll.

The NestedScrollViewMagager always knows the outer scrollview - it is it's child. The inner scrollview is "discovered" if the on_scroll_start message to children returns True. This way there is no additional overhead to track inner scrollviews or requirement for the developer to tag an inner scrollview.

Having knowledge of the inner vs outer scrollview will simplify the behavior and make the code more maintainable.
It will required the creation of the NestedScrollViewManager widget, and an update to the ScrollView.
I'll know much more after I have something working.

Happy to answer any questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: core-widget properties, eventdispatcher, widget.py, animation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants