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

Add remove #653

Merged
merged 7 commits into from
Aug 9, 2022
Merged

Add remove #653

merged 7 commits into from
Aug 9, 2022

Conversation

willmcgugan
Copy link
Collaborator

Implements a remove() method to remove a widget from the DOM.

@willmcgugan willmcgugan marked this pull request as draft August 6, 2022 12:40
@willmcgugan willmcgugan mentioned this pull request Aug 6, 2022
@@ -856,7 +856,7 @@ def _register_child(self, parent: DOMNode, child: DOMNode) -> bool:
return True
return False

def register(
def _register(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Devs should never need to call this directly.

There are probably a number of other methods that should be private. Will be an ongoing process to identify them

@@ -36,14 +37,16 @@ def __init__(
self,
node: DOMNode | None = None,
selector: str | None = None,
nodes: list[DOMNode] | None = None,
nodes: list[Widget] | None = None,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Queries now return Widgets rather than DOMNodes. This means that it excludes App, which is find since it is a special object that doesn't have any visuals. Although you can still reference App in a selector.

@willmcgugan willmcgugan requested a review from darrenburns August 6, 2022 13:16
@willmcgugan willmcgugan marked this pull request as ready for review August 6, 2022 13:16
@willmcgugan willmcgugan merged commit 031ff27 into css Aug 9, 2022
@willmcgugan willmcgugan deleted the add-remove branch August 9, 2022 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants