-
Notifications
You must be signed in to change notification settings - Fork 357
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
Automate embedded methods UI #2180
Automate embedded methods UI #2180
Conversation
end | ||
|
||
session[:edit] = @edit | ||
end | ||
|
||
def pokus | ||
"pokus method" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I forgot to remove it, thank you for comment
if @edit[:action] == 'miq_ae_method_copy' | ||
at_tree_select(:namespace) | ||
else | ||
at_tree_select(:method) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is repeated multiple times, maybe consider extracting to a common method?
app/helpers/automate_tree_helper.rb
Outdated
@@ -1,30 +1,51 @@ | |||
module AutomateTreeHelper | |||
def at_tree_select_toggle(edit_key) | |||
build_ae_tree(:automate, :automate_tree) | |||
edit_key == :method ? build_ae_tree(:ae_methods, :automate_tree) : build_ae_tree(:automate, :automate_tree) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imho this method is a bit too long, how about breaking it down to multiple smaller methods?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made it a little smaller, but we are planning to replace this old dialog with a new component and this helper will disappear. So I think that is not necessary to refactor it at this time. (https://www.pivotaltracker.com/n/projects/1613907/stories/151605408)
@pkomanek Please include a screenshot in description. |
@pkomanek |
@mkanoor |
This pull request is not mergeable. Please rebase and repush. |
@pkomanek Please rebase |
Checked commits pkomanek/manageiq-ui-classic@626b399~...ad57ff2 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0 |
Works for me. I found a problematic behavior for copying automate instances and methods while testing this. The UI seems to be stuck for a while w/o the spinner. Actually the spinner stops rotating while is seems something is being done. |
Tested looks good. 👍 |
Adding the UI for embedded methods in automate
It's based on the following PRs:
ManageIQ/manageiq#14286
ManageIQ/manageiq-automation_engine#16
ManageIQ/manageiq#14847
Method preview from explorer:
Embedded methods in method edit:
Embedded method select: