Main image does not change for more than one attribute 3.90

This topic was automatically closed 365 days after the last reply. New replies are no longer allowed.
6 years ago
Create a Product.

Create a product attributes ex. Color Squares. 4 values. Edit a value and assign an  existing product image.

Create another product attribute. Drop down. 3 values. Edit a value and choose an existing product image.

The first attribute color selection will change the product image. works fine, however when you choose the drop down list that has images assigned to each value, The second attribute will not change the main image.

Thanks, any solution yet for this issue?
6 years ago
Hi,

Thanks! We'll investigate it soon - https://github.com/nopSolutions/nopCommerce/issues/2348
6 years ago
a.m. wrote:
Hi,

Thanks! We'll investigate it soon - https://github.com/nopSolutions/nopCommerce/issues/2348


Confirmed issue is in both 3.8 and 3.9.  I believe this occurred during the refactoring of ProductDetails_AttributeChange sometime during 3.8 development.

Andrei...so you can spot it quickly, see below.  You always return the first image for a value found in the first product attribute that has a picture.  It looks like based on your comments in code, you were about to do some additional changes related to combination image support.  I am patching this now myself, but definitely should be fixed officially in 4.0. Actually something like this might warrant a update to your branch "release-3.90-bug-fixes" as this is a pretty significant bug.

if (loadPicture)
            {
                //just load (return) the first found picture (in case if we have several distinct attributes with associated pictures)
                //actually we're going to support pictures associated to attribute combinations (not attribute values) soon. it'll more flexible approach
                var attributeValues = _productAttributeParser.ParseProductAttributeValues(attributeXml);
                var attributeValueWithPicture = attributeValues.FirstOrDefault(x => x.PictureId > 0);
                if (attributeValueWithPicture != null)
                {
...


By the way, looking at your comment you put in your code block, I had previously added support myself for pictures in attribute combinations.  Adding PictureId to ProductAttributeCombinations made this very easy;
I have this...

var combination = _productAttributeService.GetProductAttributeCombinationBySku(sku);
                if (combination != null)
                {
                    if (combination.PictureId > 0)
                    {
                        var valuePicture = _pictureService.GetPictureById(combination.PictureId);

                        if (valuePicture != null)
                        {
                            var pictureModel = new PictureModel
                            {
                                FullSizeImageUrl = _pictureService.GetPictureUrl(valuePicture),
                                ImageUrl = _pictureService.GetPictureUrl(valuePicture, _mediaSettings.ProductDetailsPictureSize)
                            };
                            pictureFullSizeUrl = pictureModel.FullSizeImageUrl;
                            pictureDefaultSizeUrl = pictureModel.ImageUrl;
                        }
                    }
                    else
                    {
                      //current ootb attribute value image check
                      ...
                    }
                
                }
else
{
   //current ootb attribute value image check
                      ...
}

Very handy to handle scenarios like... a red shirt that is large vs. a red shirt that is medium. Without combination support, you can't get a picture of a red shirt that is large without worrying if someone picks a green shirt first.

Thanks
Charles
6 years ago
Chuck, do you have a fix for this bug? I am not sure when the bug release is coming out or if this will make it into that release. an example on how to fix this in 3.90 would be great if you have it. Thanks,
6 years ago
pfeighan wrote:
Chuck, do you have a fix for this bug? I am not sure when the bug release is coming out or if this will make it into that release. an example on how to fix this in 3.90 would be great if you have it. Thanks,


Hi there,

Well this is tagged as a 4.0 bug fix...so I guess the official fix will be sept/oct this year... But then you will have to upgrade of course.  I am hoping this makes it into the 3.9 bug fix branch.

For anyone using 3.8 or 3.9 that needs this functionality, there is really only a couple options;

The real fix is to pass into the constructor the "specific" attribute that was touched/triggered the event.
Presently, the constructor looks like this;

public ActionResult ProductDetails_AttributeChange(int productId, bool validateAttributeConditions,
            bool loadPicture, FormCollection form)

The routine parses the form collection which contains all the attributes (and their selected values).  But what is missing is "which" attribute value change triggerd the function. This makes it impossible out of the box to really determine what image you should display if you have more than 1 product attribute with more than 1 picture value selected in at least 2 of your attributes.

eg.
dropdown 1 (red / blue / green) shirt  with an image assoicated to each
and ...
dropdown 2 (high collar / low collar ) shirt with an image associated to each
and so on with more dropdowns.

The ootb code ALWAYS selects the first pictureId in order. So because they are using FirstOrDefault()... If a value that has an associated image in dropdown 1 is selected, no matter what you try to do with dropdown 2 and the others, your image will not change.

Technically....the more I thought about this, I guess its not a bug. Why, because the nop team put code comments saying they were not going to support image attributes the way you thought.
They wrote the following;
//just load (return) the first found picture (in case if we have several distinct attributes with associated pictures)
                    //actually we're going to support pictures associated to attribute combinations (not attribute values) soon. it'll more flexible approach

So, I say this is not a bug as it was not an accident you have encountered this. The perception of nop supporting attribute images is valid, it's just limited to a single dropdown.
I suppose the real issue is the attribute combination image support was not done for 3.8 or 3.9 and now the feature is in limbo.

Assuming you need this, and you can't downgrade to 3.7 you could do the following;
Leverage conditional attributes to make your dropdowns cascade. eg. in my simple case above, force someone to choose dropdown1 before allowing dropdown2 selection.  Also, make sure dependent attributes are optional so you get a select/-- at index 0.  This way, if someone changes their mind, and flips dropdown1 again, everything resets.
Then simply change FirstOrDefault to LastOrDefault (recompile and replace the nop.web dll in your bin directory).  Now the last dropdown you touch...if it has an image value associated with it, it will win.  This is the least intrusive way to work around this and requires 1 word change in code and then just leverage ootb conditions between your dropdowns.

Now, above is not what I did to fix the issue.  I actually changed the constructor and passed in "Which" product attribute triggered the function call.  So I look to see if a pictureId is associated with it, and if not, don't touch the image being displayed. This is the "real" fix but unfortunately affects the constructor and the view and is really moving away from core code (not recommended for non developers).  My logic also has combination image checks that override any single value image (basically, if I have an image that has value1 of dropdown1 and value2 of dropdown2 both selected, show the red low collar shirt image.  This is where the nop team is trying to get to in 4.0.

Thx
6 years ago
Chuck, THANK YOU !! worked like a charm. your quick response and detailed explanation was very helpful. You made my day. Paul
6 years ago
Happy to help
This topic was automatically closed 365 days after the last reply. New replies are no longer allowed.