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/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); } } 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) %>