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

New feature: ConfigurableSwatches now allows for auto-generation of the swatch image file based on color selection #3686

Merged
merged 50 commits into from
Apr 25, 2024

Conversation

empiricompany
Copy link
Contributor

@empiricompany empiricompany commented Dec 3, 2023

Description (*)

I avoid installing third-party extensions to manage swatches that often need to be integrated into the template and cause issues and mess up the database.
But I also hate explaining to the client how to create and how to upload swatch images.
With this modification, it's possible to choose a color that will be used for dynamically creating the swatch.

The code only comes into play if a fallback image is not present in media/wysiwyg/swatches/optionvalue.png.
A resized image is created in cache directly to avoid conflicts with the original images in:
media/catalog/swatches/STOREID/WidthxHeight/optionvalue.png.
This way, it is perfectly compatible with the other modes that always take precedence.

demo
demo_picker
demo_delete

Manual testing scenarios (*)

  1. Configure color to be displayed as a swatch
  2. Create a new configurable product with the color attribute
  3. Create a new simple product with a color option that does not have a fallback image in media/wysiwyg/swatches

( If everything is correct, should display a broken image in product view and layered )

  1. Navigate to Manage Attributes, edit the color attribute, and select a color for the option with the color picker

Results:

A new swatch image is created on the fly directly in the cache media/catalog/swatches/ foreach requested dimensions, both for the product view and layered navigation filters.

Questions or comments

This feature is heavly inspired to magento 2 and can be extended to allow direct upload of swatch images in
media/wysywyg/swatches.
Use <input type="color" /> for color picker that seems is widely supported in Chrome and Firefox https://caniuse.com/input-color

It is not possible to set a default empty value for input type="color", so all colors are black by default, which is always better than a broken image (for now😉)

I had to update the EAV module by adding a new table eav_attribute_option_swatch because it seem to me more consistent with the database schema. If you have any alternative solutions, please let me know.
I have tested also with the Mage_ConfigurableSwatches module disabled.

Please comment only on the PR implementation; for anything else, it's best to open a separate discussion.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@github-actions github-actions bot added Template : admin Relates to admin template Component: Eav Relates to Mage_Eav Component: Adminhtml Relates to Mage_Adminhtml Component: ConfigurableSwatches Relates to Mage_ConfigurableSwatches labels Dec 3, 2023
@empiricompany empiricompany marked this pull request as ready for review December 3, 2023 13:35
@fballiano
Copy link
Contributor

great idea as usual, I'm not super familiar with the swatches, isn't it possible to generate a div with a css color background instead of having to create an image with a solid color? for sure you thought about it but I'm wondering, since it would avoid having files on the filesystem for a solid color.

@empiricompany
Copy link
Contributor Author

great idea as usual, I'm not super familiar with the swatches, isn't it possible to generate a div with a css color background instead of having to create an image with a solid color? for sure you thought about it but I'm wondering, since it would avoid having files on the filesystem for a solid color.

Yes, it would be better to create the swatch using CSS or an SVG, but this solution was designed to be as backward compatible as possible with other themes that inherit the same templates and are not modified by this PR.

@ADDISON74
Copy link
Contributor

I flushed the cache before. I checkout this PR and I am getting the following error:

Error in file: "/var/www/html/app/code/core/Mage/Eav/sql/eav_setup/upgrade-1.6.0.1-1.6.0.2.php" - SQLSTATE[HY000]: General error: 1005 Can't create table `db`.`eav_attribute_option_swatch` (errno: 121 "Duplicate key on write or update"), query was: CREATE TABLE `eav_attribute_option_swatch` (
  `value_id` int UNSIGNED NOT NULL auto_increment COMMENT 'Value Id' ,
  `option_id` int UNSIGNED NOT NULL default '0' COMMENT 'Option Id' ,
  `value` varchar(255) NULL default NULL COMMENT 'Value' ,
  `filename` varchar(255) NULL default NULL COMMENT 'Filename' ,
  PRIMARY KEY (`value_id`),
  UNIQUE `IDX_EAV_ATTRIBUTE_OPTION_VALUE_OPTION_ID` (`option_id`),
  CONSTRAINT `FK_EAV_ATTR_OPT_VAL_OPT_ID_EAV_ATTR_OPT_OPT_ID` FOREIGN KEY (`option_id`) REFERENCES `eav_attribute_option` (`option_id`) ON DELETE CASCADE ON UPDATE CASCADE
) COMMENT='Eav Attribute Option Swatch' ENGINE=INNODB charset=utf8 COLLATE=utf8_general_ci

#0 /var/www/html/app/code/core/Mage/Core/Model/Resource/Setup.php(628): Mage::exception()
#1 /var/www/html/app/code/core/Mage/Core/Model/Resource/Setup.php(424): Mage_Core_Model_Resource_Setup->_modifyResourceDb()
#2 /var/www/html/app/code/core/Mage/Core/Model/Resource/Setup.php(308): Mage_Core_Model_Resource_Setup->_upgradeResourceDb()
#3 /var/www/html/app/code/core/Mage/Core/Model/Resource/Setup.php(223): Mage_Core_Model_Resource_Setup->applyUpdates()
#4 /var/www/html/app/code/core/Mage/Core/Model/App.php(441): Mage_Core_Model_Resource_Setup::applyAllUpdates()
#5 /var/www/html/app/code/core/Mage/Core/Model/App.php(347): Mage_Core_Model_App->_initModules()
#6 /var/www/html/app/Mage.php(740): Mage_Core_Model_App->run()
#7 /var/www/html/index.php(56): Mage::run()
#8 {main}

@ADDISON74
Copy link
Contributor

ADDISON74 commented Feb 11, 2024

I ran the query below directly in the database and got the same error. I removed completely the CONSTRAIN line and I managed to create the table. It is not right, but there are some issues on that line.

CREATE TABLE `eav_attribute_option_swatch` (
  `value_id` int UNSIGNED NOT NULL auto_increment COMMENT 'Value Id' ,
  `option_id` int UNSIGNED NOT NULL default '0' COMMENT 'Option Id' ,
  `value` varchar(255) NULL default NULL COMMENT 'Value' ,
  `filename` varchar(255) NULL default NULL COMMENT 'Filename' ,
  PRIMARY KEY (`value_id`),
  UNIQUE `IDX_EAV_ATTRIBUTE_OPTION_VALUE_OPTION_ID` (`option_id`),
  CONSTRAINT `FK_EAV_ATTR_OPT_VAL_OPT_ID_EAV_ATTR_OPT_OPT_ID` FOREIGN KEY (`option_id`) REFERENCES `eav_attribute_option` (`option_id`) ON DELETE CASCADE ON UPDATE CASCADE
) COMMENT='Eav Attribute Option Swatch' ENGINE=INNODB charset=utf8 COLLATE=utf8_general_ci

This is the part in the PHP file which must be checked

    ->addForeignKey(
        $installer->getFkName('eav/attribute_option_value', 'option_id', 'eav/attribute_option', 'option_id'),
        'option_id',
        $installer->getTable('eav/attribute_option'),
        'option_id',
        Varien_Db_Ddl_Table::ACTION_CASCADE,
        Varien_Db_Ddl_Table::ACTION_CASCADE
    )

@ADDISON74
Copy link
Contributor

At the first evaluation, you have to be careful with the initial values, because if there are already options with values, a black swatch is associated for all of them. In the Magento Sample Pack in size and color attributes, I am getting a black swatch for all options.

screen

Not all attributes should have swatches. Also, the options of an attribute do not have to get a value. There are situations when you may not want it to get a swatch, for example in size case, I don't need swatches.

What is the priority when I set an image on the product edit page to be a swatch. Based on this custom code, it takes the global one defined in attribute page or the one in the product image tab?

@ADDISON74
Copy link
Contributor

If I change the name of a color in the attribute options page (e.g. from Blue to Blue Print), in the /media/wysiwyg/swatches directory, there is not PNG file with the new name. The fallback should work but I didn't find any directory in /media/catalog/swatches.

@empiricompany
Copy link
Contributor Author

I ran the query below directly in the database and got the same error. I removed completely the CONSTRAIN line and I managed to create the table. It is not right, but there are some issues on that line.

