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

[BUG] ListBox wrapped in AttrMap is not scrollable with ScrollBar #878

Closed
3 tasks done
rsekman opened this issue Apr 30, 2024 · 1 comment · Fixed by #879
Closed
3 tasks done

[BUG] ListBox wrapped in AttrMap is not scrollable with ScrollBar #878

rsekman opened this issue Apr 30, 2024 · 1 comment · Fixed by #879
Labels

Comments

@rsekman
Copy link
Contributor

rsekman commented Apr 30, 2024

Description:

Wrapping a ListBox in an AttrMap should be transparent for operations such as adding a ScrollBar, but throws because the AttrMap is not realised to be scrollable.

Affected versions (if applicable)
  • master branch (7300acf)
  • Latest stable version from pypi
  • All versions since 2.5.0
Steps to reproduce (if applicable)
import urwid as uw
w = uw.ScrollBar(uw.AttrMap(uw.ListBox([]), {}))
Expected/actual outcome

Expected behaviour: w can be scrolled as if it were not wrapped in an AttrMap.
Actual behaviour:

Traceback (most recent call last):
  File "/home/koyomi/Sandbox/test_attrmap.py", line 8, in <module>
    w = uw.ScrollBar(am)
        ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/urwid/widget/scrollable.py", line 486, in __init__
    raise TypeError(f"Not a scrollable widget: {widget!r}")
TypeError: Not a scrollable widget: <AttrMap selectable box widget <ListBox selectable box widget> attr_map={}>
Cause

While Scrollable is aware of wrappers and descends into them to see if the original widgets are scrollable

def orig_iter(w: Widget) -> Iterator[Widget]:
while hasattr(w, "original_widget"):
w = w.original_widget
yield w
yield w
w = self
for w in orig_iter(self):
if isinstance(w, SupportsScroll):
return w

ScrollBar is not and does not
if not isinstance(widget, SupportsScroll):
raise TypeError(f"Not a scrollable widget: {widget!r}")

Fix

Make the check in ScrollBar analogous to the check in Scrollable.

@rsekman rsekman added the bug label Apr 30, 2024
rsekman added a commit to rsekman/urwid that referenced this issue Apr 30, 2024
penguinolog: `w.original_widget` can point to `w`, we need to filter such behaviour to prevent infinite loop
@mfncooper
Copy link
Contributor

Note that this bug also affects the tour.py example that ships with Urwid. This example thus provides a nice test case for the fix in #879, built right into the Urwid package. It would be good to verify that the examples work before releasing.

penguinolog added a commit that referenced this issue May 14, 2024
…879)

* fix: make ScrollBar check wrapped widgets for SupportsScroll (Fixes #878)

* tests: add tests for scrollable widgets wrapped in AttrMap

* fix: avoid possible infinite loop in Scrollable/ScrollBar (#878)

penguinolog: `w.original_widget` can point to `w`, we need to filter such behaviour to prevent infinite loop

* fixup! fix: avoid possible infinite loop in Scrollable/ScrollBar (#878)

* Update urwid/widget/scrollable.py

Fix static checker warning

---------

Co-authored-by: Alexey Stepanov <penguinolog@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants