Hi, I'm sending again updated patch which is applicable on current next branch (older patch isn't).
Datatables plugin (used for images and instances listing) has some issues when JS is off and pagination is server-side, this patch replaces it with simpler/cleaner HTML table which works better without JS.
From: Jan Provaznik jprovazn@redhat.com
Replaced Datatables plugin with simpler HTML table because some issues when JS is off. Major issue with Datatables is that it doesn't degrades gracefully when JS is off and server-side pagination.
Images and instances are now displayed by HTML table (there is new helper 'paginated_table'), this table keeps all common functions like sorting, paginating, searching and code is cleaner. Works with JS on or off. --- src/app/controllers/image_controller.rb | 55 +++------------- src/app/controllers/instance_controller.rb | 92 +++++++++++++++------------ src/app/controllers/pool_controller.rb | 57 ++++------------- src/app/helpers/application_helper.rb | 70 +++++++++++++++++++++ src/app/models/image.rb | 18 ------ src/app/models/instance.rb | 13 ---- src/app/views/image/_images.haml | 13 ++++ src/app/views/image/show.haml | 17 +++++ src/app/views/image/show.html.erb | 83 ------------------------- src/app/views/instance/_instances.haml | 17 +++++ src/app/views/instance/index.html.erb | 4 +- src/app/views/instance/new.html.erb | 72 ++++++++-------------- src/app/views/instance/select_image.haml | 4 + src/app/views/instance/show.html.erb | 29 +--------- src/app/views/layouts/aggregator.haml | 1 + src/app/views/pool/list.haml | 3 + src/app/views/pool/list.html.erb | 38 ----------- src/public/javascripts/application.js | 35 +++++++++++ src/public/stylesheets/dcloud.css | 79 ++++++++++++++++++++++++ 19 files changed, 346 insertions(+), 354 deletions(-) create mode 100644 src/app/views/image/_images.haml create mode 100644 src/app/views/image/show.haml delete mode 100644 src/app/views/image/show.html.erb create mode 100644 src/app/views/instance/_instances.haml create mode 100644 src/app/views/instance/select_image.haml create mode 100644 src/app/views/pool/list.haml delete mode 100644 src/app/views/pool/list.html.erb
diff --git a/src/app/controllers/image_controller.rb b/src/app/controllers/image_controller.rb index d7858b1..b75f692 100644 --- a/src/app/controllers/image_controller.rb +++ b/src/app/controllers/image_controller.rb @@ -26,57 +26,24 @@ class ImageController < ApplicationController end
def show - # FIXME: check on privilege IMAGE_VIEW which currently doesn't exist - #require_privilege(Privilege::POOL_VIEW, @pool) - end - - def images_paginate - # FIXME: check on privilege IMAGE_VIEW which currently doesn't exist - #require_privilege(Privilege::POOL_VIEW, @pool) - - # datatables sends pagination in format: - # iDisplayStart - start index - # iDisplayLength - num of recs - # => we need to count page num - page = params[:iDisplayStart].to_i / Image::per_page - - if params[:mode].to_s == 'simple' - simple_mode = true - cols = Image::COLUMNS_SIMPLE - default_order_col = 1 - else - cols = Image::COLUMNS - simple_mode = false - default_order_col = 2 + if params[:create_instance] + redirect_to :controller => 'instance', :action => 'new', 'instance[image_id]' => (params[:ids] || []).first end
- order_col_rec = cols[params[:iSortCol_0].to_i] - order_col = cols[default_order_col] unless order_col_rec && order_col_rec[:opts][:searchable] - order = order_col[:id] + " " + (params[:sSortDir_0] == 'desc' ? 'desc' : 'asc') + require_privilege(Privilege::IMAGE_VIEW)
- @images = Image.search_filter(params[:sSearch], Image::SEARCHABLE_COLUMNS).paginate( - :page => page + 1, + @order_dir = params[:order_dir] == 'desc' ? 'desc' : 'asc' + @order = params[:order] || 'name' + @images = Image.search_filter(params[:search], Image::SEARCHABLE_COLUMNS).paginate( + :page => params[:page] || 1, + :order => @order + ' ' + @order_dir, :include => :instances, - :order => order, :conditions => {:provider_id => nil} )
- expand_button_html = "<img src='/images/dir_closed.png'>" - - data = @images.map do |i| - if simple_mode - [i.id, i.name, i.architecture, i.instances.size] - else - [i.id, expand_button_html, i.name, i.architecture, i.instances.size, "TODO: some description here?"] - end + if request.xhr? and params[:partial] + render :partial => 'images' + return end - - render :json => { - :sEcho => params[:sEcho], - :iTotalRecords => @images.total_entries, - :iTotalDisplayRecords => @images.total_entries, - :aaData => data - } end - end diff --git a/src/app/controllers/instance_controller.rb b/src/app/controllers/instance_controller.rb index 5664ec5..feb9a53 100644 --- a/src/app/controllers/instance_controller.rb +++ b/src/app/controllers/instance_controller.rb @@ -23,58 +23,68 @@ require 'util/condormatic'
class InstanceController < ApplicationController before_filter :require_user + layout :layout
- def index + def layout + return "aggregator" unless request.xhr? end
- def paginated - # datatables sends pagination in format: - # iDisplayStart - start index - # iDisplayLength - num of recs - # => we need to count page num - page = params[:iDisplayStart].to_i / Instance::per_page - - order_col_rec = Instance::COLUMNS[params[:iSortCol_0].to_i] - order_col = Instance::COLUMNS[2] unless order_col_rec && order_col_rec[:opts][:searchable] - order = order_col[:id] + " " + (params[:sSortDir_0] == 'desc' ? 'desc' : 'asc') - - # FIXME only return those instances in pools which user has instance_view - @instances = Instance.search_filter(params[:sSearch], Instance::SEARCHABLE_COLUMNS).paginate( - :page => page + 1, - :order => order + def index + require_privilege(Privilege::INSTANCE_VIEW) + + @order_dir = params[:order_dir] == 'desc' ? 'desc' : 'asc' + @order = params[:order] || 'name' + @instances = Instance.search_filter(params[:search], Instance::SEARCHABLE_COLUMNS).paginate( + :page => params[:page] || 1, + :order => @order + ' ' + @order_dir )
- recs = @instances.map do |i| - [ - i.id, - i.get_action_list.map {|action| "<a href="#{url_for :controller => "instance", :action => "instance_action", :id => i.id, :instance_action => action}">#{action}</a>"}.join(" | "), - i.name, - i.state, - i.hardware_profile.name, - i.image.name, - i.cloud_account.nil? ? "" : i.cloud_account.provider.name, - i.cloud_account.nil? ? "" : i.cloud_account.name - ] + if request.xhr? and params[:partial] + render :partial => 'instances' + return end - - render :json => { - :sEcho => params[:sEcho], - :iTotalRecords => @instances.total_entries, - :iTotalDisplayRecords => @instances.total_entries, - :aaData => recs - } end
- # Right now this is essentially a duplicate of PoolController#show, - # but really it should be a single instance should we decide to have a page - # for that. Redirect on create was all that brought you here anyway, so - # should be unused for the moment. - def show - @instances = Instance.find(:all, :conditions => {:pool_id => params[:id]}) - @pool = Pool.find(params[:id]) - require_privilege(Privilege::INSTANCE_VIEW,@pool) + def select_image + if params[:select] + redirect_to :action => 'new', 'instance[image_id]' => (params[:ids] || []).first + end + + require_privilege(Privilege::IMAGE_VIEW) + @order_dir = params[:order_dir] == 'desc' ? 'desc' : 'asc' + @order = params[:order] || 'name' + @images = Image.search_filter(params[:search], Image::SEARCHABLE_COLUMNS).paginate( + :page => params[:page] || 1, + :order => @order + ' ' + @order_dir, + :conditions => {:provider_id => nil} + ) + + if request.xhr? and params[:partial] + render :partial => 'image/images' + return + end end
+ ## Right now this is essentially a duplicate of PoolController#show, + # # but really it should be a single instance should we decide to have a page + # # for that. Redirect on create was all that brought you here anyway, so + # # should be unused for the moment. + #def show + # require_privilege(Privilege::INSTANCE_VIEW,@pool) + # @pool = Pool.find(params[:id]) + # @order_dir = params[:order_dir] == 'desc' ? 'desc' : 'asc' + # @order = params[:order] || 'name' + # @instances = Instance.search_filter(params[:search], Instance::SEARCHABLE_COLUMNS).paginate( + # :page => params[:page] || 1, + # :order => @order + ' ' + @order_dir, + # :conditions => {:pool_id => @pool.id} + # ) + # if request.xhr? and params[:partial] + # render :partial => 'instances' + # return + # end + #end + def new @instance = Instance.new(params[:instance]) require_privilege(Privilege::INSTANCE_MODIFY, @instance.pool) if @instance.pool diff --git a/src/app/controllers/pool_controller.rb b/src/app/controllers/pool_controller.rb index ad2d42b..19effa9 100644 --- a/src/app/controllers/pool_controller.rb +++ b/src/app/controllers/pool_controller.rb @@ -40,7 +40,20 @@ class PoolController < ApplicationController # Go to condor and sync the database to the real instance states condormatic_instances_sync_states @pool.reload - @instances = @pool.instances + + @order_dir = params[:order_dir] == 'desc' ? 'desc' : 'asc' + @order = params[:order] || 'name' + + @instances = Instance.search_filter(params[:search], Instance::SEARCHABLE_COLUMNS).paginate( + :page => params[:page] || 1, + :order => @order + ' ' + @order_dir, + :conditions => {:pool_id => @pool.id} + ) + + if request.xhr? and params[:partial] + render :partial => 'instance/instances' + return + end end
def hardware_profiles @@ -90,48 +103,6 @@ class PoolController < ApplicationController def delete end
- def instances_paginate - @pool = Pool.find(params[:id]) - require_privilege(Privilege::POOL_VIEW, @pool) - - # datatables sends pagination in format: - # iDisplayStart - start index - # iDisplayLength - num of recs - # => we need to count page num - page = params[:iDisplayStart].to_i / Instance::per_page - - order_col_rec = Instance::COLUMNS[params[:iSortCol_0].to_i] - order_col = Instance::COLUMNS[2] unless order_col_rec && order_col_rec[:opts][:searchable] - order = order_col[:id] + " " + (params[:sSortDir_0] == 'desc' ? 'desc' : 'asc') - - @instances = Instance.search_filter(params[:sSearch], Instance::SEARCHABLE_COLUMNS).paginate( - :page => page + 1, - :order => order, - :conditions => {:pool_id => @pool.id} - ) - - recs = @instances.map do |i| - [ - i.id, - i.get_action_list.map {|action| "<a href="#{url_for :controller => "instance", :action => "instance_action", :id => i.id, :instance_action => action}">#{action}</a>"}.join(" | "), - i.name, - i.state, - i.hardware_profile.name, - i.image.name, - i.cloud_account.nil? ? "" : i.cloud_account.provider.name, - i.cloud_account.nil? ? "" : i.cloud_account.name - ] - end - - render :json => { - :sEcho => params[:sEcho], - :iTotalRecords => @instances.total_entries, - :iTotalDisplayRecords => @instances.total_entries, - :aaData => recs - } - end - - def accounts_for_pool @pool = Pool.find(params[:pool_id]) require_privilege(Privilege::ACCOUNT_VIEW,@pool) diff --git a/src/app/helpers/application_helper.rb b/src/app/helpers/application_helper.rb index f7b96af..f40f4e0 100644 --- a/src/app/helpers/application_helper.rb +++ b/src/app/helpers/application_helper.rb @@ -58,4 +58,74 @@ module ApplicationHelper end day_str + hours_to_seconds end + + def column_header(column, order, order_dir, check_all) + next_dir = order_dir.to_s == 'asc' ? 'desc' : 'asc' + if order and column[:id] == order + dir = order_dir.to_s == 'desc' ? 'desc' : 'asc' + cls = "ordercol #{dir}" + else + cls = nil + end + + if column[:sortable] + label = link_to( + column[:header], + {:action => controller.action_name, :partial => true, :order => column[:id], :order_dir => next_dir, :search => params[:search]}, + :class => cls) + elsif check_all.to_s == column[:id] + label = check_box_tag 'check_all' + else + label = column[:header] + end + + content_tag 'th', label + end + + def paginated_table(html_id, columns, data, opts = {}) + search_url = url_for(:partial => true, :order => opts[:order], :order_dir => opts[:order_dir]) + extend_table_js = "<script type="text/javascript">extend_table('#{html_id}');#{opts[:load_callback]};</script>" + extend_sfield_js = "<script type="text/javascript">extend_table_search_field('#{html_id}', '#{search_url}')</script>" + + rows = data.map do |rec| + if block_given? + capture_haml{yield rec} + else + content_tag 'tr', :class => cycle('even', 'odd') do + columns.map { |c| content_tag 'td', rec[c[:id]] } + end + end + end + + header_cols = columns.map {|c| column_header(c, opts[:order], opts[:order_dir], opts[:check_all])} + + table = content_tag 'table' do + content_tag('thead', content_tag('tr', header_cols)) + content_tag('tbody', rows) + end + + search = content_tag 'div', :class => 'search_field' do + 'Search ' + text_field_tag('search', params[:search], :size => 10) + end + + header = content_tag 'div', :class => 'header' do + search + content_tag('div', opts[:title], :class => 'title') + end + + footer = content_tag 'div', :class => 'footer' do + will_paginate(data, :params => {:partial => true}).to_s + + page_entries_info(data).to_s + end + + ajax_content = table + footer + extend_table_js + + if params[:partial] and request.xhr? + ajax_content + else + content_tag 'div', :id => html_id, :class => "dtable #{opts[:class]}" do + header + + content_tag('div', ajax_content, :class => 'wrapper') + + extend_sfield_js + end + end + end end diff --git a/src/app/models/image.rb b/src/app/models/image.rb index 9df4185..f3304cf 100644 --- a/src/app/models/image.rb +++ b/src/app/models/image.rb @@ -48,24 +48,6 @@ class Image < ActiveRecord::Base
validates_presence_of :architecture, :if => :provider
- # used to get sorting column in controller and in view to generate datatable definition and - # html table structure - COLUMNS = [ - {:id => 'id', :header => '<input type="checkbox" id="image_id_all" onclick="checkAll(event)">', :opts => {:checkbox_id => 'image_id', :searchable => false, :sortable => false, :width => '1px', :class => 'center'}}, - {:id => 'expand_button', :header => '', :opts => {:searchable => false, :sortable => false, :width => '1px'}}, - {:id => 'name', :header => 'Name', :opts => {:width => "30%"}}, - {:id => 'architecture', :header => 'Architecture', :opts => {:width => "10%"}}, - {:id => 'instances', :header => 'Instances', :opts => {:sortable => false, :width => "10%"}}, - {:id => 'details', :header => '', :opts => {:sortable => false, :visible => false, :searchable => false}}, - ] - - COLUMNS_SIMPLE = [ - {:id => 'id', :header => '', :opts => {:visible => false, :searchable => false, :sortable => false, :width => '1px', :class => 'center'}}, - {:id => 'name', :header => 'Name', :opts => {:width => "50%"}}, - {:id => 'architecture', :header => 'Architecture', :opts => {:width => "30%"}}, - {:id => 'instances', :header => 'Instances', :opts => {:sortable => false, :width => "20%"}}, - ] - SEARCHABLE_COLUMNS = %w(name architecture)
def provider_image? diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb index 6be4931..0212e24 100644 --- a/src/app/models/instance.rb +++ b/src/app/models/instance.rb @@ -59,19 +59,6 @@ class Instance < ActiveRecord::Base STATES = [STATE_NEW, STATE_PENDING, STATE_RUNNING, STATE_SHUTTING_DOWN, STATE_STOPPED, STATE_CREATE_FAILED]
- # used to get sorting column in controller and in view to generate datatable definition and - # html table structure - COLUMNS = [ - {:id => 'id', :header => '<input type="checkbox" id="image_id_all" onclick="checkAll(event)">', :opts => {:checkbox_id => 'image_id', :searchable => false, :sortable => false, :width => '1px', :class => 'center'}}, - {:id => 'actions', :header => 'Actions', :opts => {:width => "10%", :sortable => false}}, - {:id => 'name', :header => 'Name', :opts => {:width => "25%"}}, - {:id => 'state', :header => 'State', :opts => {:width => "10%"}}, - {:id => 'hwprofile', :header => 'HW profile', :opts => {:width => "15%"}}, - {:id => 'template', :header => 'Template', :opts => {:sortable => false, :width => "15%"}}, - {:id => 'provider', :header => 'Provider', :opts => {:width => "10%"}}, - {:id => 'account', :header => 'Account', :opts => {:width => "10%"}}, - ] - SEARCHABLE_COLUMNS = %w(name state)
validates_inclusion_of :state, diff --git a/src/app/views/image/_images.haml b/src/app/views/image/_images.haml new file mode 100644 index 0000000..0ef78d9 --- /dev/null +++ b/src/app/views/image/_images.haml @@ -0,0 +1,13 @@ +- columns = [ | + {:id => 'id', :header => ''}, | + {:id => 'name', :header => 'Name', :sortable => true}, | + {:id => 'architecture', :header => 'Architecture', :sortable => true}, | + {:id => 'instances', :header => 'Instances'}, | +] | + += paginated_table('images_table', columns, @images, {:load_callback => 'table_loaded()', :order => @order, :order_dir => @order_dir, :title => 'List of templates', :check_all => 'id'}) do |rec| + %tr{:class => "#{cycle('even', 'odd')}"} + %td= check_box_tag 'ids[]', rec[:id] + %td{:class => 'image_name'}= rec[:name] + %td= rec[:architecture] + %td= rec[:instances] diff --git a/src/app/views/image/show.haml b/src/app/views/image/show.haml new file mode 100644 index 0000000..5720443 --- /dev/null +++ b/src/app/views/image/show.haml @@ -0,0 +1,17 @@ +- content_for :scripts do + :javascript + function check_checkboxes() { + var checked_count = $('input[name="ids[]"]:checked', $("#images_table")).length; + $('input[name="create_instance"]').attr('disabled', checked_count != 1); + } + function table_loaded() { + //$("input[name='ids[]']", $("#images_table")).change(function() {console.warn('x');check_checkboxes()}); + //FIXME: this doesn't work in 100%, but don't know better way + $("#images_table").click(function() {check_checkboxes()}); + check_checkboxes(); + } + +- form_tag({:action => 'show'}, {:class => 'dtable_form'}) do + .action_buttons + = submit_tag "Create instance", :name => "create_instance" + = render :partial => 'images' diff --git a/src/app/views/image/show.html.erb b/src/app/views/image/show.html.erb deleted file mode 100644 index 5a7cb53..0000000 --- a/src/app/views/image/show.html.erb +++ /dev/null @@ -1,83 +0,0 @@ -<script type="text/javascript"> - - function fnOpenClose() { - $('td img', dataTable_images_table.fnGetNodes() ).each( function () { - $(this).click( function () { - var nTr = this.parentNode.parentNode; - if ( this.src.match('dir_open') ) - { - /* This row is already open - close it */ - this.src = "/images/dir_closed.png"; - /* fnClose doesn't do anything for server-side processing - do it ourselves :-) */ - var nRemove = $(nTr).next()[0]; - nRemove.parentNode.removeChild( nRemove ); - } - else - { - /* Open this row */ - this.src = "/images/dir_open.png"; - dataTable_images_table.fnOpen( nTr, dataTable_images_table.fnGetData(nTr)[5], 'details' ); - } - }); - }); - } - - function getCheckedRows() { - return $('input[name="image_id[]"]:checked', dataTable_images_table).length; - } - - function showButtons() { - getCheckedRows(); - $('input[name="create_image"]').attr('disabled', getCheckedRows() != 1); - } - - function checkRow(ev) { - var box = $('input[name="image_id[]"]', ev.target.parentNode); - if ($(ev.target).attr('name') != "image_id[]") - box.attr('checked', !box.attr('checked')); - showButtons(); - } - - function checkAll(ev) { - $('input[name="image_id[]"]', dataTable_images_table).attr('checked', $(ev.target).attr('checked')) - } - - function drawCallback() { - $('#image_id_all').attr('checked', false); - fnOpenClose(); - } - - function createInstance() { - var selected_row = $('input[name="image_id[]"]:checked', dataTable_images_table); - location.href = '<%= url_for(:controller => "instance", :action => "new") %>?instance[image_id]=' + selected_row.attr('value'); - } -</script> - -<%= datatable( - Image::COLUMNS.map {|c| c[:opts]}, - { - :table_dom_id => 'images_table', - :per_page => Image::per_page, - :sort_by => "[2, 'asc']", - :serverside => true, - :ajax_source => url_for(:controller => 'image', :action => 'images_paginate'), - :append => ".fnSetFilteringDelay()", - :persist_state => false, - :click_callback => "function(ev) {checkRow(ev);}", - :draw_callback => "drawCallback", - } -) %> - <div style="padding:20px"> - <input type="button" value="Create instance" name="create_image" disabled=true onclick="createInstance()"> - </div> - <table class="datatable display" id="images_table"> - <thead> - <tr> - <% Image::COLUMNS.each do |c| %> - <%= "<th>#{c[:header]}</th>" %> - <% end %> - </tr> - </thead> - <tbody> - </tbody> -</table> diff --git a/src/app/views/instance/_instances.haml b/src/app/views/instance/_instances.haml new file mode 100644 index 0000000..cc5a2c2 --- /dev/null +++ b/src/app/views/instance/_instances.haml @@ -0,0 +1,17 @@ +- columns = [ | + {:id => 'id', :header => ''}, | + {:id => 'name', :header => 'Name', :sortable => true}, | + {:header => 'Details'}, | + {:id => 'template', :header => 'Template'}, | + {:id => 'state', :sortable => true, :header => 'State'}, | + {:id => 'time_last_running', :header => 'Time last running'}, | +] | + += paginated_table('instances_table', columns, @instances, {:order => @order, :order_dir => @order_dir, :title => 'List of instances', :check_all => 'id'}) do |rec| + %tr{:class => "#{cycle('even', 'odd')}"} + %td= check_box_tag 'ids[]', rec.id + %td= rec.name + %td Details + %td= rec.image.name + %td= rec.state + %td= rec.time_last_running diff --git a/src/app/views/instance/index.html.erb b/src/app/views/instance/index.html.erb index c19c3fa..e57f78c 100644 --- a/src/app/views/instance/index.html.erb +++ b/src/app/views/instance/index.html.erb @@ -1 +1,3 @@ -<%= render :partial => 'list' %> +<% form_tag({:action => 'instance_action'}, {:class => 'dtable_form'}) do %> + <%= render :partial => 'instance/instances' %> +<% end %> diff --git a/src/app/views/instance/new.html.erb b/src/app/views/instance/new.html.erb index 86810bc..fa46ac4 100644 --- a/src/app/views/instance/new.html.erb +++ b/src/app/views/instance/new.html.erb @@ -1,23 +1,32 @@ <script type="text/javascript"> - function showImages(ev) { - $("#images_table_wrapper").css('display', 'block'); - images_dialog = $("#images_table_wrapper").dialog({ - title: "Select template for new instance", - dialogClass: 'flora', - width: 600, - height: 500, - modal: true, - overlay: {opacity: 0.2, background: "black"} + $(document).ready(function() { + $(".select_image").click(function() { + var wrapper = $("#select_template_dialog"); + if (wrapper.length == 0) wrapper = $('<div id="select_template_dialog"></div>'); + wrapper.dialog({ + title: "Select template for new instance", + width: 600, + height: 500, + modal: true, + overlay: {opacity: 0.2, background: "black"} + }); + wrapper.load('<%= url_for :action => 'select_image' %>', null, extend_select_button); + return false; }); - } + });
- function selectImage(ev) { - var image_id = dataTable_images_table.fnGetData(ev.target.parentNode)[0]; - var image_name = dataTable_images_table.fnGetData(ev.target.parentNode)[1]; - $("#images_table_wrapper").css('display', 'none'); - $("#images_table_wrapper").dialog('close'); - $('input[name="image_selector"]').attr('value', image_name); - $('input[name="instance[image_id]"]').attr('value', image_id); + function extend_select_button() { + $(":submit[name=select]").click(function() { + var checkbox = $("input[name='ids[]']:checked").first(); + var id = checkbox.val(); + if (id !== undefined) { + var name = $(".image_name", checkbox.parent().parent()).text(); + $(".select_image").text(name); + $("input[name='instance[image_id]']").val(id); + } + $("#select_template_dialog").dialog('close'); + return false; + }); }
function poolSelected(field) { @@ -35,7 +44,7 @@ <li><label>Name<span>Name for this new Instance</span></label><%= text_field :instance, :name, :class => "txtfield" %></li>
<li><label>Template<span>Choose a template to use</span></label> - <input type="button" style="cursor:pointer" onclick="showImages(event)" name="image_selector" value="<%= @instance.image ? @instance.image.name : "click to select template"%>"/> + <%= link_to(@instance.image ? @instance.image.name : "Select template...", {:action => 'select_image'}, {:class => 'actionlink select_image'}) %> </li> <input type=hidden name="instance[image_id]" value="<%= @instance.image ? @instance.image.id : '' %>">
@@ -71,30 +80,3 @@ <%= submit_tag "Save", :class => "submit" %> <% end %> </div> - -<%= datatable( - Image::COLUMNS_SIMPLE.map {|c| c[:opts]}, - { - :table_dom_id => 'images_table', - :per_page => Image::per_page, - :sort_by => "[2, 'asc']", - :serverside => true, - :ajax_source => url_for(:controller => 'image', :action => 'images_paginate') + "?mode=simple", - :append => ".fnSetFilteringDelay()", - :click_callback => "function(event) {selectImage(event)}", - } -) %> - -<div style="display:none" id="images_table_wrapper"> - <table class="datatable display" id="images_table"> - <thead> - <tr> - <% Image::COLUMNS_SIMPLE.each do |c| %> - <%= "<th>#{c[:header]}</th>" %> - <% end %> - </tr> - </thead> - <tbody> - </tbody> - </table> -</div> diff --git a/src/app/views/instance/select_image.haml b/src/app/views/instance/select_image.haml new file mode 100644 index 0000000..c58d459 --- /dev/null +++ b/src/app/views/instance/select_image.haml @@ -0,0 +1,4 @@ +- form_tag({:action => 'select_image'}, {:class => 'dtable_form'}) do + = render :partial => 'image/images' + .action_buttons + = submit_tag "Select", :name => "select" diff --git a/src/app/views/instance/show.html.erb b/src/app/views/instance/show.html.erb index a2bfe2f..689ead1 100644 --- a/src/app/views/instance/show.html.erb +++ b/src/app/views/instance/show.html.erb @@ -1,29 +1,2 @@ -<% if @instances.size == 0 %> -<h1>There are no pools to display</h1> -<% else %> - <table> - <thead> - <tr> - <th scope="col">Actions</th> - <th scope="col">Name</th> - </tr> - </thead> - <tbody> - <%@instances.each {|instance| %> - <tr> - <td> - <%instance.get_action_list.each {|action|%> - <%= link_to action, :controller => "instance", - :action => "instance_action", - :id => instance, - :instance_action => action %> - <br/> - <% } %> - </td> - <td><div><%= instance.name %></div></td> - </tr> - <% } %> - </tbody> - </table> -<% end %> +<%= render :partial => 'instance/instances' %> <%= link_to "Add a new instance", :controller => "instance", :action => "new", :id => @pool %> diff --git a/src/app/views/layouts/aggregator.haml b/src/app/views/layouts/aggregator.haml index 0a187bb..c79dce7 100644 --- a/src/app/views/layouts/aggregator.haml +++ b/src/app/views/layouts/aggregator.haml @@ -14,6 +14,7 @@ = stylesheet_link_tag 'jquery.ui-1.8.1/jquery-ui-1.8.1.custom.css' = stylesheet_link_tag 'jquery-datatables/demo_table_jui.css'
+ = javascript_include_tag "application.js" = javascript_include_tag "jquery-1.4.2.min.js" = javascript_include_tag "facebox.js" = javascript_include_tag "jquery.ui-1.8.1/jquery-ui-1.8.1.custom.min.js" diff --git a/src/app/views/pool/list.haml b/src/app/views/pool/list.haml new file mode 100644 index 0000000..2014ed9 --- /dev/null +++ b/src/app/views/pool/list.haml @@ -0,0 +1,3 @@ += render :partial => "instance/instances" + += link_to "Add a new instance", {:controller => "instance", :action => "new", "instance[pool_id]" => @pool}, :class=>"actionlink" diff --git a/src/app/views/pool/list.html.erb b/src/app/views/pool/list.html.erb deleted file mode 100644 index 38e46e7..0000000 --- a/src/app/views/pool/list.html.erb +++ /dev/null @@ -1,38 +0,0 @@ -<script type="text/javascript"> - function clickRow(ev) { - var box = $('input[name="image_id[]"]', ev.target.parentNode); - if ($(ev.target).attr('name') != "image_id[]") - box.attr('checked', !box.attr('checked')); - } - - function checkAll(ev) { - $('input[name="image_id[]"]', dataTable_instances_table).attr('checked', $(ev.target).attr('checked')) - } -</script> - -<%= datatable( - Instance::COLUMNS.map {|c| c[:opts]}, - { - :table_dom_id => 'instances_table', - :per_page => Instance::per_page, - :sort_by => "[3, 'asc']", - :serverside => true, - :ajax_source => url_for(:controller => 'pool', :action => 'instances_paginate') + "/#{@pool.id}", - :append => ".fnSetFilteringDelay()", - :persist_state => false, - :click_callback => "function(ev) {clickRow(ev);}", - } -) %> - -<table class="datatable display" id="instances_table"> - <thead> - <tr> - <% Instance::COLUMNS.each do |c| %> - <%= "<th>#{c[:header]}</th>" %> - <% end %> - </tr> - </thead> - <tbody> - </tbody> -</table> -<%= link_to "Add a new instance", {:controller => "instance", :action => "new", "instance[pool_id]" => @pool}, :class=>"actionlink"%> diff --git a/src/public/javascripts/application.js b/src/public/javascripts/application.js index fe45776..be1bc91 100644 --- a/src/public/javascripts/application.js +++ b/src/public/javascripts/application.js @@ -1,2 +1,37 @@ // Place your application-specific JavaScript functions and classes here // This file is automatically included by javascript_include_tag :defaults + +function extend_table(id, search_url) { + var table = $("#" + id); + var wrapper = $(".wrapper", table); + // make column head links ajax + $("table thead tr th a", table).click(function() {wrapper.load($(this).attr('href'));return false;}); + + // pagination links should by ajax too + $(".pagination a", table).click(function() {wrapper.load($(this).attr('href'));return false;}); + + // check all checkboxes when checking "check all" + $("input[name='check_all']", table).click(function() {$("input[name='ids[]']", table).attr('checked', $(this).attr('checked'))}); + + // check checkbox if row is clicked + $("table tbody tr", table).click(function(ev) { + if ($(ev.target).attr('name') == 'ids[]') return; + var box = $("input[name='ids[]']", this); + box.attr('checked', !box.attr('checked')); + }); +} + +// table ajax search with 1 sec delay +function extend_table_search_field(id, search_url) { + var table = $("#" + id); + var wrapper = $(".wrapper", table); + $(".search_field input", table).keypress(function() { + if (table.search_lock) return; + table.search_lock = true; + setTimeout(function() { + table.search_lock = false; + var search = $(".search_field input", table).val(); + wrapper.load(search_url + '&search=' + search); + }, 1000) + }); +} diff --git a/src/public/stylesheets/dcloud.css b/src/public/stylesheets/dcloud.css index b643754..619f655 100644 --- a/src/public/stylesheets/dcloud.css +++ b/src/public/stylesheets/dcloud.css @@ -301,3 +301,82 @@ a.button_link { #dashboard-tabs ul, #provider-tabs ul, #pool-tabs ul{ float: none; } + +/* paginated data table */ +.dtable { + border: 1px solid #bbb; +} + +.dtable table { + width: 100%; +} + +.dtable_form { + width: 96%; + padding: 20px 2% 20px 2%; +} + +.dtable tr:hover { + background-color: #eee; +} + +.dtable table th { + text-align: left; +} + +.dtable .ordercol { + background-position:left center; + background-repeat:no-repeat; + padding-left:20px; +} + +.dtable .asc { + background-image: url(../images/Sort_down_11.png); +} + +.dtable .desc { + background-image: url(../images/Sort_up_11.png); +} + +.dtable .header { + height: 25px; + background-color: #eee; + border-bottom: 1px solid #bbb; +} + +.dtable .footer { + height: 24px; + background-color: #eee; + border-top: 1px solid #bbb; + line-height: 1.8; + padding-left: 10px; +} + +.dtable .search_field { + font-size: 15px; + float: right; +} + +.dtable .search_field input { + font-size: 15px; +} + +.dtable .pagination { + float: right; + padding-right: 10px; +} + +.dtable .pagination a { + color: blue; +} + +.dtable .title { + font-size: 15px; + font-weight: bold; + padding-left: 10px; +} + +.action_buttons { + padding-top: 10px; + padding-bottom: 10px; +}
Hi, I found some minor bugs in original patch, so I'm sending part 1/2 again.
From: Jan Provaznik jprovazn@redhat.com
Replaced Datatables plugin with simpler HTML table because some issues when JS is off. Major issue with Datatables is that it doesn't degrades gracefully when JS is off and server-side pagination.
Images and instances are now displayed by HTML table (there is new helper 'paginated_table'), this table keeps all common functions like sorting, paginating, searching and code is cleaner. Works with JS on or off. --- src/app/controllers/image_controller.rb | 55 +++------------- src/app/controllers/instance_controller.rb | 93 +++++++++++++++------------ src/app/controllers/pool_controller.rb | 57 ++++------------- src/app/helpers/application_helper.rb | 70 +++++++++++++++++++++ src/app/models/image.rb | 18 ----- src/app/models/instance.rb | 13 ---- src/app/views/image/_images.haml | 13 ++++ src/app/views/image/show.haml | 17 +++++ src/app/views/image/show.html.erb | 83 ------------------------- src/app/views/instance/_instances.haml | 19 ++++++ src/app/views/instance/_list.html.erb | 37 ----------- src/app/views/instance/index.html.erb | 4 +- src/app/views/instance/new.html.erb | 72 ++++++++------------- src/app/views/instance/select_image.haml | 4 + src/app/views/instance/show.html.erb | 29 +-------- src/app/views/layouts/aggregator.haml | 1 + src/app/views/pool/list.haml | 3 + src/app/views/pool/list.html.erb | 38 ----------- src/public/javascripts/application.js | 54 ++++++++++++++++ src/public/stylesheets/dcloud.css | 79 +++++++++++++++++++++++ 20 files changed, 368 insertions(+), 391 deletions(-) create mode 100644 src/app/views/image/_images.haml create mode 100644 src/app/views/image/show.haml delete mode 100644 src/app/views/image/show.html.erb create mode 100644 src/app/views/instance/_instances.haml delete mode 100644 src/app/views/instance/_list.html.erb create mode 100644 src/app/views/instance/select_image.haml create mode 100644 src/app/views/pool/list.haml delete mode 100644 src/app/views/pool/list.html.erb
diff --git a/src/app/controllers/image_controller.rb b/src/app/controllers/image_controller.rb index d7858b1..b75f692 100644 --- a/src/app/controllers/image_controller.rb +++ b/src/app/controllers/image_controller.rb @@ -26,57 +26,24 @@ class ImageController < ApplicationController end
def show - # FIXME: check on privilege IMAGE_VIEW which currently doesn't exist - #require_privilege(Privilege::POOL_VIEW, @pool) - end - - def images_paginate - # FIXME: check on privilege IMAGE_VIEW which currently doesn't exist - #require_privilege(Privilege::POOL_VIEW, @pool) - - # datatables sends pagination in format: - # iDisplayStart - start index - # iDisplayLength - num of recs - # => we need to count page num - page = params[:iDisplayStart].to_i / Image::per_page - - if params[:mode].to_s == 'simple' - simple_mode = true - cols = Image::COLUMNS_SIMPLE - default_order_col = 1 - else - cols = Image::COLUMNS - simple_mode = false - default_order_col = 2 + if params[:create_instance] + redirect_to :controller => 'instance', :action => 'new', 'instance[image_id]' => (params[:ids] || []).first end
- order_col_rec = cols[params[:iSortCol_0].to_i] - order_col = cols[default_order_col] unless order_col_rec && order_col_rec[:opts][:searchable] - order = order_col[:id] + " " + (params[:sSortDir_0] == 'desc' ? 'desc' : 'asc') + require_privilege(Privilege::IMAGE_VIEW)
- @images = Image.search_filter(params[:sSearch], Image::SEARCHABLE_COLUMNS).paginate( - :page => page + 1, + @order_dir = params[:order_dir] == 'desc' ? 'desc' : 'asc' + @order = params[:order] || 'name' + @images = Image.search_filter(params[:search], Image::SEARCHABLE_COLUMNS).paginate( + :page => params[:page] || 1, + :order => @order + ' ' + @order_dir, :include => :instances, - :order => order, :conditions => {:provider_id => nil} )
- expand_button_html = "<img src='/images/dir_closed.png'>" - - data = @images.map do |i| - if simple_mode - [i.id, i.name, i.architecture, i.instances.size] - else - [i.id, expand_button_html, i.name, i.architecture, i.instances.size, "TODO: some description here?"] - end + if request.xhr? and params[:partial] + render :partial => 'images' + return end - - render :json => { - :sEcho => params[:sEcho], - :iTotalRecords => @images.total_entries, - :iTotalDisplayRecords => @images.total_entries, - :aaData => data - } end - end diff --git a/src/app/controllers/instance_controller.rb b/src/app/controllers/instance_controller.rb index 5664ec5..f65f7c1 100644 --- a/src/app/controllers/instance_controller.rb +++ b/src/app/controllers/instance_controller.rb @@ -23,58 +23,69 @@ require 'util/condormatic'
class InstanceController < ApplicationController before_filter :require_user + layout :layout
- def index + def layout + return "aggregator" unless request.xhr? end
- def paginated - # datatables sends pagination in format: - # iDisplayStart - start index - # iDisplayLength - num of recs - # => we need to count page num - page = params[:iDisplayStart].to_i / Instance::per_page - - order_col_rec = Instance::COLUMNS[params[:iSortCol_0].to_i] - order_col = Instance::COLUMNS[2] unless order_col_rec && order_col_rec[:opts][:searchable] - order = order_col[:id] + " " + (params[:sSortDir_0] == 'desc' ? 'desc' : 'asc') - - # FIXME only return those instances in pools which user has instance_view - @instances = Instance.search_filter(params[:sSearch], Instance::SEARCHABLE_COLUMNS).paginate( - :page => page + 1, - :order => order + def index + require_privilege(Privilege::INSTANCE_VIEW) + + @order_dir = params[:order_dir] == 'desc' ? 'desc' : 'asc' + @order = params[:order] || 'name' + @instances = Instance.search_filter(params[:search], Instance::SEARCHABLE_COLUMNS).paginate( + :page => params[:page] || 1, + :order => @order + ' ' + @order_dir )
- recs = @instances.map do |i| - [ - i.id, - i.get_action_list.map {|action| "<a href="#{url_for :controller => "instance", :action => "instance_action", :id => i.id, :instance_action => action}">#{action}</a>"}.join(" | "), - i.name, - i.state, - i.hardware_profile.name, - i.image.name, - i.cloud_account.nil? ? "" : i.cloud_account.provider.name, - i.cloud_account.nil? ? "" : i.cloud_account.name - ] + if request.xhr? and params[:partial] + render :partial => 'instances' + return end - - render :json => { - :sEcho => params[:sEcho], - :iTotalRecords => @instances.total_entries, - :iTotalDisplayRecords => @instances.total_entries, - :aaData => recs - } end
- # Right now this is essentially a duplicate of PoolController#show, - # but really it should be a single instance should we decide to have a page - # for that. Redirect on create was all that brought you here anyway, so - # should be unused for the moment. - def show - @instances = Instance.find(:all, :conditions => {:pool_id => params[:id]}) - @pool = Pool.find(params[:id]) - require_privilege(Privilege::INSTANCE_VIEW,@pool) + def select_image + if params[:select] + redirect_to :action => 'new', 'instance[image_id]' => (params[:ids] || []).first + end + + require_privilege(Privilege::IMAGE_VIEW) + @order_dir = params[:order_dir] == 'desc' ? 'desc' : 'asc' + @order = params[:order] || 'name' + @images = Image.search_filter(params[:search], Image::SEARCHABLE_COLUMNS).paginate( + :page => params[:page] || 1, + :order => @order + ' ' + @order_dir, + :conditions => {:provider_id => nil} + ) + @single_select = true + + if request.xhr? and params[:partial] + render :partial => 'image/images' + return + end end
+ ## Right now this is essentially a duplicate of PoolController#show, + # # but really it should be a single instance should we decide to have a page + # # for that. Redirect on create was all that brought you here anyway, so + # # should be unused for the moment. + #def show + # require_privilege(Privilege::INSTANCE_VIEW,@pool) + # @pool = Pool.find(params[:id]) + # @order_dir = params[:order_dir] == 'desc' ? 'desc' : 'asc' + # @order = params[:order] || 'name' + # @instances = Instance.search_filter(params[:search], Instance::SEARCHABLE_COLUMNS).paginate( + # :page => params[:page] || 1, + # :order => @order + ' ' + @order_dir, + # :conditions => {:pool_id => @pool.id} + # ) + # if request.xhr? and params[:partial] + # render :partial => 'instances' + # return + # end + #end + def new @instance = Instance.new(params[:instance]) require_privilege(Privilege::INSTANCE_MODIFY, @instance.pool) if @instance.pool diff --git a/src/app/controllers/pool_controller.rb b/src/app/controllers/pool_controller.rb index ad2d42b..19effa9 100644 --- a/src/app/controllers/pool_controller.rb +++ b/src/app/controllers/pool_controller.rb @@ -40,7 +40,20 @@ class PoolController < ApplicationController # Go to condor and sync the database to the real instance states condormatic_instances_sync_states @pool.reload - @instances = @pool.instances + + @order_dir = params[:order_dir] == 'desc' ? 'desc' : 'asc' + @order = params[:order] || 'name' + + @instances = Instance.search_filter(params[:search], Instance::SEARCHABLE_COLUMNS).paginate( + :page => params[:page] || 1, + :order => @order + ' ' + @order_dir, + :conditions => {:pool_id => @pool.id} + ) + + if request.xhr? and params[:partial] + render :partial => 'instance/instances' + return + end end
def hardware_profiles @@ -90,48 +103,6 @@ class PoolController < ApplicationController def delete end
- def instances_paginate - @pool = Pool.find(params[:id]) - require_privilege(Privilege::POOL_VIEW, @pool) - - # datatables sends pagination in format: - # iDisplayStart - start index - # iDisplayLength - num of recs - # => we need to count page num - page = params[:iDisplayStart].to_i / Instance::per_page - - order_col_rec = Instance::COLUMNS[params[:iSortCol_0].to_i] - order_col = Instance::COLUMNS[2] unless order_col_rec && order_col_rec[:opts][:searchable] - order = order_col[:id] + " " + (params[:sSortDir_0] == 'desc' ? 'desc' : 'asc') - - @instances = Instance.search_filter(params[:sSearch], Instance::SEARCHABLE_COLUMNS).paginate( - :page => page + 1, - :order => order, - :conditions => {:pool_id => @pool.id} - ) - - recs = @instances.map do |i| - [ - i.id, - i.get_action_list.map {|action| "<a href="#{url_for :controller => "instance", :action => "instance_action", :id => i.id, :instance_action => action}">#{action}</a>"}.join(" | "), - i.name, - i.state, - i.hardware_profile.name, - i.image.name, - i.cloud_account.nil? ? "" : i.cloud_account.provider.name, - i.cloud_account.nil? ? "" : i.cloud_account.name - ] - end - - render :json => { - :sEcho => params[:sEcho], - :iTotalRecords => @instances.total_entries, - :iTotalDisplayRecords => @instances.total_entries, - :aaData => recs - } - end - - def accounts_for_pool @pool = Pool.find(params[:pool_id]) require_privilege(Privilege::ACCOUNT_VIEW,@pool) diff --git a/src/app/helpers/application_helper.rb b/src/app/helpers/application_helper.rb index f7b96af..82762af 100644 --- a/src/app/helpers/application_helper.rb +++ b/src/app/helpers/application_helper.rb @@ -58,4 +58,74 @@ module ApplicationHelper end day_str + hours_to_seconds end + + def column_header(column, order, order_dir, check_all) + next_dir = order_dir.to_s == 'asc' ? 'desc' : 'asc' + if order and column[:id] == order + dir = order_dir.to_s == 'desc' ? 'desc' : 'asc' + cls = "ordercol #{dir}" + else + cls = nil + end + + if column[:sortable] + label = link_to( + column[:header], + {:action => controller.action_name, :partial => true, :order => column[:id], :order_dir => next_dir, :search => params[:search]}, + :class => cls) + elsif check_all.to_s == column[:id] + label = check_box_tag 'check_all' + else + label = column[:header] + end + + content_tag 'th', label + end + + def paginated_table(html_id, columns, data, opts = {}) + search_url = url_for(:partial => true, :order => opts[:order], :order_dir => opts[:order_dir]) + extend_table_js = "<script type="text/javascript">extend_table('#{html_id}', #{opts[:single_select] ? true : false});#{opts[:load_callback]};</script>" + extend_sfield_js = "<script type="text/javascript">extend_table_search_field('#{html_id}', '#{search_url}')</script>" + + rows = data.map do |rec| + if block_given? + capture_haml{yield rec} + else + content_tag 'tr', :class => cycle('even', 'odd') do + columns.map { |c| content_tag 'td', rec[c[:id]] } + end + end + end + + header_cols = columns.map {|c| column_header(c, opts[:order], opts[:order_dir], opts[:check_all])} + + table = content_tag 'table' do + content_tag('thead', content_tag('tr', header_cols)) + content_tag('tbody', rows) + end + + search = content_tag 'div', :class => 'search_field' do + 'Search ' + text_field_tag('search', params[:search], :size => 10) + end + + header = content_tag 'div', :class => 'header' do + search + content_tag('div', opts[:title], :class => 'title') + end + + footer = content_tag 'div', :class => 'footer' do + will_paginate(data, :params => {:partial => true}).to_s + + page_entries_info(data).to_s + end + + ajax_content = table + footer + extend_table_js + + if params[:partial] and request.xhr? + ajax_content + else + content_tag 'div', :id => html_id, :class => "dtable #{opts[:class]}" do + header + + content_tag('div', ajax_content, :class => 'wrapper') + + extend_sfield_js + end + end + end end diff --git a/src/app/models/image.rb b/src/app/models/image.rb index 9df4185..f3304cf 100644 --- a/src/app/models/image.rb +++ b/src/app/models/image.rb @@ -48,24 +48,6 @@ class Image < ActiveRecord::Base
validates_presence_of :architecture, :if => :provider
- # used to get sorting column in controller and in view to generate datatable definition and - # html table structure - COLUMNS = [ - {:id => 'id', :header => '<input type="checkbox" id="image_id_all" onclick="checkAll(event)">', :opts => {:checkbox_id => 'image_id', :searchable => false, :sortable => false, :width => '1px', :class => 'center'}}, - {:id => 'expand_button', :header => '', :opts => {:searchable => false, :sortable => false, :width => '1px'}}, - {:id => 'name', :header => 'Name', :opts => {:width => "30%"}}, - {:id => 'architecture', :header => 'Architecture', :opts => {:width => "10%"}}, - {:id => 'instances', :header => 'Instances', :opts => {:sortable => false, :width => "10%"}}, - {:id => 'details', :header => '', :opts => {:sortable => false, :visible => false, :searchable => false}}, - ] - - COLUMNS_SIMPLE = [ - {:id => 'id', :header => '', :opts => {:visible => false, :searchable => false, :sortable => false, :width => '1px', :class => 'center'}}, - {:id => 'name', :header => 'Name', :opts => {:width => "50%"}}, - {:id => 'architecture', :header => 'Architecture', :opts => {:width => "30%"}}, - {:id => 'instances', :header => 'Instances', :opts => {:sortable => false, :width => "20%"}}, - ] - SEARCHABLE_COLUMNS = %w(name architecture)
def provider_image? diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb index 6be4931..0212e24 100644 --- a/src/app/models/instance.rb +++ b/src/app/models/instance.rb @@ -59,19 +59,6 @@ class Instance < ActiveRecord::Base STATES = [STATE_NEW, STATE_PENDING, STATE_RUNNING, STATE_SHUTTING_DOWN, STATE_STOPPED, STATE_CREATE_FAILED]
- # used to get sorting column in controller and in view to generate datatable definition and - # html table structure - COLUMNS = [ - {:id => 'id', :header => '<input type="checkbox" id="image_id_all" onclick="checkAll(event)">', :opts => {:checkbox_id => 'image_id', :searchable => false, :sortable => false, :width => '1px', :class => 'center'}}, - {:id => 'actions', :header => 'Actions', :opts => {:width => "10%", :sortable => false}}, - {:id => 'name', :header => 'Name', :opts => {:width => "25%"}}, - {:id => 'state', :header => 'State', :opts => {:width => "10%"}}, - {:id => 'hwprofile', :header => 'HW profile', :opts => {:width => "15%"}}, - {:id => 'template', :header => 'Template', :opts => {:sortable => false, :width => "15%"}}, - {:id => 'provider', :header => 'Provider', :opts => {:width => "10%"}}, - {:id => 'account', :header => 'Account', :opts => {:width => "10%"}}, - ] - SEARCHABLE_COLUMNS = %w(name state)
validates_inclusion_of :state, diff --git a/src/app/views/image/_images.haml b/src/app/views/image/_images.haml new file mode 100644 index 0000000..217c441 --- /dev/null +++ b/src/app/views/image/_images.haml @@ -0,0 +1,13 @@ +- columns = [ | + {:id => 'id', :header => ''}, | + {:id => 'name', :header => 'Name', :sortable => true}, | + {:id => 'architecture', :header => 'Architecture', :sortable => true}, | + {:id => 'instances', :header => 'Instances'}, | +] | + += paginated_table('images_table', columns, @images, {:load_callback => 'table_loaded()', :order => @order, :order_dir => @order_dir, :title => 'List of templates', :check_all => 'id', :single_select => @single_select || false}) do |rec| + %tr{:class => "#{cycle('even', 'odd')}"} + %td= check_box_tag 'ids[]', rec.id + %td{:class => 'image_name'}= rec.name + %td= rec.architecture + %td= rec.instances.count diff --git a/src/app/views/image/show.haml b/src/app/views/image/show.haml new file mode 100644 index 0000000..5720443 --- /dev/null +++ b/src/app/views/image/show.haml @@ -0,0 +1,17 @@ +- content_for :scripts do + :javascript + function check_checkboxes() { + var checked_count = $('input[name="ids[]"]:checked', $("#images_table")).length; + $('input[name="create_instance"]').attr('disabled', checked_count != 1); + } + function table_loaded() { + //$("input[name='ids[]']", $("#images_table")).change(function() {console.warn('x');check_checkboxes()}); + //FIXME: this doesn't work in 100%, but don't know better way + $("#images_table").click(function() {check_checkboxes()}); + check_checkboxes(); + } + +- form_tag({:action => 'show'}, {:class => 'dtable_form'}) do + .action_buttons + = submit_tag "Create instance", :name => "create_instance" + = render :partial => 'images' diff --git a/src/app/views/image/show.html.erb b/src/app/views/image/show.html.erb deleted file mode 100644 index 5a7cb53..0000000 --- a/src/app/views/image/show.html.erb +++ /dev/null @@ -1,83 +0,0 @@ -<script type="text/javascript"> - - function fnOpenClose() { - $('td img', dataTable_images_table.fnGetNodes() ).each( function () { - $(this).click( function () { - var nTr = this.parentNode.parentNode; - if ( this.src.match('dir_open') ) - { - /* This row is already open - close it */ - this.src = "/images/dir_closed.png"; - /* fnClose doesn't do anything for server-side processing - do it ourselves :-) */ - var nRemove = $(nTr).next()[0]; - nRemove.parentNode.removeChild( nRemove ); - } - else - { - /* Open this row */ - this.src = "/images/dir_open.png"; - dataTable_images_table.fnOpen( nTr, dataTable_images_table.fnGetData(nTr)[5], 'details' ); - } - }); - }); - } - - function getCheckedRows() { - return $('input[name="image_id[]"]:checked', dataTable_images_table).length; - } - - function showButtons() { - getCheckedRows(); - $('input[name="create_image"]').attr('disabled', getCheckedRows() != 1); - } - - function checkRow(ev) { - var box = $('input[name="image_id[]"]', ev.target.parentNode); - if ($(ev.target).attr('name') != "image_id[]") - box.attr('checked', !box.attr('checked')); - showButtons(); - } - - function checkAll(ev) { - $('input[name="image_id[]"]', dataTable_images_table).attr('checked', $(ev.target).attr('checked')) - } - - function drawCallback() { - $('#image_id_all').attr('checked', false); - fnOpenClose(); - } - - function createInstance() { - var selected_row = $('input[name="image_id[]"]:checked', dataTable_images_table); - location.href = '<%= url_for(:controller => "instance", :action => "new") %>?instance[image_id]=' + selected_row.attr('value'); - } -</script> - -<%= datatable( - Image::COLUMNS.map {|c| c[:opts]}, - { - :table_dom_id => 'images_table', - :per_page => Image::per_page, - :sort_by => "[2, 'asc']", - :serverside => true, - :ajax_source => url_for(:controller => 'image', :action => 'images_paginate'), - :append => ".fnSetFilteringDelay()", - :persist_state => false, - :click_callback => "function(ev) {checkRow(ev);}", - :draw_callback => "drawCallback", - } -) %> - <div style="padding:20px"> - <input type="button" value="Create instance" name="create_image" disabled=true onclick="createInstance()"> - </div> - <table class="datatable display" id="images_table"> - <thead> - <tr> - <% Image::COLUMNS.each do |c| %> - <%= "<th>#{c[:header]}</th>" %> - <% end %> - </tr> - </thead> - <tbody> - </tbody> -</table> diff --git a/src/app/views/instance/_instances.haml b/src/app/views/instance/_instances.haml new file mode 100644 index 0000000..639ddb9 --- /dev/null +++ b/src/app/views/instance/_instances.haml @@ -0,0 +1,19 @@ +- columns = [ | + {:id => 'id', :header => ''}, | + {:header => 'Actions'}, | + {:id => 'name', :header => 'Name', :sortable => true}, | + {:header => 'Details'}, | + {:id => 'template', :header => 'Template'}, | + {:id => 'state', :sortable => true, :header => 'State'}, | + {:id => 'time_last_running', :header => 'Time last running'}, | +] | + += paginated_table('instances_table', columns, @instances, {:order => @order, :order_dir => @order_dir, :title => 'List of instances', :check_all => 'id'}) do |rec| + %tr{:class => "#{cycle('even', 'odd')}"} + %td= check_box_tag 'ids[]', rec.id + %td= rec.get_action_list.map {|action| link_to action, :controller => "instance", :action => "instance_action", :id => rec, :instance_action => action}.join(" | ") + %td= rec.name + %td Details + %td= rec.image.name + %td= rec.state + %td= rec.time_last_running diff --git a/src/app/views/instance/_list.html.erb b/src/app/views/instance/_list.html.erb deleted file mode 100644 index 4328978..0000000 --- a/src/app/views/instance/_list.html.erb +++ /dev/null @@ -1,37 +0,0 @@ -<script type="text/javascript"> - function clickRow(ev) { - var box = $('input[name="image_id[]"]', ev.target.parentNode); - if ($(ev.target).attr('name') != "image_id[]") - box.attr('checked', !box.attr('checked')); - } - - function checkAll(ev) { - $('input[name="image_id[]"]', dataTable_instances_table).attr('checked', $(ev.target).attr('checked')) - } -</script> - -<%= datatable( - Instance::COLUMNS.map {|c| c[:opts]}, - { - :table_dom_id => 'instances_table', - :per_page => Instance::per_page, - :sort_by => "[3, 'asc']", - :serverside => true, - :ajax_source => url_for(:controller => 'instance', :action => 'paginated'), - :append => ".fnSetFilteringDelay()", - :persist_state => false, - :click_callback => "function(ev) {clickRow(ev);}", - } -) %> - -<table class="datatable display" id="instances_table"> - <thead> - <tr> - <% Instance::COLUMNS.each do |c| %> - <%= "<th>#{c[:header]}</th>" %> - <% end %> - </tr> - </thead> - <tbody> - </tbody> -</table> diff --git a/src/app/views/instance/index.html.erb b/src/app/views/instance/index.html.erb index c19c3fa..e57f78c 100644 --- a/src/app/views/instance/index.html.erb +++ b/src/app/views/instance/index.html.erb @@ -1 +1,3 @@ -<%= render :partial => 'list' %> +<% form_tag({:action => 'instance_action'}, {:class => 'dtable_form'}) do %> + <%= render :partial => 'instance/instances' %> +<% end %> diff --git a/src/app/views/instance/new.html.erb b/src/app/views/instance/new.html.erb index 86810bc..fa46ac4 100644 --- a/src/app/views/instance/new.html.erb +++ b/src/app/views/instance/new.html.erb @@ -1,23 +1,32 @@ <script type="text/javascript"> - function showImages(ev) { - $("#images_table_wrapper").css('display', 'block'); - images_dialog = $("#images_table_wrapper").dialog({ - title: "Select template for new instance", - dialogClass: 'flora', - width: 600, - height: 500, - modal: true, - overlay: {opacity: 0.2, background: "black"} + $(document).ready(function() { + $(".select_image").click(function() { + var wrapper = $("#select_template_dialog"); + if (wrapper.length == 0) wrapper = $('<div id="select_template_dialog"></div>'); + wrapper.dialog({ + title: "Select template for new instance", + width: 600, + height: 500, + modal: true, + overlay: {opacity: 0.2, background: "black"} + }); + wrapper.load('<%= url_for :action => 'select_image' %>', null, extend_select_button); + return false; }); - } + });
- function selectImage(ev) { - var image_id = dataTable_images_table.fnGetData(ev.target.parentNode)[0]; - var image_name = dataTable_images_table.fnGetData(ev.target.parentNode)[1]; - $("#images_table_wrapper").css('display', 'none'); - $("#images_table_wrapper").dialog('close'); - $('input[name="image_selector"]').attr('value', image_name); - $('input[name="instance[image_id]"]').attr('value', image_id); + function extend_select_button() { + $(":submit[name=select]").click(function() { + var checkbox = $("input[name='ids[]']:checked").first(); + var id = checkbox.val(); + if (id !== undefined) { + var name = $(".image_name", checkbox.parent().parent()).text(); + $(".select_image").text(name); + $("input[name='instance[image_id]']").val(id); + } + $("#select_template_dialog").dialog('close'); + return false; + }); }
function poolSelected(field) { @@ -35,7 +44,7 @@ <li><label>Name<span>Name for this new Instance</span></label><%= text_field :instance, :name, :class => "txtfield" %></li>
<li><label>Template<span>Choose a template to use</span></label> - <input type="button" style="cursor:pointer" onclick="showImages(event)" name="image_selector" value="<%= @instance.image ? @instance.image.name : "click to select template"%>"/> + <%= link_to(@instance.image ? @instance.image.name : "Select template...", {:action => 'select_image'}, {:class => 'actionlink select_image'}) %> </li> <input type=hidden name="instance[image_id]" value="<%= @instance.image ? @instance.image.id : '' %>">
@@ -71,30 +80,3 @@ <%= submit_tag "Save", :class => "submit" %> <% end %> </div> - -<%= datatable( - Image::COLUMNS_SIMPLE.map {|c| c[:opts]}, - { - :table_dom_id => 'images_table', - :per_page => Image::per_page, - :sort_by => "[2, 'asc']", - :serverside => true, - :ajax_source => url_for(:controller => 'image', :action => 'images_paginate') + "?mode=simple", - :append => ".fnSetFilteringDelay()", - :click_callback => "function(event) {selectImage(event)}", - } -) %> - -<div style="display:none" id="images_table_wrapper"> - <table class="datatable display" id="images_table"> - <thead> - <tr> - <% Image::COLUMNS_SIMPLE.each do |c| %> - <%= "<th>#{c[:header]}</th>" %> - <% end %> - </tr> - </thead> - <tbody> - </tbody> - </table> -</div> diff --git a/src/app/views/instance/select_image.haml b/src/app/views/instance/select_image.haml new file mode 100644 index 0000000..c58d459 --- /dev/null +++ b/src/app/views/instance/select_image.haml @@ -0,0 +1,4 @@ +- form_tag({:action => 'select_image'}, {:class => 'dtable_form'}) do + = render :partial => 'image/images' + .action_buttons + = submit_tag "Select", :name => "select" diff --git a/src/app/views/instance/show.html.erb b/src/app/views/instance/show.html.erb index a2bfe2f..689ead1 100644 --- a/src/app/views/instance/show.html.erb +++ b/src/app/views/instance/show.html.erb @@ -1,29 +1,2 @@ -<% if @instances.size == 0 %> -<h1>There are no pools to display</h1> -<% else %> - <table> - <thead> - <tr> - <th scope="col">Actions</th> - <th scope="col">Name</th> - </tr> - </thead> - <tbody> - <%@instances.each {|instance| %> - <tr> - <td> - <%instance.get_action_list.each {|action|%> - <%= link_to action, :controller => "instance", - :action => "instance_action", - :id => instance, - :instance_action => action %> - <br/> - <% } %> - </td> - <td><div><%= instance.name %></div></td> - </tr> - <% } %> - </tbody> - </table> -<% end %> +<%= render :partial => 'instance/instances' %> <%= link_to "Add a new instance", :controller => "instance", :action => "new", :id => @pool %> diff --git a/src/app/views/layouts/aggregator.haml b/src/app/views/layouts/aggregator.haml index 0a187bb..c79dce7 100644 --- a/src/app/views/layouts/aggregator.haml +++ b/src/app/views/layouts/aggregator.haml @@ -14,6 +14,7 @@ = stylesheet_link_tag 'jquery.ui-1.8.1/jquery-ui-1.8.1.custom.css' = stylesheet_link_tag 'jquery-datatables/demo_table_jui.css'
+ = javascript_include_tag "application.js" = javascript_include_tag "jquery-1.4.2.min.js" = javascript_include_tag "facebox.js" = javascript_include_tag "jquery.ui-1.8.1/jquery-ui-1.8.1.custom.min.js" diff --git a/src/app/views/pool/list.haml b/src/app/views/pool/list.haml new file mode 100644 index 0000000..2014ed9 --- /dev/null +++ b/src/app/views/pool/list.haml @@ -0,0 +1,3 @@ += render :partial => "instance/instances" + += link_to "Add a new instance", {:controller => "instance", :action => "new", "instance[pool_id]" => @pool}, :class=>"actionlink" diff --git a/src/app/views/pool/list.html.erb b/src/app/views/pool/list.html.erb deleted file mode 100644 index 38e46e7..0000000 --- a/src/app/views/pool/list.html.erb +++ /dev/null @@ -1,38 +0,0 @@ -<script type="text/javascript"> - function clickRow(ev) { - var box = $('input[name="image_id[]"]', ev.target.parentNode); - if ($(ev.target).attr('name') != "image_id[]") - box.attr('checked', !box.attr('checked')); - } - - function checkAll(ev) { - $('input[name="image_id[]"]', dataTable_instances_table).attr('checked', $(ev.target).attr('checked')) - } -</script> - -<%= datatable( - Instance::COLUMNS.map {|c| c[:opts]}, - { - :table_dom_id => 'instances_table', - :per_page => Instance::per_page, - :sort_by => "[3, 'asc']", - :serverside => true, - :ajax_source => url_for(:controller => 'pool', :action => 'instances_paginate') + "/#{@pool.id}", - :append => ".fnSetFilteringDelay()", - :persist_state => false, - :click_callback => "function(ev) {clickRow(ev);}", - } -) %> - -<table class="datatable display" id="instances_table"> - <thead> - <tr> - <% Instance::COLUMNS.each do |c| %> - <%= "<th>#{c[:header]}</th>" %> - <% end %> - </tr> - </thead> - <tbody> - </tbody> -</table> -<%= link_to "Add a new instance", {:controller => "instance", :action => "new", "instance[pool_id]" => @pool}, :class=>"actionlink"%> diff --git a/src/public/javascripts/application.js b/src/public/javascripts/application.js index fe45776..0d3b749 100644 --- a/src/public/javascripts/application.js +++ b/src/public/javascripts/application.js @@ -1,2 +1,56 @@ // Place your application-specific JavaScript functions and classes here // This file is automatically included by javascript_include_tag :defaults + +function extend_table(id, single_select) { + var table = $("#" + id); + var wrapper = $(".wrapper", table); + // make column head links ajax + $("table thead tr th a", table).click(function() {wrapper.load($(this).attr('href'));return false;}); + + // pagination links should by ajax too + $(".pagination a", table).click(function() {wrapper.load($(this).attr('href'));return false;}); + + // check all checkboxes when checking "check all" + $("input[name='check_all']", table).click(function() {$("input[name='ids[]']", table).attr('checked', $(this).attr('checked'))}); + + // check checkbox if row is clicked + $("table tbody tr", table).click(function(ev) { + if (single_select) { + var checked, checkbox; + if ($(ev.target).attr('name') == 'ids[]') { + checked = $(ev.target).attr('checked'); + checkbox = $(ev.target); + } else { + checked = true; + checkbox = $("input[name='ids[]']", this); + } + $("input[name='ids[]']", table).attr('checked', false); + checkbox.attr('checked', checked); + } else { + if ($(ev.target).attr('name') == 'ids[]') return; + var box = $("input[name='ids[]']", this); + if (ev.ctrlKey) { + $("input[name='ids[]']", this); + box.attr('checked', !box.attr('checked')); + } else { + $("input[name='ids[]']", table).attr('checked', false); + box.attr('checked', true); + } + } + }); +} + +// table ajax search with 1 sec delay +function extend_table_search_field(id, search_url) { + var table = $("#" + id); + var wrapper = $(".wrapper", table); + $(".search_field input", table).keypress(function() { + if (table.search_lock) return; + table.search_lock = true; + setTimeout(function() { + table.search_lock = false; + var search = $(".search_field input", table).val(); + wrapper.load(search_url + '&search=' + search); + }, 1000) + }); +} diff --git a/src/public/stylesheets/dcloud.css b/src/public/stylesheets/dcloud.css index b643754..619f655 100644 --- a/src/public/stylesheets/dcloud.css +++ b/src/public/stylesheets/dcloud.css @@ -301,3 +301,82 @@ a.button_link { #dashboard-tabs ul, #provider-tabs ul, #pool-tabs ul{ float: none; } + +/* paginated data table */ +.dtable { + border: 1px solid #bbb; +} + +.dtable table { + width: 100%; +} + +.dtable_form { + width: 96%; + padding: 20px 2% 20px 2%; +} + +.dtable tr:hover { + background-color: #eee; +} + +.dtable table th { + text-align: left; +} + +.dtable .ordercol { + background-position:left center; + background-repeat:no-repeat; + padding-left:20px; +} + +.dtable .asc { + background-image: url(../images/Sort_down_11.png); +} + +.dtable .desc { + background-image: url(../images/Sort_up_11.png); +} + +.dtable .header { + height: 25px; + background-color: #eee; + border-bottom: 1px solid #bbb; +} + +.dtable .footer { + height: 24px; + background-color: #eee; + border-top: 1px solid #bbb; + line-height: 1.8; + padding-left: 10px; +} + +.dtable .search_field { + font-size: 15px; + float: right; +} + +.dtable .search_field input { + font-size: 15px; +} + +.dtable .pagination { + float: right; + padding-right: 10px; +} + +.dtable .pagination a { + color: blue; +} + +.dtable .title { + font-size: 15px; + font-weight: bold; + padding-left: 10px; +} + +.action_buttons { + padding-top: 10px; + padding-bottom: 10px; +}
On Tue, 2010-07-13 at 14:32 +0200, jprovazn@redhat.com wrote:
From: Jan Provaznik jprovazn@redhat.com
Replaced Datatables plugin with simpler HTML table because some issues when JS is off. Major issue with Datatables is that it doesn't degrades gracefully when JS is off and server-side pagination.
Images and instances are now displayed by HTML table (there is new helper 'paginated_table'), this table keeps all common functions like sorting, paginating, searching and code is cleaner. Works with JS on or off.
Possible issues I noticed: * Search box on images list works fine if you just type and wait, but if you type something quickly and hit 'enter' as would usually be expected in a search form, it brings you to unexpected places like 'add new instance'). The 'typing brings back results' behavior is fine for js, but it should also fall back so enter submits the form to the right place as well (this part should work with or without js).
* Also, the js version should indicate in some way that it is searching. This could be as simple as adding some italic text 'Searching' in the table header right near where you type, so the user notices it. Another option would be a spinner somewhere (probably either above the table, or overlayed).
* Not sure what the issue was in the behavior of the javascript 'check-all' function, it worked for me, but if you want to try another solution that looks clean and simple, take a look at http://briancray.com/2009/08/06/check-all-jquery-javascript/ I think you can get rid of the two functions and just bind an anonymous function to the checkbox on document.ready - you might need to alter his code a bit, as we dont have all those divs and fieldsets, but if you combine that general concept with http://api.jquery.com/checkbox-selector/ , I think you should end up with something pretty clean. If you have trouble, let me know, I can help if needed.
* General note on the javascript/style. named functions like are in this patch are generally not considered good practice anymore, as they clutter the main namespace. Where possible, it is better to bind anonymous functions to elements (especially if the behavior is the same for many elements in different situations - this can also allow those things to go in application.js, so they are always available). Where that does not make sense and you want to explicitly attach behavior, the functions should be added to a named object, so in our case probably 'Aggregator' or 'Deltacloud' would make the most sense. Again, if this does not make sense, I can elaborate in chat and/or provide links with more information.
* I am torn on how much to suggest revision on the js logic within these functions. I think for now, with the general feedback above, we can probably leave the logic as-is, but then revisit it in a subsequent patch to simplify it. I see a number of things (a lot around binding checkbox behavior) that could be written much more simply, but it might be easier if I just went through afterwards, and made some suggestions for update in the form of a patch, as it is rather hard to say 'just change this or that' in the more involved functions.
* I feel like I am forgetting something I wanted to bring up after typing all that, but I have probably given you more than enough to think about with that diatribe above :)
Things that worked nicely: * Pages operate (including sort) w/o javascript enabled. * List comes back immediately instead of having to wait for it to load after page.
-j
On 07/13/2010 10:28 PM, Jason Guiditta wrote:
On Tue, 2010-07-13 at 14:32 +0200, jprovazn@redhat.com wrote:
From: Jan Provaznikjprovazn@redhat.com
Replaced Datatables plugin with simpler HTML table because some issues when JS is off. Major issue with Datatables is that it doesn't degrades gracefully when JS is off and server-side pagination.
Images and instances are now displayed by HTML table (there is new helper 'paginated_table'), this table keeps all common functions like sorting, paginating, searching and code is cleaner. Works with JS on or off.
Possible issues I noticed:
- Search box on images list works fine if you just type and wait, but if
you type something quickly and hit 'enter' as would usually be expected in a search form, it brings you to unexpected places like 'add new instance'). The 'typing brings back results' behavior is fine for js, but it should also fall back so enter submits the form to the right place as well (this part should work with or without js).
Fixed this (keeping same functionality w/o JS is sometimes quite difficult)
- Also, the js version should indicate in some way that it is searching.
This could be as simple as adding some italic text 'Searching' in the table header right near where you type, so the user notices it. Another option would be a spinner somewhere (probably either above the table, or overlayed).
Fixed by external plugin.
- Not sure what the issue was in the behavior of the javascript
'check-all' function, it worked for me, but if you want to try another solution that looks clean and simple, take a look at http://briancray.com/2009/08/06/check-all-jquery-javascript/ I think you can get rid of the two functions and just bind an anonymous function to the checkbox on document.ready - you might need to alter his code a bit, as we dont have all those divs and fieldsets, but if you combine that general concept with http://api.jquery.com/checkbox-selector/ , I think you should end up with something pretty clean. If you have trouble, let me know, I can help if needed.
It's small misunderstanding here - these two functions controlled enabling/disabling of "create instance" button in dependency of number of checked rows. Nevertheless I replaced this code.
- General note on the javascript/style. named functions like are in
this patch are generally not considered good practice anymore, as they clutter the main namespace. Where possible, it is better to bind anonymous functions to elements (especially if the behavior is the same for many elements in different situations - this can also allow those things to go in application.js, so they are always available). Where that does not make sense and you want to explicitly attach behavior, the functions should be added to a named object, so in our case probably 'Aggregator' or 'Deltacloud' would make the most sense. Again, if this does not make sense, I can elaborate in chat and/or provide links with more information.
OK, I replaced all named functions (except two basic table functions which are moved to Aggregator object) by anonymous funcs.
- I am torn on how much to suggest revision on the js logic within these
functions. I think for now, with the general feedback above, we can probably leave the logic as-is, but then revisit it in a subsequent patch to simplify it. I see a number of things (a lot around binding checkbox behavior) that could be written much more simply, but it might be easier if I just went through afterwards, and made some suggestions for update in the form of a patch, as it is rather hard to say 'just change this or that' in the more involved functions.
OK, I'll welcome any improvements and tips.
- I feel like I am forgetting something I wanted to bring up after
typing all that, but I have probably given you more than enough to think about with that diatribe above :)
Things that worked nicely:
- Pages operate (including sort) w/o javascript enabled.
- List comes back immediately instead of having to wait for it to load
after page.
-j
I'm sending patch as attachments (long lines issues in all of them).
Patches 0001-Reworked-image-instance-listing.patch and 0002-Removed-Datatables-files.patch are replacement of previous version of this patch.
Use Revision-of-image-instance-listing.patch only if you applied previous verion of this patch, in other case ignore it.
Jan
On Thu, 2010-07-15 at 16:18 +0200, Jan Provaznik wrote:
On 07/13/2010 10:28 PM, Jason Guiditta wrote:
On Tue, 2010-07-13 at 14:32 +0200, jprovazn@redhat.com wrote:
From: Jan Provaznikjprovazn@redhat.com
Replaced Datatables plugin with simpler HTML table because some issues when JS is off. Major issue with Datatables is that it doesn't degrades gracefully when JS is off and server-side pagination.
Images and instances are now displayed by HTML table (there is new helper 'paginated_table'), this table keeps all common functions like sorting, paginating, searching and code is cleaner. Works with JS on or off.
ACK. Much improved, thanks for the second rev here. However, there are a couple small issues that should be addressed (separately, not in this patch):
* When javascript is off, if you click a pool in left nav, you get an unstyled list instead of tabs. Instead, this should still appear to be tabs, and clicking them should just cause a page reload. With a minor caveat, I suspect this should be as simple as setting a pre-defined style from jq-ui on the list (at least it used to work this way). If not, we'll need to mix that style into our compass stuff so the tabs always look right. Tomas, this might be a good one for you to look at if you have some time. * When javascript is off, clicking the link in the list above still returns the partial only. It should instead return the full layout with tab selected. (I may have a quick at least partial fix for this if I have time before I knock off today) * Sort of columns in table should probably hook into the mask as well, so it is clear that something is happening, given the long pause before anything is returned to update the table.
There was another issue I hit, but completely unrelated to this patch, so I will send a mail describing that to the list in a bit.
-j
On 07/20/2010 10:13 PM, Jason Guiditta wrote:
On Thu, 2010-07-15 at 16:18 +0200, Jan Provaznik wrote:
On 07/13/2010 10:28 PM, Jason Guiditta wrote:
On Tue, 2010-07-13 at 14:32 +0200, jprovazn@redhat.com wrote:
From: Jan Provaznikjprovazn@redhat.com
Replaced Datatables plugin with simpler HTML table because some issues when JS is off. Major issue with Datatables is that it doesn't degrades gracefully when JS is off and server-side pagination.
Images and instances are now displayed by HTML table (there is new helper 'paginated_table'), this table keeps all common functions like sorting, paginating, searching and code is cleaner. Works with JS on or off.
ACK. Much improved, thanks for the second rev here. However, there are a couple small issues that should be addressed (separately, not in this patch):
- When javascript is off, if you click a pool in left nav, you get an
unstyled list instead of tabs. Instead, this should still appear to be tabs, and clicking them should just cause a page reload. With a minor caveat, I suspect this should be as simple as setting a pre-defined style from jq-ui on the list (at least it used to work this way). If not, we'll need to mix that style into our compass stuff so the tabs always look right. Tomas, this might be a good one for you to look at if you have some time.
- When javascript is off, clicking the link in the list above still
returns the partial only. It should instead return the full layout with tab selected. (I may have a quick at least partial fix for this if I have time before I knock off today)
This issue is relevant to previous "unstyled list instead of tabs" issue and will be fixed with it (there is ajax=true param for tab links even if JS is off).
- Sort of columns in table should probably hook into the mask as well,
so it is clear that something is happening, given the long pause before anything is returned to update the table.
Patch is on the way.
There was another issue I hit, but completely unrelated to this patch, so I will send a mail describing that to the list in a bit.
-j
From: Jan Provaznik jprovazn@redhat.com
--- src/public/javascripts/application.js | 16 ++++++++++++---- 1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/public/javascripts/application.js b/src/public/javascripts/application.js index 1af4f77..5b2e8ad 100644 --- a/src/public/javascripts/application.js +++ b/src/public/javascripts/application.js @@ -11,10 +11,18 @@ var Aggregator = { $(".search_field", table).css('display', 'block');
// make column head links ajax - $("table thead tr th a", table).click(function() {wrapper.load($(this).attr('href'));return false;}); + $("table thead tr th a", table).click(function() { + wrapper.mask('Loading...'); + wrapper.load($(this).attr('href'), function() {wrapper.unmask()}); + return false; + });
// pagination links should be ajax too - $(".pagination a", table).click(function() {wrapper.load($(this).attr('href'));return false;}); + $(".pagination a", table).click(function() { + wrapper.mask('Loading...'); + wrapper.load($(this).attr('href'), function() {wrapper.unmask()}); + return false; + });
// check all checkboxes when checking "check all" $("input[name='check_all']", table).click(function() {$("input[name='ids[]']", table).attr('checked', $(this).attr('checked'))}); @@ -41,11 +49,11 @@ var Aggregator = { } if (table.searching) clearTimeout(table.searching); table.searching = setTimeout(function() { - $(".wrapper", table).mask('Searching...'); + wrapper.mask('Searching...'); table.search_lock = false; var search = $(".search_field input", table).val(); var url = $("form", table).attr('action') + '&search=' + search; - wrapper.load(url, function() {$(".wrapper", table).unmask()}); + wrapper.load(url, function() {wrapper.unmask()}); }, delay); }); }
Sending "Reworked images and instances listing" patch 2/2 as attachment because of long lines issue.
On Mon, 2010-07-12 at 10:56 +0200, Jan Provaznik wrote:
Sending "Reworked images and instances listing" patch 2/2 as attachment because of long lines issue.
ACK, pending changes in 1/2. Too bad this did note degrade as nicely as we hoped, looked like a pretty good plugin otherwise. However, I have to say that making a paginated table doesn't have to be that hard unless you are trying to do really fancy things, so your updated direction is just fine (and probably what I would have done if working on this chunk of functionality as well).
On Mon, 2010-07-12 at 10:53 +0200, jprovazn@redhat.com wrote:
Hi, I'm sending again updated patch which is applicable on current next branch (older patch isn't).
Datatables plugin (used for images and instances listing) has some issues when JS is off and pagination is server-side, this patch replaces it with simpler/cleaner HTML table which works better without JS.
I'll review this one tomorrow.
-j
deltacloud-devel@lists.fedorahosted.org