From 764f185969ed5a1cbe3d4d7958ce53053d42a20a Mon Sep 17 00:00:00 2001 From: Martin Tomov Date: Wed, 28 Dec 2016 09:47:50 +0000 Subject: [PATCH 1/2] Change `cancel` button tooltip to orange from red - by suggestion from @Mandily as the red is The red is associated with delete/destroy --- .../stylesheets/spree/backend/plugins/_bootstrap_tooltip.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/app/assets/stylesheets/spree/backend/plugins/_bootstrap_tooltip.scss b/backend/app/assets/stylesheets/spree/backend/plugins/_bootstrap_tooltip.scss index 35d950d7abb..98c8f3b5ee6 100644 --- a/backend/app/assets/stylesheets/spree/backend/plugins/_bootstrap_tooltip.scss +++ b/backend/app/assets/stylesheets/spree/backend/plugins/_bootstrap_tooltip.scss @@ -38,10 +38,10 @@ &.action-edit, &.action-save, &.action-capture, &.action-add { @include bootstrap-tooltip-color($brand-success); } - &.action-clone { + &.action-clone, &.action-cancel { @include bootstrap-tooltip-color($brand-warning); } - &.action-void, &.action-failure, &.action-cancel, &.action-remove { + &.action-void, &.action-failure, &.action-remove { @include bootstrap-tooltip-color($brand-danger); } } From 306eb309dc314eb45e385000ca1ca1838cad26e1 Mon Sep 17 00:00:00 2001 From: Martin Tomov Date: Tue, 8 Nov 2016 20:01:15 +0000 Subject: [PATCH 2/2] Allow users to inline update the variant of an image in admin - On clicking an image row, users are able to select the variant of the image from a dropdown and persist the change using Enter from keyboard or the OK button to the right of the row - Use Escape or Cancel button to revert back without persisting the change The editing table functionality is generic enough to be used accross on similar table to allow users to edit the contents of the tables inline. Instructions to turn a table into an inline-editable one: 1. Add `inline-editable-table` class to the table 2. Add `check` and `cancel` buttons, and specify the update url on the `check/save` button. 3. Change the static text into input fields See `backend/app/views/spree/admin/images/_image_row.html.erb` for a usage example --- .../spree/api/images_controller.rb | 4 +- .../app/assets/javascripts/spree/backend.js | 1 + .../components/editable_table.js.coffee | 3 + .../javascripts/spree/backend/views/index.js | 2 + .../views/tables/editable_table.js.coffee | 14 +++++ .../views/tables/editable_table_row.js.coffee | 45 +++++++++++++++ .../backend/components/_editable_table.scss | 37 +++++++++++++ .../spree/backend/sections/_payments.scss | 14 ----- .../spree/backend/shared/_tables.scss | 2 +- .../spree/backend/shared/_utilities.scss | 13 +++++ .../spree/backend/spree_admin.scss | 2 +- .../app/helpers/spree/admin/images_helper.rb | 17 ------ .../helpers/spree/admin/navigation_helper.rb | 2 +- .../spree/admin/images/_image_row.html.erb | 26 +++++++-- .../views/spree/admin/images/create.js.erb | 13 +++-- .../views/spree/admin/images/index.html.erb | 7 ++- .../views/spree/admin/payments/_list.html.erb | 2 +- .../admin/products/edit/images_spec.rb | 55 ++++++++++++++++--- 18 files changed, 201 insertions(+), 58 deletions(-) create mode 100644 backend/app/assets/javascripts/spree/backend/components/editable_table.js.coffee create mode 100644 backend/app/assets/javascripts/spree/backend/views/tables/editable_table.js.coffee create mode 100644 backend/app/assets/javascripts/spree/backend/views/tables/editable_table_row.js.coffee create mode 100644 backend/app/assets/stylesheets/spree/backend/components/_editable_table.scss delete mode 100644 backend/app/assets/stylesheets/spree/backend/sections/_payments.scss delete mode 100644 backend/app/helpers/spree/admin/images_helper.rb diff --git a/api/app/controllers/spree/api/images_controller.rb b/api/app/controllers/spree/api/images_controller.rb index 5457fa8fe41..6993b692360 100644 --- a/api/app/controllers/spree/api/images_controller.rb +++ b/api/app/controllers/spree/api/images_controller.rb @@ -18,13 +18,13 @@ def create end def update - @image = scope.images.accessible_by(current_ability, :update).find(params[:id]) + @image = Spree::Image.accessible_by(current_ability, :update).find(params[:id]) @image.update_attributes(image_params) respond_with(@image, default_template: :show) end def destroy - @image = scope.images.accessible_by(current_ability, :destroy).find(params[:id]) + @image = Spree::Image.accessible_by(current_ability, :destroy).find(params[:id]) @image.destroy respond_with(@image, status: 204) end diff --git a/backend/app/assets/javascripts/spree/backend.js b/backend/app/assets/javascripts/spree/backend.js index 20ae30e3982..5beba64ea54 100644 --- a/backend/app/assets/javascripts/spree/backend.js +++ b/backend/app/assets/javascripts/spree/backend.js @@ -32,6 +32,7 @@ //= require spree/backend/components/number_with_currency //= require spree/backend/components/tabs //= require spree/backend/components/tooltips +//= require spree/backend/components/editable_table //= require spree/backend/flash //= require spree/backend/gateway //= require spree/backend/handlebars_extensions diff --git a/backend/app/assets/javascripts/spree/backend/components/editable_table.js.coffee b/backend/app/assets/javascripts/spree/backend/components/editable_table.js.coffee new file mode 100644 index 00000000000..87761482328 --- /dev/null +++ b/backend/app/assets/javascripts/spree/backend/components/editable_table.js.coffee @@ -0,0 +1,3 @@ +Spree.ready -> + $('.inline-editable-table tr').each -> + Spree.Views.Tables.EditableTable.add $(this) diff --git a/backend/app/assets/javascripts/spree/backend/views/index.js b/backend/app/assets/javascripts/spree/backend/views/index.js index 37ab061b8f0..c75b9ddce56 100644 --- a/backend/app/assets/javascripts/spree/backend/views/index.js +++ b/backend/app/assets/javascripts/spree/backend/views/index.js @@ -14,3 +14,5 @@ //= require 'spree/backend/views/number_with_currency' //= require 'spree/backend/views/payment/new' //= require 'spree/backend/views/payment/edit_credit_card' +//= require 'spree/backend/views/tables/editable_table' +//= require 'spree/backend/views/tables/editable_table_row' diff --git a/backend/app/assets/javascripts/spree/backend/views/tables/editable_table.js.coffee b/backend/app/assets/javascripts/spree/backend/views/tables/editable_table.js.coffee new file mode 100644 index 00000000000..4989a2130f0 --- /dev/null +++ b/backend/app/assets/javascripts/spree/backend/views/tables/editable_table.js.coffee @@ -0,0 +1,14 @@ +Spree.Views.Tables ||= {} + +class Spree.Views.Tables.EditableTable + @add: ($el) -> + new Spree.Views.Tables.EditableTableRow el: $el + + @append: (html) -> + $row = $(html) + + $('#images-table').removeClass('hidden').find('tbody').append($row) + $row.find('.select2').select2() + $('.no-objects-found').hide() + + @add($row) diff --git a/backend/app/assets/javascripts/spree/backend/views/tables/editable_table_row.js.coffee b/backend/app/assets/javascripts/spree/backend/views/tables/editable_table_row.js.coffee new file mode 100644 index 00000000000..bda612739d6 --- /dev/null +++ b/backend/app/assets/javascripts/spree/backend/views/tables/editable_table_row.js.coffee @@ -0,0 +1,45 @@ +Spree.Views.Tables.EditableTableRow = Backbone.View.extend + events: + "select2-open": "onEdit" + "focus input": "onEdit" + "click [data-action=save]": "onSave" + "click [data-action=cancel]": "onCancel" + 'keyup input': 'onKeypress' + + onEdit: (e) -> + return if @$el.hasClass('editing') + @$el.addClass('editing') + + @$el.find('input, select').each -> + $input = $(this) + $input.data 'original-value', $input.val() + + onCancel: (e) -> + e.preventDefault() + @$el.removeClass("editing") + + @$el.find('input, select').each -> + $input = $(this) + originalValue = $input.data('original-value') + $input.val(originalValue).change() + + onSave: (e) -> + e.preventDefault() + + Spree.ajax @$el.find('.actions [data-action=save]').attr('href'), + data: @$el.find('select, input').serialize() + dataType: 'json' + method: 'put' + success: (response) => + @$el.removeClass("editing") + error: (response) => + show_flash 'error', response.responseJSON.error + + ENTER_KEY: 13 + ESC_KEY: 27 + + onKeypress: (e) -> + key = e.keyCode || e.which + switch key + when @ENTER_KEY then @onSave(e) + when @ESC_KEY then @onCancel(e) diff --git a/backend/app/assets/stylesheets/spree/backend/components/_editable_table.scss b/backend/app/assets/stylesheets/spree/backend/components/_editable_table.scss new file mode 100644 index 00000000000..f257a45ff23 --- /dev/null +++ b/backend/app/assets/stylesheets/spree/backend/components/_editable_table.scss @@ -0,0 +1,37 @@ +.inline-editable-table tr:not(:hover):not(.editing) { + input { + border: 1px solid transparent; + color: inherit; + background-color: transparent; + } + + .select2-arrow { + display: none; + } + + .select2-choice { + background-color: transparent; + border-color: transparent; + color: inherit; + } + + .select2-chosen { + color: inherit; + } +} + +.inline-editable-table tr { + .fa-check, .fa-cancel { + display: none !important; + } + + &.editing { + .fa-check, .fa-cancel { + display: inline-block !important; + } + + .fa { + display: none; + } + } +} diff --git a/backend/app/assets/stylesheets/spree/backend/sections/_payments.scss b/backend/app/assets/stylesheets/spree/backend/sections/_payments.scss deleted file mode 100644 index 4138ccfe143..00000000000 --- a/backend/app/assets/stylesheets/spree/backend/sections/_payments.scss +++ /dev/null @@ -1,14 +0,0 @@ -/* hide and show elements based on editing */ -tr.payment { - .edit-payment-amount { - text-align: right; - } - - &:not(.editing) .editing-show { - display: none; - } - - &.editing .editing-hide { - display: none; - } -} diff --git a/backend/app/assets/stylesheets/spree/backend/shared/_tables.scss b/backend/app/assets/stylesheets/spree/backend/shared/_tables.scss index 4a9fd22e3ac..8fdc4984be2 100644 --- a/backend/app/assets/stylesheets/spree/backend/shared/_tables.scss +++ b/backend/app/assets/stylesheets/spree/backend/shared/_tables.scss @@ -40,7 +40,7 @@ table { font-size: $font-size-base; } - [class*='fa-'].no-text { + .fa { font-size: 120%; background-color: very-light($color-3); border: 1px solid $color-border; diff --git a/backend/app/assets/stylesheets/spree/backend/shared/_utilities.scss b/backend/app/assets/stylesheets/spree/backend/shared/_utilities.scss index 4b1d4c8ac84..6c319775fc9 100644 --- a/backend/app/assets/stylesheets/spree/backend/shared/_utilities.scss +++ b/backend/app/assets/stylesheets/spree/backend/shared/_utilities.scss @@ -20,6 +20,19 @@ display: none; } +/* hide and show elements based on editing */ +.editing .editing-hide { + display: none; +} + +.editing .editing-show { + display: block; +} + +.editing-show { + display: none; +} + // For block grids .frameless { margin-left: -10px; diff --git a/backend/app/assets/stylesheets/spree/backend/spree_admin.scss b/backend/app/assets/stylesheets/spree/backend/spree_admin.scss index 84839db4fb1..c4f05523c3b 100644 --- a/backend/app/assets/stylesheets/spree/backend/spree_admin.scss +++ b/backend/app/assets/stylesheets/spree/backend/spree_admin.scss @@ -35,6 +35,7 @@ @import 'spree/backend/components/sidebar'; @import 'spree/backend/components/number_field_update'; @import 'spree/backend/components/stock_table'; +@import 'spree/backend/components/editable_table'; @import 'spree/backend/components/tabs'; @import 'font-awesome'; @@ -56,4 +57,3 @@ @import 'spree/backend/sections/transfer_items'; @import 'spree/backend/sections/stock_transfers'; @import 'spree/backend/sections/style_guide'; -@import 'spree/backend/sections/payments'; diff --git a/backend/app/helpers/spree/admin/images_helper.rb b/backend/app/helpers/spree/admin/images_helper.rb deleted file mode 100644 index eed14efb912..00000000000 --- a/backend/app/helpers/spree/admin/images_helper.rb +++ /dev/null @@ -1,17 +0,0 @@ -module Spree - module Admin - module ImagesHelper - def options_text_for(image) - if image.viewable.is_a?(Spree::Variant) - if image.viewable.is_master? - Spree.t(:all) - else - image.viewable.sku_and_options_text - end - else - Spree.t(:all) - end - end - end - end -end diff --git a/backend/app/helpers/spree/admin/navigation_helper.rb b/backend/app/helpers/spree/admin/navigation_helper.rb index 4209e7bf444..90d86a8f3f2 100644 --- a/backend/app/helpers/spree/admin/navigation_helper.rb +++ b/backend/app/helpers/spree/admin/navigation_helper.rb @@ -103,7 +103,7 @@ def link_to_delete(resource, options = {}) url = options[:url] || object_url(resource) name = options[:name] || Spree.t('actions.delete') confirm = options[:confirm] || Spree.t(:are_you_sure) - options[:class] = "delete-resource" + options[:class] = "#{options[:class]} delete-resource".strip options[:data] = { confirm: confirm, action: 'remove' } link_to_with_icon 'trash', name, url, options end diff --git a/backend/app/views/spree/admin/images/_image_row.html.erb b/backend/app/views/spree/admin/images/_image_row.html.erb index 4000574cef2..023a14e90f6 100644 --- a/backend/app/views/spree/admin/images/_image_row.html.erb +++ b/backend/app/views/spree/admin/images/_image_row.html.erb @@ -1,22 +1,40 @@ + <% if can?(:update_positions, Spree::Image) %> <% end %> + <%= link_to image.attachment.url(:product) do %> <%= render 'spree/admin/shared/image', image: image, size: :mini %> <% end %> + <% if @product.has_variants? %> - <%= options_text_for(image) %> - <% end %> - <%= image.alt %> + + <%= fields_for image do |f| %> + <%= f.select :viewable_id, options_for_select(@variants, image.viewable_id), {}, class: 'select2 fullwidth', autocomplete: "off" %> + <% end %> + + <% end # @product.has_variants? %> + + + <%= fields_for image do |f| %> + <%= f.text_field :alt %> + <% end %> + + <% if can?(:update, image) %> - <%= link_to_with_icon 'edit', Spree.t('actions.edit'), edit_admin_product_image_url(@product, image), no_text: true, data: {action: 'edit'} %> + <%= link_to_with_icon 'check', Spree.t('actions.save'), api_variant_image_path(@product, image), no_text: true, data: {action: 'save'} %> + + <%= link_to_with_icon 'cancel', Spree.t('actions.cancel'), nil, no_text: true, data: {action: 'cancel'} %> + + <%= link_to_with_icon 'edit', Spree.t('actions.edit'), edit_admin_product_image_path(@product, image), no_text: true, data: {action: 'edit'} %> <% end %> + <% if can?(:destroy, image) %> <%= link_to_delete image, { url: admin_product_image_url(@product, image), no_text: true } %> <% end %> diff --git a/backend/app/views/spree/admin/images/create.js.erb b/backend/app/views/spree/admin/images/create.js.erb index 39ca95f65e2..76f3d9fdc14 100644 --- a/backend/app/views/spree/admin/images/create.js.erb +++ b/backend/app/views/spree/admin/images/create.js.erb @@ -1,9 +1,12 @@ -var uploadedRow = $('[data-upload-id="<%= params[:upload_id] %>"]'); +var $uploadEl = $('[data-upload-id="<%= params[:upload_id] %>"]'); <% if @image.persisted? %> - uploadedRow.trigger('clear'); - $('#images-table').removeClass('hidden').find('tbody').append('<%= j render partial: "image_row", locals: {image: @image } %>'); - $('.no-objects-found').hide(); + + $uploadEl.trigger('clear'); + Spree.Views.Tables.EditableTable.append('<%= j render "image_row", image: @image %>'); + <% else %> - uploadedRow.find('error').removeClass('hidden').html('<%= j @image.errors.full_messages.join("
").html_safe %>'); + + $uploadEl.find('error').removeClass('hidden').html('<%= j @image.errors.full_messages.join("
").html_safe %>'); + <% end %> diff --git a/backend/app/views/spree/admin/images/index.html.erb b/backend/app/views/spree/admin/images/index.html.erb index eaf4ee9ea27..8d98af63952 100644 --- a/backend/app/views/spree/admin/images/index.html.erb +++ b/backend/app/views/spree/admin/images/index.html.erb @@ -37,16 +37,17 @@ <% no_images = @product.images.empty? && @product.variant_images.empty? %> - +
<% if @product.has_variants? %> - + <% end %> - + + diff --git a/backend/app/views/spree/admin/payments/_list.html.erb b/backend/app/views/spree/admin/payments/_list.html.erb index 6687e040f83..81703dd2dba 100644 --- a/backend/app/views/spree/admin/payments/_list.html.erb +++ b/backend/app/views/spree/admin/payments/_list.html.erb @@ -16,7 +16,7 @@ diff --git a/backend/spec/features/admin/products/edit/images_spec.rb b/backend/spec/features/admin/products/edit/images_spec.rb index 429e890b730..a5395dcd88b 100644 --- a/backend/spec/features/admin/products/edit/images_spec.rb +++ b/backend/spec/features/admin/products/edit/images_spec.rb @@ -30,22 +30,33 @@ click_button "Update" expect(page).to have_content("successfully created!") + # Icons are hidden, so hover to have them pop-up + find('tbody > tr').hover within_row(1) do - click_icon(:edit) + within ".actions" do + click_icon :edit + end end + fill_in "image_alt", with: "ruby on rails t-shirt" click_button "Update" - expect(page).to have_content("successfully updated!") - expect(page).to have_content("ruby on rails t-shirt") + expect(page).to have_content "successfully updated!" + expect(page).to have_field "image[alt]", with: "ruby on rails t-shirt" + + find('tbody > tr').hover accept_alert do click_icon :trash end - expect(page).not_to have_content("ruby on rails t-shirt") + expect(page).not_to have_field "image[alt]", with: "ruby on rails t-shirt" end end context 'Via the upload zone', js: true do + before do + create(:variant, product: product) + end + it "uploads an image with ajax and appends it to the images table" do visit spree.admin_product_images_path(product) expect(page).to have_content("No images found") @@ -58,24 +69,30 @@ expect(page).not_to have_content("No images found") within("table.index") do - expect(page).to have_css("tbody tr", count: 1) + expect(page).to have_css "tbody tr", count: 1 within("tbody") do - expect(page).to have_xpath("//img[contains(@src,'ror_ringer')]") + expect(page).to have_xpath "//img[contains(@src,'ror_ringer')]" + expect(page).to have_content "All" end + + # Change the image to the other variant + targetted_select2 "Size: S", from: "#s2id_image_viewable_id" + click_icon :check + expect(page).to have_content "Size: S" end expect(Spree::Image.last.viewable).to eq(product.master) end end - # Regression test for https://github.com/spree/spree/issues/2228 - it "should see variant images" do + it "should see variant images and allow for inline changing the image's variant", js: true do variant = create(:variant) variant.images.create!(attachment: File.open(file_path)) visit spree.admin_product_images_path(variant.product) expect(page).not_to have_content("No Images Found.") + within("table.index") do expect(page).to have_content(variant.options_text) @@ -87,10 +104,30 @@ expect(page).to have_content("Variant") end - # ensure variant header is displayed within("tbody") do expect(page).to have_content("Size: S") end + + # Do an inline change of variant and alt + targetted_select2 "All", from: "#s2id_image_viewable_id" + fill_in 'image[alt]', with: 'ruby on rails t-shirt' + click_icon :check + + expect(page).to have_content "All" + expect(page).to have_field "image[alt]", with: "ruby on rails t-shirt" + + # test escape + find("#image_alt").click # to focus + fill_in 'image[alt]', with: 'red shirt' + find("#image_alt").send_keys(:escape) + expect(page).to have_field "image[alt]", with: "ruby on rails t-shirt" + + # And then go back to Size S variant, but using Enter key + targetted_select2 "Size: S", from: "#s2id_image_viewable_id" + find("#s2id_image_viewable_id").send_keys(:return) + + expect(page).to have_content "Size: S" + expect(page).to have_field "image[alt]", with: "ruby on rails t-shirt" end end
<%= Spree.t(:thumbnail) %><%= link_to payment.number, spree.admin_order_payment_path(@order, payment) %> <%= pretty_time(payment.created_at) %> - <%= text_field_tag :amount, payment.amount, class: 'js-edit-amount edit-payment-amount editing-show' %> + <%= text_field_tag :amount, payment.amount, class: 'js-edit-amount align-right editing-show' %> <%= payment.display_amount.to_html %> <%= payment_method_name(payment) %>