Promote interactive Run action to group level#2414
Merged
ntjess merged 7 commits intopyqtgraph:masterfrom Sep 21, 2022
Merged
Conversation
Member
|
I think shortcuts not working if a parameter is hidden is fine; especially for an initial go. If people complain, we can certainly revisit! |
4cae4a6 to
f982c00
Compare
j9ac9k
reviewed
Sep 16, 2022
|
|
||
| ``sigActivated(self)`` is emitted when the button is clicked. | ||
|
|
||
| ============== ============================================================ |
Member
There was a problem hiding this comment.
Would you mind changing this docstring to numpy style docstring? We are hoping to migrate the rest of the repo to that, but it may be a while.
Also I think it should be
:class:`QIcon` and :class`QKeySequence`
Our docs are now smart enough to link to the upstream Qt docs for Qt types like that 👍🏻
0c27677 to
736bbd1
Compare
Save space by placing the button inside the nested group where possible. This has the added benefit of allowing a function to be run while parameters are collapsed
For now, simply remove positional arguments from consideration
- Add extra `action` arguments - Rename `FunctionParameterGroup` -> `ActionGroup` for clarity - Bugfix on setting icon in `action` arguments
736bbd1 to
279996b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Much less screen real estate is required if an


interactiveparameter's "Run" button is associated with the group rather than as a subchild (similar to thepenparameter):A very nice additional benefit is being able to run the function even when all parameters are not expanded:
Because "Run" buttons would no longer clutter a parameter and make unnecessary children, there is minimal drawback to making
[RunOptions.ON_CHANGED, RunOptions.ON_ACTION]the default instead ofRunOptions.ON_CHANGED.To facilitate this change, minor refactoring of
action's button meant more of its options (icon,shortcut, other button properties as desired) could be exposed as parameteropts.Note: The current
shortcutbehavior means an action only listens to the shortcut when any of its items are visible. So if a tree is hidden (or no parameter items were spawned), the shortcut doesn't work. There are many other ways of implementing parameter shortcuts, but this is the most pain-free (doesn't require the user to specify a widget parent inopts, doesn't depend on shortcut parent's life cycle). If this behavior is unintuitive, my next plan would be to force the user to provide ashortcutParent(or something similar) ifshortcutis specified. This is the widget whose focus determines whether the shortcut will be listening. The downside is if this object is deleted, it can lead to confusing behavior. However, it would allow the shortcut to be tied to the parameter itself, rather than an instantiated button. (I could also simply removeshortcutfrom the list of available options, but I find it useful enough)ActionParameterdocs to incorporate new options once the discussion regardingshortcutis settled.