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

dynamic bindings #4516

Merged
merged 30 commits into from
May 18, 2024
Merged

dynamic bindings #4516

merged 30 commits into from
May 18, 2024

Conversation

willmcgugan
Copy link
Collaborator

Stab at dynamic bindings

Wedding present for @darrenburns

@willmcgugan willmcgugan marked this pull request as draft May 16, 2024 19:50
@willmcgugan willmcgugan marked this pull request as ready for review May 17, 2024 16:46
Copy link
Contributor

@TomJGooding TomJGooding left a comment

Choose a reason for hiding this comment

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

Just a few suggestions where I spotted some typos in the docs - hope you don't mind!

docs/guide/actions.md Outdated Show resolved Hide resolved
docs/guide/actions.md Outdated Show resolved Hide resolved
docs/guide/actions.md Outdated Show resolved Hide resolved
docs/guide/actions.md Outdated Show resolved Hide resolved
docs/guide/actions.md Outdated Show resolved Hide resolved
@TomJGooding
Copy link
Contributor

TomJGooding commented May 17, 2024

Sorry also just a quick question which I've seen asked before relating to dynamic bindings.

What would be the most practical way with this new feature of hiding bindings when an Input or TextArea is currently focussed? Here's a contrived example where lots of bindings obviously won't work while the Input is focussed.

from textual.app import App, ComposeResult
from textual.widgets import Input, Footer, Button


class ExampleApp(App):
    CSS = """
    Screen {
        align: center middle;
    }

    Input {
        width: 16;
    }
    """

    BINDINGS = [
        ("a", "notify_binding('a')", "A"),
        ("b", "notify_binding('b')", "B"),
        ("c", "notify_binding('c')", "C"),
        ("f1", "notify_binding('F1')", "F1"),
        ("f2", "notify_binding('F2')", "F2"),
        ("f3", "notify_binding('F3')", "F3"),
    ]

    def compose(self) -> ComposeResult:
        yield Button()
        yield Input(placeholder="Input")
        yield Footer()

    def action_notify_binding(self, key: str) -> None:
        self.notify(f"You pressed {key}")


if __name__ == "__main__":
    app = ExampleApp()
    app.run()

@TomJGooding
Copy link
Contributor

TomJGooding commented May 17, 2024

I wonder if an exception should be added for Ctrl+c to quit the app, and maybe other default actions? If you wanted to hide all (custom) bindings when a particular widget was focussed for example, you could easily forget to add this check.

@darrenburns
Copy link
Member

😍

@willmcgugan
Copy link
Collaborator Author

Sorry also just a quick question which I've seen asked before relating to dynamic bindings.

What would be the most practical way with this new feature of hiding bindings when an Input or TextArea is currently focussed? Here's a contrived example where lots of bindings obviously won't work while the Input is focussed.

The issue there is that inputs capture most keys with the Key event, which bypasses bindings.

You could do the following:

    def check_action(self, name, params):
        if name == "notify_binding" and isinstance(self.focused, Input):
            return False
        return True

But that does mean you would have to modify the app. Ideally there would be a mechanism for the widget itself to declare it will capture those keys.

@willmcgugan
Copy link
Collaborator Author

I wonder if an exception should be added for Ctrl+c to quit the app, and maybe other default actions? If you wanted to hide all (custom) bindings when a particular widget was focussed for example, you could easily forget to add this check.

I can't see that being much of an issue. As long as you default to return True. But maybe I have misunderstood. Can you give me some example code?

@willmcgugan willmcgugan merged commit 11e7e84 into main May 18, 2024
20 checks passed
@willmcgugan willmcgugan deleted the dynamic-bindings branch May 18, 2024 09:55
@TomJGooding
Copy link
Contributor

TomJGooding commented May 18, 2024

Perhaps just worth a note in the docs explaining that this also applies to builtin actions?

For example using the Input example above, let's say you had a long list of of 'printable' key bindings you wanted to hide. You might think you could do something like this and then not understand why you can't quit the app:

    def check_action(self, name, params):
        if isinstance(self.focused, Input):
            return False
        return True

@davep
Copy link
Contributor

davep commented May 18, 2024

Glancing over this just now, I wonder if #3690 is at least partially satisfied by this PR.

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.

4 participants