Create a base scrollable container class and move scrolling bindings to that #2332
Assignees
Labels
Comments
davep
added a commit
to davep/textual
that referenced
this issue
Apr 20, 2023
The idea here is that not every widget will scroll, and as such not every widget needs to have default bindings for calling the scrolling methods. Generally scrolling is something done in a container. These days we have *Scroll containers. As such it makes sense to introduce the bindings in a common parent class for those containers. This commit moves the binding from widget and creates that common parent class, and then has HorizontalScroll and VerticalScroll inherit from it. This is, it should be noted, a breaking change. Any code that creates a scrolling widget that assumes that the bindings are just there, where that widget doesn't inherit either from HorizontalScroll or VerticalScroll, will suddenly find that scrolling with the keyboard is no longer possible. See Textualize#2332.
davep
added a commit
to davep/textual
that referenced
this issue
Apr 20, 2023
davep
added a commit
to davep/textual
that referenced
this issue
Apr 20, 2023
This is, to some degree, rendered moot by Textualize#2332, but for the moment it still feels worth doing. The initial intention was to make sure that non-scrolling containers and their child classes don't have bindings that may mask other uses for navigation keys. However, it was realised that the "problem" affected more than just containers (hence Textualize#2332). But... on the off chance we add any more default bindings to `Widget` (unlikely, but still), this will mean that they don't leak into the containers unless we intend them to. See Textualize#2331.
|
Don't forget to star the repository! Follow @textualizeio for Textual updates. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently the navigation keys used in scrolling are all bound in Widget. This causes some problems, such as #2100. Given that no widget should be doing scrolling that doesn't inherit from a
*Scrollcontainer in some way, that code should move to aScrollableContainerwidget, whereVerticalScrollandHorizontalScrollinherit form that.This way the bulk of our widgets won't be competing with
Widgetwhen it comes to doing things with navigation key bindings.See also #2331.
The text was updated successfully, but these errors were encountered: