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

refreshing invisible widgets is a no-op #5063

Merged
merged 15 commits into from
Sep 30, 2024
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## Unreleased

### Fixed

- Fixed issue with screen not updating when auto_refresh was enabled https://github.com/Textualize/textual/pull/5063

### Added

- Added `DOMNode.is_on_screen` property https://github.com/Textualize/textual/pull/5063
- Added support for keymaps (user configurable key bindings) https://github.com/Textualize/textual/pull/5038

## [0.81.0] - 2024-09-25
Expand Down
23 changes: 17 additions & 6 deletions src/textual/_compositor.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,6 @@ def __init__(self) -> None:
# Mapping of line numbers on to lists of widget and regions
self._layers_visible: list[list[tuple[Widget, Region, Region]]] | None = None

# New widgets added between updates
self._new_widgets: set[Widget] = set()

def clear(self) -> None:
"""Remove all references to widgets (used when the screen closes)."""
self._full_map.clear()
Expand Down Expand Up @@ -392,8 +389,7 @@ def reflow(self, parent: Widget, size: Size) -> ReflowResult:
new_widgets = map.keys()

# Newly visible widgets
shown_widgets = (new_widgets - old_widgets) | self._new_widgets
self._new_widgets.clear()
shown_widgets = new_widgets - old_widgets

# Newly hidden widgets
hidden_widgets = self.widgets - widgets
Expand Down Expand Up @@ -490,7 +486,6 @@ def full_map(self) -> CompositorMap:
self._full_map_invalidated = False
map, _widgets = self._arrange_root(self.root, self.size, visible_only=False)
# Update any widgets which became visible in the interim
self._new_widgets.update(map.keys() - self._full_map.keys())
self._full_map = map
self._visible_widgets = None
self._visible_map = None
Expand Down Expand Up @@ -803,6 +798,22 @@ def layers_visible(self) -> list[list[tuple[Widget, Region, Region]]]:
self._layers_visible = layers_visible
return self._layers_visible

def __contains__(self, widget: Widget) -> bool:
"""Check if the widget was included in the last update.

Args:
widget: A widget.

Returns:
`True` if the widget was in the last refresh, or `False` if it wasn't.
"""
# Try to avoid a recalculation of full_map if possible.
return (
widget in self.widgets
or (self._visible_map is not None and widget in self._visible_map)
or widget in self.full_map
)

def get_offset(self, widget: Widget) -> Offset:
"""Get the offset of a widget.

Expand Down
7 changes: 6 additions & 1 deletion src/textual/dom.py
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,11 @@ def is_modal(self) -> bool:
"""Is the node a modal?"""
return False

@property
def is_on_screen(self) -> bool:
"""Check if the node was displayed in the last screen update."""
return False

def automatic_refresh(self) -> None:
"""Perform an automatic refresh.

Expand All @@ -497,7 +502,7 @@ def automatic_refresh(self) -> None:
during an automatic refresh.

"""
if self.display and self.visible:
if self.is_on_screen:
self.refresh()

def __init_subclass__(
Expand Down
10 changes: 6 additions & 4 deletions src/textual/screen.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ class Screen(Generic[ScreenResultType], Widget):

DEFAULT_CSS = """
Screen {

layout: vertical;
overflow-y: auto;
background: $surface;
Expand All @@ -162,11 +163,11 @@ class Screen(Generic[ScreenResultType], Widget):
background: ansi_default;
color: ansi_default;

&.-screen-suspended {
&.-screen-suspended {
text-style: dim;
ScrollBar {
text-style: not dim;
}
text-style: dim;
}
}
}
Expand Down Expand Up @@ -1136,8 +1137,9 @@ async def _on_update(self, message: messages.Update) -> None:
widget = message.widget
assert isinstance(widget, Widget)

self._dirty_widgets.add(widget)
self.check_idle()
if self in self._compositor:
self._dirty_widgets.add(widget)
self.check_idle()

async def _on_layout(self, message: messages.Layout) -> None:
message.stop()
Expand Down
9 changes: 9 additions & 0 deletions src/textual/widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -1927,6 +1927,15 @@ def _has_relative_children_height(self) -> bool:
return True
return False

@property
def is_on_screen(self) -> bool:
"""Check if the node was displayed in the last screen update."""
try:
self.screen.find_widget(self)
except (NoScreen, errors.NoWidget):
return False
return True

def animate(
self,
attribute: str,
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
36 changes: 36 additions & 0 deletions tests/snapshot_tests/test_snapshots.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
Footer,
Log,
OptionList,
Placeholder,
SelectionList,
)
from textual.widgets import ProgressBar, Label, Switch
Expand Down Expand Up @@ -2098,3 +2099,38 @@ def action_toggle_console(self) -> None:

app = MRE()
assert snap_compare(app, press=["space", "space", "z"])


def test_updates_with_auto_refresh(snap_compare):
"""Regression test for https://github.com/Textualize/textual/issues/5056

After hiding and unhiding the RichLog, you should be able to see 1.5 fully rendered placeholder widgets.
Prior to this fix, the bottom portion of the screen did not
refresh after the RichLog was hidden/unhidden while in the presence of the auto-refreshing ProgressBar widget.
"""

class MRE(App):
BINDINGS = [
("z", "toggle_widget('RichLog')", "Console"),
]
CSS = """
Placeholder { height: 15; }
RichLog { height: 6; }
.hidden { display: none; }
"""

def compose(self):
with VerticalScroll():
for i in range(10):
yield Placeholder()
yield ProgressBar(classes="hidden")
yield RichLog(classes="hidden")

def on_ready(self) -> None:
self.query_one(RichLog).write("\n".join(f"line #{i}" for i in range(5)))

def action_toggle_widget(self, widget_type: str) -> None:
self.query_one(widget_type).toggle_class("hidden")

app = MRE()
assert snap_compare(app, press=["z", "z"])
Loading