Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UX improvements for creation of new products [OFN-12744] #12760

Merged
merged 6 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions app/components/searchable_dropdown_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ def initialize(
selected_option:,
placeholder_value:,
include_blank: false,
aria_label: ''
aria_label: '',
other_attrs: {}
)
@f = form
@name = name
Expand All @@ -20,11 +21,13 @@ def initialize(
@placeholder_value = placeholder_value
@include_blank = include_blank
@aria_label = aria_label
@other_attrs = other_attrs
end

private

attr_reader :f, :name, :options, :selected_option, :placeholder_value, :include_blank, :aria_label
attr_reader :f, :name, :options, :selected_option, :placeholder_value, :include_blank,
:aria_label, :other_attrs

def classes
"fullwidth #{remove_search_plugin? ? 'no-input' : ''}"
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
= f.select name, options_for_select(options, selected_option), { include_blank: }, class: classes, data:, 'aria-label': aria_label
= f.select name, options_for_select(options, selected_option), { include_blank: }, class: classes, data:, 'aria-label': aria_label, **other_attrs
10 changes: 2 additions & 8 deletions app/models/spree/product.rb
Original file line number Diff line number Diff line change
Expand Up @@ -295,14 +295,8 @@ def validate_variant_attrs
# eg clone product. Will raise error if clonning a product with no variant
return if variants.first&.valid?

unless Spree::Taxon.find_by(id: primary_taxon_id)
errors.add(:primary_taxon_id,
I18n.t('activerecord.errors.models.spree/product.must_exist'))
end
return if Enterprise.find_by(id: supplier_id)

errors.add(:supplier_id,
I18n.t('activerecord.errors.models.spree/product.must_exist'))
errors.add(:primary_taxon_id, :blank) unless Spree::Taxon.find_by(id: primary_taxon_id)
errors.add(:supplier_id, :blank) unless Enterprise.find_by(id: supplier_id)
dacook marked this conversation as resolved.
Show resolved Hide resolved
end

def update_units
Expand Down
2 changes: 1 addition & 1 deletion app/services/permitted_attributes/product.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def self.attributes
:unit_description, :variant_unit_name,
:display_as, :sku, :group_buy, :group_buy_unit_size,
:taxon_ids, :primary_taxon_id, :tax_category_id, :supplier_id,
:meta_keywords, :notes, :inherits_properties,
:meta_keywords, :notes, :inherits_properties, :shipping_category_id,
{ product_properties_attributes: [:id, :property_name, :value],
variants_attributes: [PermittedAttributes::Variant.attributes],
image_attributes: [:attachment] }
Expand Down
10 changes: 8 additions & 2 deletions app/views/spree/admin/products/_primary_taxon_form.html.haml
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
= f.field_container :primary_taxon do
= f.field_container :primary_taxon_id do
= f.label :primary_taxon_id, t('.product_category')
%span.required *
%br
= f.collection_select(:primary_taxon_id, Spree::Taxon.order(:name), :id, :name, {:include_blank => true}, {:class => "select2 fullwidth"})
= render(SearchableDropdownComponent.new(form: f,
name: :primary_taxon_id,
aria_label: t('.product_category'),
options: Spree::Taxon.select(:name, :id).order(:name).pluck(:name, :id),
selected_option: @product.primary_taxon_id,
include_blank: true,
placeholder_value: t('.search_for_categories')))
= f.error_message_on :primary_taxon_id
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
= f.field_container :shipping_categories do
= f.field_container :shipping_category_id do
= f.label :shipping_category_id, t(:shipping_category)
= f.collection_select(:shipping_category_id, Spree::ShippingCategory.all, :id, :name, {:include_blank => false}, {:class => 'select2 fullwidth'})
= f.error_message_on :shipping_category_id
33 changes: 24 additions & 9 deletions app/views/spree/admin/products/new.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@
%legend{align: "center"}= t(".new_product")
.sixteen.columns.alpha
.eight.columns.alpha
= f.field_container :supplier do
= f.label :supplier, t(".supplier")
= f.field_container :supplier_id do
= f.label :supplier_id, t(".supplier")
%span.required *
= f.select :supplier_id, options_from_collection_for_select(@producers, :id, :name, @product.supplier_id), { include_blank: t("spree.admin.products.new.supplier_select_placeholder") }, { "data-controller": "tom-select", class: "primary" }
= render(SearchableDropdownComponent.new(form: f,
name: :supplier_id,
aria_label: t('.supplier'),
options: @producers.select(:name, :id).order(:name).pluck(:name, :id),
selected_option: @product.supplier_id,
include_blank: true,
placeholder_value: t('.search_for_suppliers')))
= f.error_message_on :supplier_id
.eight.columns.omega
= f.field_container :name do
Expand All @@ -25,25 +31,34 @@
= f.field_container :variant_unit do
= f.label :variant_unit, t(".units")
%span.required *
%select{id: 'product_variant_unit_with_scale', 'ng-model' => 'product.variant_unit_with_scale', 'ng-options' => 'unit[1] as unit[0] for unit in variant_unit_options', "data-controller": "tom-select","data-tom-select-options-value": '{"allowEmptyOption":false}', class: "primary"}
%option{'value' => '', 'ng-hide' => "hasUnit(product)"}
= f.select 'variant_unit', [],
{ include_blank: true },
{ id: 'product_variant_unit_with_scale',
name: 'product_variant_unit_with_scale',
'ng-model' => 'product.variant_unit_with_scale',
'ng-options' => 'unit[1] as unit[0] for unit in variant_unit_options',
"data-controller": "tom-select",
"data-tom-select-options-value": '{"allowEmptyOption":false}',
class: "primary",
}
dacook marked this conversation as resolved.
Show resolved Hide resolved
%input{ type: 'hidden', 'ng-value': 'product.variant_unit', "ng-init": "product.variant_unit='#{@product.variant_unit}'", name: 'product[variant_unit]' }
%input{ type: 'hidden', 'ng-value': 'product.variant_unit_scale', "ng-init": "product.variant_unit_scale='#{@product.variant_unit_scale}'", name: 'product[variant_unit_scale]' }
= f.error_message_on :variant_unit
.two.columns
= f.field_container :unit_value do
= f.label :unit_value, t(".value"), 'ng-disabled' => "!hasUnit(product)"
%span.required *
%input.fullwidth{ id: 'product_unit_value', 'ng-model' => 'product.master.unit_value_with_description', :type => 'text', placeholder: "eg. 2", 'ng-disabled' => "!hasUnit(product)" }
= f.text_field :unit_value, placeholder: "eg. 2", 'ng-model' => 'product.master.unit_value_with_description', class: 'fullwidth', 'ng-disabled' => "!hasUnit(product)"
%input{ type: 'hidden', 'ng-value': 'product.master.unit_value', "ng-init": "product.master.unit_value='#{@product.unit_value}'", name: 'product[unit_value]' }
%input{ type: 'hidden', 'ng-value': 'product.master.unit_description', "ng-init": "product.master.unit_description='#{@product.unit_description}'", name: 'product[unit_description]' }
= f.error_message_on :unit_value
= render 'display_as', f: f
.six.columns.omega{ 'ng-show' => "product.variant_unit_with_scale == 'items'" }
= f.field_container :unit_name do
= f.label :product_variant_unit_name, t(".unit_name")
= f.field_container :variant_unit_name do
= f.label :variant_unit_name, t(".unit_name")
%span.required *
%input.fullwidth{ id: 'product_variant_unit_name','ng-model' => 'product.variant_unit_name', :name => 'product[variant_unit_name]', :placeholder => t('admin.products.unit_name_placeholder'), :type => 'text' }
= f.text_field :variant_unit_name, :placeholder => t('admin.products.unit_name_placeholder'), 'ng-model' => 'product.variant_unit_name', class: 'fullwidth', 'ng-init': "product.variant_unit_name='#{@product.variant_unit_name}'"
= f.error_message_on :variant_unit_name
.sixteen.columns.alpha
.eight.columns.alpha
= render 'spree/admin/products/primary_taxon_form', f: f
Expand Down
9 changes: 7 additions & 2 deletions app/webpacker/controllers/trixeditor_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,14 @@ import { Controller } from "stimulus";

export default class extends Controller {
connect() {
window.addEventListener("trix-change", this.#trixChange);
this.element.addEventListener("trix-change", this.#trixChange);
this.#trixInitialize();
window.addEventListener("trix-initialize", this.#trixInitialize);
this.element.addEventListener("trix-initialize", this.#trixInitialize);
wandji20 marked this conversation as resolved.
Show resolved Hide resolved
}

disconnect() {
this.element.removeEventListener("trix-change", this.#trixChange);
this.element.removeEventListener("trix-initialize", this.#trixInitialize);
}

#trixChange = (event) => {
Expand Down
5 changes: 3 additions & 2 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ en:
attributes:
base:
card_expired: "has expired"
spree/product:
must_exist: 'must exist'
order_cycle:
attributes:
orders_close_at:
Expand Down Expand Up @@ -4470,6 +4468,8 @@ See the %{link} to find out more about %{sitename}'s features and to start using
new_product: "New Product"
supplier: "Supplier"
supplier_select_placeholder: "Select a supplier"
search_for_suppliers: "Search for suppliers"
search_for_units: "Search for units"
product_name: "Product Name"
units: "Unit Size"
value: "Value"
Expand Down Expand Up @@ -4504,6 +4504,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using
product_name: Product Name
primary_taxon_form:
product_category: Product Category
search_for_categories: "Search for categories"
group_buy_form:
group_buy: "Group Buy?"
bulk_unit_size: Bulk unit size
Expand Down
4 changes: 2 additions & 2 deletions spec/system/admin/products_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@
click_button 'Create'

expect(current_path).to eq spree.admin_products_path
expect(page).to have_content "Product Category must exist"
expect(page).to have_content "Product Category can't be blank"
end

it "creating product with empty product supplier fails" do
Expand All @@ -201,7 +201,7 @@
click_button 'Create'

expect(current_path).to eq spree.admin_products_path
expect(page).to have_content "Supplier must exist"
expect(page).to have_content "Supplier can't be blank"
end

describe "localization settings" do
Expand Down
Loading