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

(fix): PDP placeholde image display #3812

Merged
merged 3 commits into from Feb 21, 2018

Conversation

Projects
None yet
2 participants
@kieckhafer
Copy link
Member

commented Feb 21, 2018

Impact: major
Type: bugfix

Issue

Placeholder images were not showing up on the PDP. This issue was cause by a line of code that was pre-emptively adding a &store=size to the end of a URL in an || check. Because the &store=size was always added, the first part of the || check would never be null / undefinded, and would never fall back to the second part of the || check, which was where the placeholder image was loaded.

Solution

Re-write the checks to only add the size after an initial source was checked, not during the initial source check.

Breaking changes

None

Testing

  1. Visit the Product Grid and the PDP as both an anonymous and Admin user
  2. See that the placeholder image is shown if no product image is available.
  3. See that the correct image is shown, with the correct &store=size being loaded

spencern and others added some commits Feb 21, 2018

@spencern
Copy link
Member

left a comment

Good changes. More readable, less repetition, and good comments.

@spencern spencern merged commit 2af7e86 into release-1.8.0 Feb 21, 2018

4 checks passed

Codacy/PR Quality Review Good work! A positive pull request.
Details
WIP ready for review
Details
ci/circleci Your tests passed on CircleCI!
Details
security/snyk No dependency changes
Details

@spencern spencern deleted the fix-pdp-placeholder branch Feb 21, 2018

@spencern spencern referenced this pull request Feb 22, 2018

Merged

Release 1.8.0 #3645

@spencern spencern referenced this pull request Mar 9, 2018

Merged

Release 1.9.0 #3941

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.