CREATE TABLE `eav_attribute_option_swatch` (
  `value_id` int UNSIGNED NOT NULL auto_increment COMMENT 'Value Id' ,
  `option_id` int UNSIGNED NOT NULL default '0' COMMENT 'Option Id' ,
  `value` varchar(255) NULL default NULL COMMENT 'Value' ,
  `filename` varchar(255) NULL default NULL COMMENT 'Filename' ,
  PRIMARY KEY (`value_id`),
  UNIQUE `IDX_EAV_ATTRIBUTE_OPTION_VALUE_OPTION_ID` (`option_id`),
  CONSTRAINT `FK_EAV_ATTR_OPT_VAL_OPT_ID_EAV_ATTR_OPT_OPT_ID` FOREIGN KEY (`option_id`) REFERENCES `eav_attribute_option` (`option_id`) ON DELETE CASCADE ON UPDATE CASCADE
) COMMENT='Eav Attribute Option Swatch' ENGINE=INNODB charset=utf8 COLLATE=utf8_general_ci

This is the part in the PHP file which must be checked

    ->addForeignKey(
        $installer->getFkName('eav/attribute_option_value', 'option_id', 'eav/attribute_option', 'option_id'),
        'option_id',
        $installer->getTable('eav/attribute_option'),
        'option_id',
        Varien_Db_Ddl_Table::ACTION_CASCADE,
        Varien_Db_Ddl_Table::ACTION_CASCADE
    )

I have retested the installation and I don't encounter any issues. Can you please check in the 'eav_attribute_option' table if you have any duplicate option_id and if the schema is not corrupted?

@empiricompany
Copy link
Contributor Author

empiricompany commented Feb 11, 2024

If I change the name of a color in the attribute options page (e.g. from Blue to Blue Print), in the /media/wysiwyg/swatches directory, there is not PNG file with the new name. The fallback should work but I didn't find any directory in /media/catalog/swatches.

I have tested it, and it works correctly for me. The swatch with the new name (even with spaces) is dynamically created in the media\catalog\swatches folder when you view the product page.

@empiricompany
Copy link
Contributor Author

empiricompany commented Feb 11, 2024

What is the priority when I set an image on the product edit page to be a swatch. Based on this custom code, it takes the global one defined in attribute page or the one in the product image tab?

The swatch created by the color picker is the last fallback, so all other methods take precedence.

Complete fallback:

  • check if configurable product as image with the same label of selected attribute + '-swatch', ex. Royal Blue-swatch
  • check in media/wysywyg/swatches if there is an image named attribute.png (all lower case space replaced with -, ex. royal-blue.png for Royal Blue)
  • the last attempt check if there is a color defined for the attribute and generate it on the fly in media/catalog/swatches cache (added by this PR)

@fballiano
Copy link
Contributor

I tested the "remove fallback color" functionality and it works great

@empiricompany
Copy link
Contributor Author

@fballiano thanks a lot for all your suggestions!
Regarding the deprecation of prototype code, can I safely merge it? Have you already tested it?

@fballiano
Copy link
Contributor

@empiricompany I will test my alternative code asap

@fballiano
Copy link
Contributor

  • I rewrote the javascript part removing prototypejs
  • removed safari hacks because as far as I could test, the hack is not needed anymore
  • retested everything

imho this is ready to be merged

fballiano
fballiano previously approved these changes Apr 20, 2024
@fballiano
Copy link
Contributor

@empiricompany can you give this one a test to confirm everything is working for you too?

@empiricompany
Copy link
Contributor Author

yep I've already tested!

@fballiano fballiano requested a review from Sekiphp April 22, 2024 09:42
@fballiano
Copy link
Contributor

I have to retest this properly because it doesn't seem to save/delete the images from media/catalog/swatches anymore

@fballiano fballiano dismissed Sekiphp’s stale review April 24, 2024 08:55

777 is used everywhere so this PR is compliant and it should be addressed globally

@fballiano
Copy link
Contributor

I've retested everything and it works great, when modifying an attribute the cache folder gets deleted (correctly) and it gets recreated by the frontend

@fballiano fballiano changed the title New feature: configurable swatches enhanced New feature: ConfigurableSwatches now allows for auto-generation of the swatch image file based on color selection Apr 25, 2024
@fballiano
Copy link
Contributor

after thorough tests and long development, no BC is introduced, no errors seem to be logged because of this, we also addressed the last comments by @Sekiphp so it seems only right to merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Adminhtml Relates to Mage_Adminhtml Component: ConfigurableSwatches Relates to Mage_ConfigurableSwatches Component: Eav Relates to Mage_Eav new feature Template : admin Relates to admin template translations Relates to app/locale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants