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.

Sign up for GitHub

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

Jump to bottom

Extend @on decorator to filter matchable attributes #2498

Merged
merged 27 commits into from
May 8, 2023
Merged

Extend @on decorator to filter matchable attributes #2498

merged 27 commits into from
May 8, 2023

Conversation

rodrigogiraoserrao
Copy link
Contributor

@rodrigogiraoserrao rodrigogiraoserrao commented May 5, 2023 •
edited
Loading

The decorator @on now accepts keyword arguments to provide selectors for attributes that the message has whitelisted in ON_MATCHABLE_ATTRIBUTES.

This will close #2484.

@rodrigogiraoserrao rodrigogiraoserrao self-assigned this May 5, 2023
@rodrigogiraoserrao rodrigogiraoserrao added enhancement New feature or request Task labels May 5, 2023
@rodrigogiraoserrao rodrigogiraoserrao mentioned this pull request May 5, 2023
Closed
@rodrigogiraoserrao
Copy link
Contributor Author

Tests will pass as soon as I merge #2490.

@rodrigogiraoserrao rodrigogiraoserrao marked this pull request as ready for review May 8, 2023 10:28
rodrigogiraoserrao
rodrigogiraoserrao commented May 8, 2023
View reviewed changes
docs/guide/events.md
Copy link
Contributor Author

@rodrigogiraoserrao rodrigogiraoserrao May 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@willmcgugan still trying to strike a balance between my personal verbose style and the intended style for the docs.
Let me know how this looks.

willmcgugan
willmcgugan requested changes May 8, 2023
View reviewed changes
Copy link
Collaborator

@willmcgugan willmcgugan left a comment

Choose a reason for hiding this comment

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

The dead code in the example is a blocker. Either we come up with a working example, or we don't bother with an example for this.

docs/examples/events/on_decorator03.py Outdated Show resolved Hide resolved
docs/examples/events/on_decorator03.py Outdated Show resolved Hide resolved
src/textual/_on.py Outdated Show resolved Hide resolved
docs/guide/events.md Outdated Show resolved Hide resolved
src/textual/message_pump.py Outdated Show resolved Hide resolved
rodrigogiraoserrao and others added 2 commits May 8, 2023 13:25
@rodrigogiraoserrao @willmcgugan
Update src/textual/_on.py
64f1253 
Co-authored-by: Will McGugan <[email protected]>
@rodrigogiraoserrao @willmcgugan
Update docs/guide/events.md
6b08b3c 
Co-authored-by: Will McGugan <[email protected]>
@willmcgugan willmcgugan mentioned this pull request May 8, 2023
Merged
@rodrigogiraoserrao rodrigogiraoserrao changed the title Extend @on decorator to filter arbitrary attributes Extend @on decorator to filter matchable attributes May 8, 2023
@rodrigogiraoserrao
Copy link
Contributor Author

Changed the API so that now the **kwargs can only match attributes that have been whitelisted in the message's ON_MATCHABLE_ATTRIBUTES classvar.

willmcgugan
willmcgugan requested changes May 8, 2023
View reviewed changes
src/textual/message_pump.py Outdated Show resolved Hide resolved
src/textual/message_pump.py Outdated Show resolved Hide resolved
src/textual/message.py Outdated Show resolved Hide resolved
docs/guide/events.md Outdated Show resolved Hide resolved
src/textual/_on.py Outdated Show resolved Hide resolved
src/textual/_on.py Outdated Show resolved Hide resolved
willmcgugan
willmcgugan approved these changes May 8, 2023
View reviewed changes
Copy link
Collaborator

@willmcgugan willmcgugan left a comment

Choose a reason for hiding this comment

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

LGTM

rodrigogiraoserrao
rodrigogiraoserrao commented May 8, 2023
View reviewed changes
Copy link
Contributor Author

@rodrigogiraoserrao rodrigogiraoserrao left a comment

Choose a reason for hiding this comment

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

LGTM

@willmcgugan willmcgugan merged commit 855c90d into main May 8, 2023
@willmcgugan willmcgugan deleted the on-kwargs branch May 8, 2023 16:30
@renovate renovate bot mentioned this pull request Dec 1, 2023
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@willmcgugan willmcgugan willmcgugan approved these changes

@davep davep Awaiting requested review from davep

@darrenburns darrenburns Awaiting requested review from darrenburns

Assignees

@rodrigogiraoserrao rodrigogiraoserrao

Labels
enhancement New feature or request Task
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Extend decorator on to specify non-control attributes to match.
2 participants
@rodrigogiraoserrao @willmcgugan