Toolbars: allow passing additional html attributes to button <a> tags.

[martinpovolny]

No description provided.

[martinpovolny]

ping @dkorn

[zeari]

I couldnt really figure out how to use this.
Can I just set any attributes on buttons in a _center.rb file?

[martinpovolny]

Sorry ;-)

diff --git a/app/helpers/application_helper/toolbar/ems_container_center.rb b/app/helpers/application_helper/toolbar/ems_container_center.rb
index 6ff9900..299270d 100644
--- a/app/helpers/application_helper/toolbar/ems_container_center.rb
+++ b/app/helpers/application_helper/toolbar/ems_container_center.rb
@@ -16,6 +16,7 @@ class ApplicationHelper::Toolbar::EmsContainerCenter < ApplicationHelper::Toolba
           'fa fa-refresh fa-lg',
           N_('Refresh items and relationships related to this #{ui_lookup(:table=>"ems_container")}'),
           N_('Refresh items and relationships'),
+          :html_attributes => {'ng-click' => 'calSomeJavascriptStuff();'},
           :confirm => N_("Refresh items and relationships related to this \#{ui_lookup(:table=>\"ems_container\")}?")),
         separator,
         button(

[himdel]

I'm a bit afraid that allowing to add any html_attributes may be an overkill - sooner or later, somebody will try adding onclick or something similar, which would hopelessly break the code that's supposed to handle the click...

Probably better to allow just custom data-* attributes, and define, say, data-function to mean "when I click, that function will be called (by miqToolbarOnClick) and will get passed all the custom data (or the element itself, so that function can get the data out)".

[martinpovolny]

@himdel : I agree with the overkill. We need to figure out the usecases. Ping @pilhuhn.

I think that we need to be able to execute arbitrary javascript. And we need to be able to submit to the URL that the button would submit to + pass in additional data.

[martinpovolny]

Also related is the custom toolbar content #6320. There will be a need to add code to support the custom toolbar content. Such as collecting values from input fields. Etc.

[martinpovolny]

[martinpovolny]

@zeari : the example shows how the value of from the html_attributes is passed to the a tag on the button.

I think we shall go in a bit different direction as @himdel wrote, but I want to make sure I understand what you need before we change the toolbars.

[pilhuhn]

Yes, display of an input form with one or more fields that are then passed to the backend ruby code are a use case for me. And I would like to get this done by passing a definition of my form and not by coding it in html/js/css.
A definition could look like this on the button definition.
:form => [ :field => {:name => "bla", :label => "Timeout", :type=> numeric}]
When the button is hit, MiQ would see the :form tag, render it (like it does for the confirmation) and then after the user has clicked ok, would pass a map of name+value to the Controller#button as it is done with ids and so on.

[zeari]

@zeari : the example shows how the value of from the html_attributes is passed to the a tag on the button.

Thanks! I'll try that out.

[zeari]

@abonas

[miq-bot]

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

[pilhuhn]

@martinpovolny c8d922d illustrates my idea via code.
If this is cool, I can submit it as individual PR.

[himdel]

@pilhuhn that would need some more refactoring on miqToolbarOnClick so that it doesn't call miqJqueryRequest in such case and just pass the generated url fragment to the miqShowForm method - your example only works because prompt is magic and blocks everything - we probably want a patternfly-like modal with an actual html form instead..

[pilhuhn]

@himdel This is certainly not a/the final version, but for illustration purposes. One would probably also want to support more than 1 field.

[himdel]

Understood :) but.. to tie it back into the previous discussion, is the form case really that significant compared to the other possible customizations? What I'm trying to see is whether it really makes sense to add another special case to that code that works just for forms, or if it'd be better to go the more generic function name + data way, and handling that form case by simply setting function name to "miqShowForm".

[pilhuhn]

The form case allows to declaratively put this without the knowledge of the underlying javascript, which can/could then be exchanged transparently.

[himdel]

Yeah, but if we do have a predefined function to do that, it's the same..

[martinpovolny]

To utilize data-function with angular you need:

function callAngular(data) {
   ManageIQ.angular.scope.$apply(function() {
    ManageIQ.angular.scope[data.name].apply(ManageIQ.angular.scope, data.args);
  });
}

[himdel]

This is actually broken because new Function("foobar") returns a function that evaluates foobar and ..nothing, doesn't call it, whereas new Function("foobar()") would call it but can't pass in arguments...

Simple fix:

var fn = new Function("return " + button.data('function')); // eval - returns a function returning the right function
fn().call(button, button.data('functionData'));

And then any function name (without parentheses) will work in data-function.

[martinpovolny]

Updated. Thx!

[dkorn]

To utilize data-function with angular you need:

function callAngular(data) {
ManageIQ.angular.scope.$apply(function() {
ManageIQ.angular.scope[data.name].apply(ManageIQ.angular.scope, data.args);
});
}

@martinpovolny do I need to place this function in my controller/somewhere else?

[martinpovolny]

@dkorn : anywhere. I thought you just try to use is. It uses the scope saved in ManageIQ.angular.scope to call function data.name with args data.args

This is to call angular stuff from non-angular context (toolbar button click).

Adding it in a separate commit. Wait a sec...

[martinpovolny]

@dkorn: can you, please, update #7620 to work with this branch (after the last commit)?

If everything is ok, I'd like to merge this.

[dkorn]

@martinpovolny you tagged the wrong guy :) (its dkorn)
your last commit required a small change from my side but now its working for me 👍

[dkorn]

@martinpovolny please also update the example in the commit message

Example:

button(
    :ems_container_deployment,
    'pficon pficon-add-circle-o fa-lg',
    t = N_('Create #{ui_lookup(:table=>"ems_container")}'),
    t,
    :data => {
     'function' => 'miqCallAngular',
     'function-data' => '{ "name": "showListener", "args": [] }'}
  ),

[martinpovolny]

@h-kataria, @dclarizio : merge, please?

[martinpovolny]

@dkorn : updated the commit message

[miq-bot]

Checked commits martinpovolny/manageiq@d410d44~...754a318 with ruby 2.2.3, rubocop 0.37.2, and haml-lint 0.16.1
3 files checked, 0 offenses detected
Everything looks good. ⭐

[chessbyte]

@dclarizio is there a BZ for this?

[JPrause]

https://bugzilla.redhat.com/show_bug.cgi?id=1374830