On 10/27/2010 04:14 PM, Jason Guiditta wrote:
> On Wed, 2010-10-27 at 14:37 -0400, Mohammed Morsi wrote:
>> eventually this should be expanded to select / disable select links
>>    depending on which instance actions are appropriate for the current
>>    instance state.
>> ---
>>   src/app/views/instance/index.haml |   18 ++++++++++++++++++
>>   1 files changed, 18 insertions(+), 0 deletions(-)
>>
> This js already exists in a slight different form in application.js
> ($.fn.buttonSensitivity).  You might want to take a look at that and see
> if you can just alter it a bit to work with the radio buttons (or maybe
> delete it, looks a little clunky on quick glance).  Note this worked
> before the checkbox-radio button switch, though it may do more than we
> need now.  At the least, I would suggest making the toggle_actions
> function in the style of the other functions in application.js in the
> Aggregator namespace.
>
> Actually, looking at that existing js, we can probably just blow it
> away, but that doesn't have to be part of this patch, as long as it
> isn't being called anymore.

Ah ok I missed this and agree can use some reworking to simplify. Are we 
sure we want to include this in application.js though, as this only 
applies to limited content on a few pages on the site (pools, templates, 
and instances index). Plus the actual content of the side bars for each 
of those sections will be somewhat different.


>> diff --git a/src/app/views/instance/index.haml 
>> b/src/app/views/instance/index.haml
>> index 8024a2f..949b3cf 100644
>> --- a/src/app/views/instance/index.haml
>> +++ b/src/app/views/instance/index.haml
>> @@ -1,3 +1,21 @@
>> +:javascript
>> +  $(document).ready(function() {
>> +    // toggle which action links are enabled depending on selected action
>> +    // TODO right now actions which are enabled / disabled are fixed, but
>> +    // should depend on which instance is selected
>> +    function toggle_actions(){
>> +      if($("input[name='id[]']:checked").length == "0"){
>> +        $("li.shutdown").addClass('disabled');
>> +        $("li.delete").addClass('disabled');
>> +      }else{
>> +        $("li.shutdown").removeClass('disabled');
>> +        $("li.delete").removeClass('disabled');
>> +      }
>> +    }
> This whole method could be simplified to something like:
> var Aggregator = {
>    toggle_actions: function() {
>      $("input[name='id[]']:checked").click(function() {
>        $("li.shutdown").toggleClass('disabled');
>        $("li.delete").toggleClass('disabled');
>      }
>    }
> }
>> +    toggle_actions();
> This changes to Aggregator.toggle_actions()
>> +    $("input[name='id[]']").click(toggle_actions);
> And then the above line could go.
This doesn't fully work since when the page is initially loaded none of 
the radio boxes are checked but the links need to be enabled (for the 
non-js case). Upon clicking an instance the disabled class will be 
toggled so that the links are disabled. We can either go the original 
route and explicitly check the number of selected items or just have the 
first instance be selected by default.

I'm reworking this patch to resolve this and take care of some edge 
cases, and writing a followup patch adding a few tests for the 1st patch 
in this series. Will send both out in a bit.

    -Mo
_______________________________________________
deltacloud-devel mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/deltacloud-devel

Reply via email to