Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 7, 2025

Fix Swatches Display on Product Detail Page

  • Understand the issue: isEnabled() incorrectly requires listing attribute for all contexts
  • Add new method isEnabledForProductDetail() that only checks general enabled flag and detail attributes
  • Update shouldRender() in Swatches block to use new method
  • Update getSwatchesProductJs() to use new method
  • Update Observer methods appropriately for their context
  • Run code review - passed with no comments
  • Run security check - passed (no code changes for CodeQL languages)
  • Add caching to isEnabledForProductDetail() method to avoid repeated getStoreConfigFlag() calls
  • Add docblock comments to cached properties for clarity
  • Fix PHPDoc type order to null|bool for PHP CS Fixer compliance
  • Improve property docblock descriptions to clarify what each property caches
  • Fix fatal error in Mediafallback helper when listing attribute is not configured
  • Fix code formatting
  • Use instanceof for robust type checking
  • Final code review - passed

Summary

Successfully fixed the bug where swatches would not display on product detail pages unless a listing attribute was configured. The fix introduces isEnabledForProductDetail() with caching to separate product detail and listing concerns, allowing swatches to work independently on each page type as originally intended by the configuration structure. Also fixed a critical bug that caused fatal errors when the listing attribute was not configured.

Original prompt

This section details on the original issue you should resolve

<issue_title>[BUG] Swatches Not Displaying Correctly on Product Page</issue_title>
<issue_description>### Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

I have encountered a bug with the swatches for configurable products in the latest version, which I believe needs attention. The issue occurs when trying to display the swatches (such as color or size) on the product page.

When "Size" is not selected in the "Product Attribute to Use for Swatches in Product Listing" field, the swatches do not appear on the product page. Instead, the attribute remains as a regular dropdown.

Expected Behavior

Swatches should appear on the product page regardless of the setting for "Product Attribute to Use for Swatches in Product Listing", since displaying swatches in the product detail page is not related to how products are listed in the category or the product listing configuration.

I consider this is a bug, as the swatches should be independent of the listing configuration and should display properly on the product detail page regardless of the "Product Attribute to Use for Swatches in Product Listing" setting.

Steps To Reproduce

  1. Select "Size" as an attribute in the "Product Attributes to Show as Swatches in Product Detail" section.

  2. Ensure that "Size" is not selected for "Product Attribute to Use for Swatches in Product Listing".

  3. View the product page.

Environment

- OpenMage: latest from the cloned repo
- PHP: 8.4

Anything else?

Visual Comparison

Without Size selected in Product Listing

Image

With Size selected in Product Listing

Image

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@github-actions github-actions bot added the Component: ConfigurableSwatches Relates to Mage_ConfigurableSwatches label Dec 7, 2025
Copilot AI changed the title [WIP] Fix swatches not displaying correctly on product page Fix: Decouple product detail swatches from listing attribute requirement Dec 7, 2025
Copilot AI requested a review from addison74 December 7, 2025 20:43
@github-actions
Copy link
Contributor

github-actions bot commented Dec 7, 2025

Test Results

966 tests  ±0   958 ✅ ±0   17s ⏱️ -1s
213 suites ±0     8 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 0b9dc20. ± Comparison against base commit b74526a.

♻️ This comment has been updated with latest results.

@addison74 addison74 marked this pull request as ready for review December 7, 2025 20:57
Copilot AI review requested due to automatic review settings December 7, 2025 20:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug where product detail page swatches failed to render when the "Product Attribute to Use for Swatches in Product Listing" configuration was unset. The fix introduces a new isEnabledForProductDetail() method that only checks the global enabled flag, decoupling product detail swatches from the listing attribute requirement. The architecture now correctly treats listing and detail page swatches as independent features, each with their own appropriate enable checks.

Key Changes

  • Added isEnabledForProductDetail() method to check only the global enabled flag without requiring listing attribute configuration
  • Updated product detail contexts (Swatches block and JavaScript) to use the new context-specific method
  • Modified shared components (observers, image filtering, media blocks) to use OR logic, supporting both listing and detail contexts independently

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
app/code/core/Mage/ConfigurableSwatches/Helper/Data.php Added isEnabledForProductDetail() method and updated documentation; modified getSwatchesProductJs() to use the new method for detail page context
app/code/core/Mage/ConfigurableSwatches/Block/Catalog/Product/View/Type/Configurable/Swatches.php Changed shouldRender() to use isEnabledForProductDetail() for proper detail page swatch rendering
app/code/core/Mage/ConfigurableSwatches/Model/Observer.php Updated productLoadAfter() and loadChildProductImagesOnMediaLoad() with OR logic to support both listing and detail contexts
app/code/core/Mage/ConfigurableSwatches/Helper/Productimg.php Modified filterImageInGallery() with OR logic to work in both contexts
app/code/core/Mage/ConfigurableSwatches/Block/Catalog/Media/Js/Abstract.php Updated _toHtml() with OR logic to render for both listing and detail pages

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Copilot AI requested a review from addison74 December 22, 2025 17:51
@sreichel sreichel removed their request for review December 22, 2025 18:15
addison74
addison74 approved these changes Dec 22, 2025
@addison74
Copy link
Contributor

addison74 commented Dec 22, 2025

LGTM

Copilot AI and others added 3 commits December 22, 2025 21:32
Add null checks before calling methods on $listSwatchAttr to prevent
fatal errors when no product listing attribute is configured. This was
a pre-existing bug that was exposed when product detail swatches were
decoupled from listing requirements.

Co-authored-by: addison74 <[email protected]>
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: ConfigurableSwatches Relates to Mage_ConfigurableSwatches

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Swatches Not Displaying Correctly on Product Page

3 participants