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

Queries/walk_children no longer includes self in results by default #1416

Merged
merged 8 commits into from
Dec 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [0.8.0] - Unreleased

### Fixed
### Fixed

- Fixed issues with nested auto dimensions https://github.com/Textualize/textual/issues/1402
- Fixed watch method incorrectly running on first set when value hasn't changed and init=False https://github.com/Textualize/textual/pull/1367
Expand All @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Changed

- Moved Ctrl+C, tab, and shift+tab to App BINDINGS
- Queries/`walk_children` no longer includes self in results by default https://github.com/Textualize/textual/pull/1416

## [0.7.0] - 2022-12-17

Expand Down
2 changes: 1 addition & 1 deletion src/textual/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -1491,7 +1491,7 @@ def _on_idle(self) -> None:
nodes: set[DOMNode] = {
child
for node in self._require_stylesheet_update
for child in node.walk_children()
for child in node.walk_children(with_self=True)
}
self._require_stylesheet_update.clear()
self.stylesheet.update_nodes(nodes, animate=True)
Expand Down
6 changes: 3 additions & 3 deletions src/textual/css/match.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ def match(selector_sets: Iterable[SelectorSet], node: DOMNode) -> bool:


def _check_selectors(selectors: list[Selector], css_path_nodes: list[DOMNode]) -> bool:
"""Match a list of selectors against a node.
"""Match a list of selectors against DOM nodes.

Args:
selectors (list[Selector]): A list of selectors.
node (DOMNode): A DOM node.
css_path_nodes (list[DOMNode]): The DOM nodes to check the selectors against.

Returns:
bool: True if the node matches the selector.
bool: True if any node in css_path_nodes matches a selector.
"""

DESCENDENT = CombinatorType.DESCENDENT
Expand Down
2 changes: 1 addition & 1 deletion src/textual/css/stylesheet.py
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ def update(self, root: DOMNode, animate: bool = False) -> None:
animate (bool, optional): Enable CSS animation. Defaults to False.
"""

self.update_nodes(root.walk_children(), animate=animate)
self.update_nodes(root.walk_children(with_self=True), animate=animate)

