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

Draft: Alt Command Toggle #2790

Closed
wants to merge 7 commits into from
Closed

Draft: Alt Command Toggle #2790

wants to merge 7 commits into from

Conversation

matthewRekos
Copy link

@matthewRekos matthewRekos commented Jul 20, 2023

Description

Alternative command feature in ability YAML which allows a user to flip a toggle when adding an operational link.

Type of change

How Has This Been Tested?

Mostly manual tests - would need advisement on UI element testing

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Wohoo! Your first PR -- thanks for contributing!

@matthewRekos matthewRekos self-assigned this Jul 26, 2023
Copy link
Contributor

@clenk clenk left a comment

Choose a reason for hiding this comment

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

What does the slider look like for abilities that don't have an alt_command? Ideally it doesn't appear.

I wonder if labels should go under command and alt_command instead of their own list. What happens if there's more than 2 labels? I assume any beyond the first 2 are ignored. What if there are no labels? What if there's only 1? Ideally we have default labels if they aren't present in the yaml.

@matthewRekos
Copy link
Author

  • If the labels aren't present then it defaults to Command1 and Command2 in the operations.html template.
  • If alt_command isn't given then the toggle doesn't appear. The UI element only appears if the alt_command is present.

Fixing the "1 label" use case now..

@matthewRekos
Copy link
Author

app/objects/c_executor.py now has the default labels and if the label length is 1 it'll add a "Command2 default. If you have more than 2 its just uses the first 2.

@sonarqubecloud
Copy link

Please retry analysis of this Pull-Request directly on SonarCloud.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

62.5% 62.5% Coverage
0.0% 0.0% Duplication

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

@matthewRekos
Copy link
Author

@clenk any progress?

@elegantmoose elegantmoose changed the title Alt Command Toggle Draft: Alt Command Toggle Sep 26, 2023
@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants