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 control to widget messages. #2490

Merged
merged 9 commits into from
May 8, 2023
Merged

Add control to widget messages. #2490

merged 9 commits into from
May 8, 2023

Conversation

rodrigogiraoserrao
Copy link
Contributor

@rodrigogiraoserrao rodrigogiraoserrao commented May 4, 2023

This adds the property control to all the widget messages.

Closes #2489.
Closes #2486.

Adding these references to the sub-widgets that make up a markdown document is necessary in order for the blocks to be able to post messages with a reference to the original document, which in turn is needed for the Message.control property to work.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This may look like an unrelated change, but I needed to update this documentation to add control.
But this was the source of the issue with #2486 so I just went ahead and took care of that as well.

@rodrigogiraoserrao rodrigogiraoserrao merged commit a31e086 into main May 8, 2023
@rodrigogiraoserrao rodrigogiraoserrao deleted the messages-control branch May 8, 2023 10:26
rodrigogiraoserrao added a commit that referenced this pull request May 8, 2023
Test now fully works, as of #2490.
willmcgugan added a commit that referenced this pull request May 8, 2023
* Add tests for #2484.

* Implement @on extension.

[skip ci]
Related issues: #2484.

* Changelog.

* Add missing @on test.

* Remove debug prints.

* Document changes.

* Update tests.

Test now fully works, as of #2490.

* Cache parsed selectors.

* Streamline exit condition.

* Fix typing.

* More succint wording.

* Document 'on' kwargs.

* Update src/textual/_on.py

Co-authored-by: Will McGugan <[email protected]>

* Update docs/guide/events.md

Co-authored-by: Will McGugan <[email protected]>

* Change 'on' API.

* Remove example code.

* Address feedback.

* Update src/textual/_on.py

Co-authored-by: Will McGugan <[email protected]>

* Address review feedback.

* Fix #2499.

* don't require control to be manually specified

* update docstring

* deleted words

---------

Co-authored-by: Will McGugan <[email protected]>
@@ -64,19 +64,32 @@ class FileSelected(Message, bubble=True):
`DirectoryTree` or in a parent widget in the DOM.
"""

def __init__(self, node: TreeNode[DirEntry], path: Path) -> None:
def __init__(
self, tree: DirectoryTree, node: TreeNode[DirEntry], path: Path
Copy link
Contributor

Choose a reason for hiding this comment

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

I realise this is a bit late now, but this will have broken DirectoryTree as it didn't update the use of the message later on in the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there an issue for this?
This means we need to add tests. 😬

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe it got fixed with 0.24.1 yesterday evening; I was just making a after-the-fact note. But I do think we may need to be careful as we go along with adding this sort of reference to all messages; sometimes the context might matter (in this case the message is a node-oriented message, so the node is the subject of the message and the node knows which tree it came from).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Sorry for the mess-up D:

This is an alias for [`FileSelected.tree`][textual.widgets.DirectoryTree.FileSelected.tree]
which is used by the [`on`][textual.on] decorator.
"""
return self.tree
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be done without the Tree reference by using:

return self.node.tree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

All widget events should have a control property Fix documentation for OptionList
3 participants