Currently to assign a product attribute to a product variation type you must create an entity reference field that references "Product attribute value", and select the right attribute (Color, size, etc) on the field settings page.

We can make this easier. When you add/edit the product variation type, right on that form we can provide a set of "Product attribute" checkboxes.
Selecting or deselecting Size, Color, etc will create or delete* the appropriate field.
We don't need to store this list on the product type, we can figure it out based on those type's field definitions.

* - We'll refuse to unselect an attribute if the field already has data inside.

CommentFileSizeAuthor
#10 interdiff-9-4.txt1.59 KBmglaman
#10 allow_product-2689519-10.patch6.23 KBmglaman
#9 Edit_Default___Site-Install.png59.12 KBmglaman
#9 allow_product-2689519-9.patch6.38 KBmglaman
#4 attributes-variation-form2.png105.22 KBbojanz
#4 attributes-variation-form1.png82.29 KBbojanz
#4 2689519-4-product-attributes.patch5.78 KBbojanz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bojanz created an issue. See original summary.

Londova’s picture

Londova CreditAttribution: Londova commented

This is exactly I was looking for.

bojanz’s picture

bojanz CreditAttribution: bojanz commented
Assigned: Unassigned » bojanz

#2690685: Create an attribute field manager has added the API for this ($attibute_field_manager ->createField(), canDeleteField(), deleteField()), what remains is for ProductVariationTypeForm to use it.

I'll go ahead and do that.

bojanz’s picture

bojanz CreditAttribution: bojanz commented
Status: Active » Needs review
FileSize
2689519-4-product-attributes.patch5.78 KB
attributes-variation-form1.png82.29 KB
attributes-variation-form2.png105.22 KB

Here's a working patch.

Looking for visual feedback.
- Is "Attributes" enough for the label? Do we need a description?
- Is there a better wording for the error message shown when there are already variations with that attribute?

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented 
- Is there a better wording for the error message shown when there are already variations with that attribute?

We should disable it, so they do not have the option to even uncheck it.

Which means we maybe need a description describing the "once added with data, cannot be removed."

bojanz’s picture

bojanz CreditAttribution: bojanz commented

We can't disable one checkbox out of a set.

bojanz’s picture

bojanz CreditAttribution: bojanz commented
Assigned: bojanz » mglaman

Okay, mglaman confirmed he knows a way. That + a small extension of the ProductVariationTypeFormTest and we're good to go.

skyredwang’s picture

skyredwang
Location Shanghai
CreditAttribution: skyredwang at INsReady commented

I tested #4 patch, it works beautifully. Please merge?

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Assigned: mglaman » Unassigned
Issue summary: View changes
FileSize
allow_product-2689519-9.patch6.38 KB
Edit_Default___Site-Install.png59.12 KB

Here is my enhancement that disabled checkboxes which have fields which are not deletable.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
FileSize
allow_product-2689519-10.patch6.23 KB
interdiff-9-4.txt1.59 KB

Here is a revised version, and interdiff from #4.

  • bojanz committed 821711d on 8.x-2.x 
    Issue #2689519 by mglaman, bojanz: Allow product attributes to be...
mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Status: Needs review » Fixed

Fix. 821711d had test regression, committed fix to make tests pass.

bojanz’s picture

bojanz CreditAttribution: bojanz commented

Compared to #10 I made the following changes:
1) The #disabled check crashed if only some of the fields were used, since it would pass a non-used field to canDelete().
2) The attribute checkboxes element was not hidden when there are no attributes.
3) The validateForm logic was unneeded with the new #disabled handling.

(Comment was pending until I fixed the tests, Matt figured it out first)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

rei’s picture

rei CreditAttribution: rei commented
Status: Closed (fixed) » Fixed

any chance to port this to commerce 1.x ?

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Status: Fixed » Closed (fixed)

No, because in 1.x attributes are a field setting when using a List or Reference field.