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

Action button #3097

Merged
merged 15 commits into from
Aug 1, 2024
Merged

Action button #3097

merged 15 commits into from
Aug 1, 2024

Conversation

shroffk
Copy link
Member

@shroffk shroffk commented Jul 30, 2024

A clean version of the PR #3017

Resolves the merge conflicts from that branch
Fixes the conversion of embedded scripts
Fixing the the broken script editing
Adds a preference for removing the EDM "close" button in the auto converted screens

@kasemir
Copy link
Collaborator

kasemir commented Jul 30, 2024

When the list of available action types was fixed, they were offered in a known and fixed order in the actions dialog:

Screenshot 2024-07-30 at 2 29 00 PM

Now that they're provided via SPI and thus discovered in a random order, is there a way to get a predictable order?
For instance, "Open Display" should certainly remain at the top, with maybe "Write PV" as second.
A locally added action type of "Open Local XYZ App" shouldn't suddenly appear on top.

Could add a numeric "order" or "priority" to the action info, where the existing ones use values Open Display 10, Write PV 20, ... Present them sorted by that "priority". That allows a local addition to use 100 to appear at the end, but also 15 to sneak into the middle.

@shroffk
Copy link
Member Author

shroffk commented Jul 30, 2024

Ah yes, the ordered actions should be supported.
@georgweiss do you want me to add the solution as suggested by Kay

@georgweiss
Copy link
Collaborator

Fine with me, though personally I would prefer alphabetical order...

@kasemir
Copy link
Collaborator

kasemir commented Jul 30, 2024

Alphabetical order isn't totally crazy.
Certainly predictable.
Would place "Execute script" on top, though, and we really don't want to encourage that the first thing people try is executing a script. "Open Display" should be the most frequently used option, and placing that on top would help.

@georgweiss
Copy link
Collaborator

georgweiss commented Jul 30, 2024

I find a order property a bit ... well, vague. New actions could simple squeeze themselves into any position. What if someone defines order -10?
I like the predictability of alphabetical order.
Plus: I really hope nobody selects Execute script only because it is on top...

@kasemir
Copy link
Collaborator

kasemir commented Jul 30, 2024

New actions could simple squeeze themselves into any position. What if someone defines order -10?

Exactly. If you do want to appear before "Open Display" at 10, you can use 9 or 0 or -10.

Yes, people will pick the first option simply because it's the first one.

But whatever, do what you want, I'll stay out of "actions" from now on.

Copy link
Collaborator

@georgweiss georgweiss left a comment

Choose a reason for hiding this comment

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

Execute script seems to work as expected now

@shroffk shroffk merged commit 0960815 into master Aug 1, 2024
2 checks passed
@shroffk shroffk deleted the action_button branch October 14, 2024 17:40
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.

3 participants