Action docs update#3456
Action docs update#3456holdenweb wants to merge 2 commits intoTextualize:mainfrom holdenweb:action-docs-update
Conversation
|
Hey Steve, I think I'd have an easier time reviewing your changes if you told us exactly what issues you had with the documentation. |
| ## Action methods | ||
|
|
||
| Action methods are methods on your app or widgets prefixed with `action_`. Aside from the prefix these are regular methods which you could call directly if you wished. | ||
| Actions are handled by _action methods_ of your app or widgets - methods whose names are prefixed with `action_`. These are regular methods which you can call directly, but normally they are triggered by actions. |
There was a problem hiding this comment.
This change is partly pedantry and partly a wish to add more context, but "aside from the prefix" seems completely redundant. It's quite probably a matter of taste and style preference.
| ``` | ||
|
|
||
| The `action_set_background` method is an action which sets the background of the screen. The key handler above will call this action if you press the ++r++ key. | ||
| The `action_set_background` method is an action method that sets the background of the screen. The key handler above will call this action if you press the ++r++ key. |
There was a problem hiding this comment.
A method isn't an action, and sloppy usage will inevitably lead to misunderstanding on the part of at least some readers. "Which" is replaced with "that" because a professional copy editor insisted a long time ago that "which" should only follow a comma, beginning a subordinate clause. You may prefer the comma.
| The `action_set_background` method is an action method that sets the background of the screen. The key handler above will call this action if you press the ++r++ key. | ||
|
|
||
| Although it is possible (and occasionally useful) to call action methods in this way, they are intended to be parsed from an _action string_. For instance, the string `"set_background('red')"` is an action string which would call `self.action_set_background('red')`. | ||
| Although it is possible (and occasionally useful) to call action methods directly in this way, they are more often parsed from an _action string_. For instance, the string `"set_background('red')"` is an action string which would call `action_set_background('red')`. |
There was a problem hiding this comment.
A re-reading convinces me this change was based on my misreading without the context of visible code, and with that context it looks like a nit-pick. I'll modify the PR to exclude it.
| !!! important | ||
|
|
||
| As much as they *look* like Python code, Textual does **not** call Python's `eval` function to compile action strings. | ||
| Though they *look* like Python code, Textual does **not** call Python's `eval` function to compile action strings. |
There was a problem hiding this comment.
This change is mostly for the benefit of readers whose first language isn't English. While such idiomatic uses are fine for exchange among native speakers, they increase the cognitive load unnecessarily for others.
| - the widget containing the renderable of which the action string is a part, | ||
| - the Screen containing that widget, or | ||
| - the App | ||
|
|
There was a problem hiding this comment.
This insertion is to immediately emphasise that actions are different from events, and processed differently. It's position seems a little incongruous, but then so does a description of action handling under a "Syntax" heading. In any case I feel this point should be emphasised. If I'm the only person who's ever found this problematic you can probably just stop reading this PR now. If included it should occur before "You can specify handling ..." below.
| - Actions may be followed by braces containing Python objects. For example, the action string `set_background("red")` will call `action_set_background("red")`. | ||
| - Actions may be prefixed with a _namespace_ (see below) followed by a dot. | ||
| - The name of an action (with or without a namespace) on its own will call the action method with no parameters in the given namespace, if any. For example, an action string of `"app.bell"` will call your app's `action_bell()` method. | ||
| - The action name may be followed by parentheses optionally containing a list of Python objects, which become parameters to the action method call. For example, the action string `set_background("red")` will first try to call `action_set_background("red")` as a widget method, then as a screen method, and finally as an app method. |
There was a problem hiding this comment.
This is simply a refactoring of the explanation. The first bullet explains parameterless actions while pointing out this is orthogonal to the presence of a namespace. The second demonstrates actions with parameters and explains the lookup sequence when no namespace is present. "Braces" was, of course, just plain wrong, so I presume it's a holdover from an earlier design.
| ### Parameters | ||
|
|
||
| If the action string contains parameters, these must be valid Python literals. Which means you can include numbers, strings, dicts, lists etc. but you can't include variables or references to any other Python symbols. | ||
| If the action string contains parameters, these must be valid Python literals. So you can include numbers, strings, dicts, lists etc. but NOT variables or references to any other Python symbols. |
There was a problem hiding this comment.
A simplifying rewording, I felt. Again, perhaps largely a matter of opinion.
|
Thanks for your comment, Rodrigo. I did indeed have issues with the documentation, though clearly I only have my own personal experience to go on. Having learned about events and event-handling, and being somewhat familiar with DOM concepts, I found that the different handling sequence for actions was insufficiently well-explained. In reviewing documentation I try to maintain the viewpoint of someone coming to the software for the first time, which isn't that far wrong in textual's case. I've added some comments to the changes, and will revert one change immediately. |
Thanks to #3456. Co-authored-by: Steve Holden <steve@holdenweb.com>
|
While we appreciate the time and effort it takes, it is hard for an external contributor to make non-trivial documentation changes because it is difficult to balance the voice of the documentation, which we want to be consistent, together with the information layout and presentation that suits best each reader. I've opened #3511 as a follow-up to this PR, making use of some of the more objective improvements that you made with this PR and I've added you as a co-author there. In the future, we encourage you to open (discussion) issues pointing out deficiencies in the documentation so that we can figure out the best way to fix those. Thanks for your understanding 🙏 |
|
@rodrigogiraoserrao You may have to elaborate on "inglorious". I doubt that's the term you were looking for! |
|
Sorry about that. We have a 🇵🇹 phrase that I tried to translate without success! |
|
Thanks very much for the feedback. I'll engage more closely on Discord before making a PR in future. |
3 participants
Please review the following checklist.
This is a fairly straightforward rewrite of the actions documentation, which clarifies a few issues that caused me problems.
It doesn't seem significant enough to merit a CHANGELOG entry.