def update_nodes(self, nodes: Iterable[DOMNode], animate: bool = False) -> None:
"""Update styles for nodes.
Expand Down
12 changes: 6 additions & 6 deletions src/textual/dom.py
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ def reset_styles(self) -> None:
"""Reset styles back to their initial state"""
from .widget import Widget

for node in self.walk_children():
for node in self.walk_children(with_self=True):
node._css_styles.reset()
if isinstance(node, Widget):
node._set_dirty()
Expand Down Expand Up @@ -648,7 +648,7 @@ def walk_children(
self,
filter_type: type[WalkType],
*,
with_self: bool = True,
with_self: bool = False,
method: WalkMethod = "depth",
reverse: bool = False,
) -> list[WalkType]:
Expand All @@ -658,7 +658,7 @@ def walk_children(
def walk_children(
self,
*,
with_self: bool = True,
with_self: bool = False,
method: WalkMethod = "depth",
reverse: bool = False,
) -> list[DOMNode]:
Expand All @@ -668,16 +668,16 @@ def walk_children(
self,
filter_type: type[WalkType] | None = None,
*,
with_self: bool = True,
with_self: bool = False,
method: WalkMethod = "depth",
reverse: bool = False,
) -> list[DOMNode] | list[WalkType]:
"""Generate descendant nodes.
"""Walk the subtree rooted at this node, and return every descendant encountered in a list.

Args:
filter_type (type[WalkType] | None, optional): Filter only this type, or None for no filter.
Defaults to None.
with_self (bool, optional): Also yield self in addition to descendants. Defaults to True.
with_self (bool, optional): Also yield self in addition to descendants. Defaults to False.
method (Literal["breadth", "depth"], optional): One of "depth" or "breadth". Defaults to "depth".
reverse (bool, optional): Reverse the order (bottom up). Defaults to False.

Expand Down
56 changes: 40 additions & 16 deletions tests/test_query.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import pytest

from textual.app import App, ComposeResult
from textual.containers import Container
from textual.widget import Widget
from textual.css.query import InvalidQueryFormat, WrongType, NoMatches, TooManyMatches

Expand Down Expand Up @@ -50,17 +52,16 @@ class App(Widget):
assert list(app.query(".frob")) == []
assert list(app.query("#frob")) == []

assert app.query("App")
assert not app.query("NotAnApp")

assert list(app.query("App")) == [app]
assert list(app.query("App")) == [] # Root is not included in queries
assert list(app.query("#main")) == [main_view]
assert list(app.query("View#main")) == [main_view]
assert list(app.query("View2#help")) == [help_view]
assert list(app.query("#widget1")) == [widget1]
assert list(app.query("#Widget1")) == [] # Note case.
assert list(app.query("#Widget1")) == [] # Note case.
assert list(app.query("#widget2")) == [widget2]
assert list(app.query("#Widget2")) == [] # Note case.
assert list(app.query("#Widget2")) == [] # Note case.

assert list(app.query("Widget.float")) == [sidebar, tooltip, helpbar]
assert list(app.query(Widget).filter(".float")) == [sidebar, tooltip, helpbar]
Expand Down Expand Up @@ -110,10 +111,10 @@ class App(Widget):
tooltip,
]

assert list(app.query("App,View")) == [app, main_view, sub_view, help_view]
assert list(app.query("View")) == [main_view, sub_view, help_view]
assert list(app.query("#widget1, #widget2")) == [widget1, widget2]
assert list(app.query("#widget1 , #widget2")) == [widget1, widget2]
assert list(app.query("#widget1, #widget2, App")) == [app, widget1, widget2]
assert list(app.query("#widget1, #widget2, App")) == [widget1, widget2]

assert app.query(".float").first() == sidebar
assert app.query(".float").last() == helpbar
Expand All @@ -130,14 +131,13 @@ class App(Widget):


def test_query_classes():

class App(Widget):
pass

class ClassTest(Widget):
pass

CHILD_COUNT=100
CHILD_COUNT = 100

# Create a fake app to hold everything else.
app = App()
Expand All @@ -147,34 +147,35 @@ class ClassTest(Widget):
app._add_child(ClassTest(id=f"child{n}"))

# Let's just be 100% sure everything was created fine.
assert len(app.query(ClassTest))==CHILD_COUNT
assert len(app.query(ClassTest)) == CHILD_COUNT

# Now, let's check there are *no* children with the test class.
assert len(app.query(".test"))==0
assert len(app.query(".test")) == 0

# Add the test class to everything and then check again.
app.query(ClassTest).add_class("test")
assert len(app.query(".test"))==CHILD_COUNT
assert len(app.query(".test")) == CHILD_COUNT

# Remove the test class from everything then try again.
app.query(ClassTest).remove_class("test")
assert len(app.query(".test"))==0
assert len(app.query(".test")) == 0

# Add the test class to everything using set_class.
app.query(ClassTest).set_class(True, "test")
assert len(app.query(".test"))==CHILD_COUNT
assert len(app.query(".test")) == CHILD_COUNT

# Remove the test class from everything using set_class.
app.query(ClassTest).set_class(False, "test")
assert len(app.query(".test"))==0
assert len(app.query(".test")) == 0

# Add the test class to everything using toggle_class.
app.query(ClassTest).toggle_class("test")
assert len(app.query(".test"))==CHILD_COUNT
assert len(app.query(".test")) == CHILD_COUNT

# Remove the test class from everything using toggle_class.
app.query(ClassTest).toggle_class("test")
assert len(app.query(".test"))==0
assert len(app.query(".test")) == 0


def test_invalid_query():
class App(Widget):
Expand All @@ -187,3 +188,26 @@ class App(Widget):

with pytest.raises(InvalidQueryFormat):
app.query("#foo").exclude("#2")


async def test_universal_selector_doesnt_select_self():
class ExampleApp(App):
def compose(self) -> ComposeResult:
yield Container(
Widget(
Widget(),
Widget(
Widget(),
),
),
Widget(),
id="root-container",
)

app = ExampleApp()
async with app.run_test():
container = app.query_one("#root-container", Container)
query = container.query("*")
results = list(query.results())
assert len(results) == 5
assert not any(node.id == "root-container" for node in results)