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.

Sign up for GitHub

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

Jump to bottom

[PWA-1101] Add support for Configurable Product Image Setting #2909

Closed
wants to merge 22 commits into from
Closed

[PWA-1101] Add support for Configurable Product Image Setting #2909

wants to merge 22 commits into from

Conversation

eug123
Copy link
Contributor

@eug123 eug123 commented Dec 15, 2020 •
edited
Loading

Description

As a admin user, I would like the ability to change settings in the admin panel to control the configurable product image, and view it in my Venia storefront.

Admin Config Settings:

Stores > Configuration > Sales > Checkout > Shopping Cart > Configurable Product Image

Related Issue

Closes https://jira.corp.magento.com/browse/PWA-1101

Acceptance

Verification Stakeholders

@dpatil-magento
@sirugh

Specification

Verification Steps

  1. Change admin configuration setting for Stores > Configuration > Sales > Checkout > Shopping Cart > Configurable Product Image
  2. Save configuration and navigates to the Venia storefront
  3. Add a Configurable Product to Cart and verify that product thumbnail corresponds admin configuration setting
  4. Verify product thumbnail on Shopping Cart page
  5. Verify product thumbnail on Checkout review page
  6. Verify product thumbnail on Order Confirmation page
  7. Verify product thumbnail on Order History page

Screenshots / Screen Captures (if appropriate)

Checklist

  • I have added tests to cover my changes, if necessary.
  • I have added translations for new strings, if necessary.
  • I have updated the documentation accordingly, if necessary.

@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Dec 15, 2020 •
edited
Loading

Fails
🚫 A version label is required. A maintainer must add one.
Messages
📖

Associated JIRA tickets: PWA-1101.
📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Generated by 🚫 dangerJS against 6da2ab0

sirugh
sirugh reviewed Dec 18, 2020
View reviewed changes
packages/peregrine/lib/Apollo/policies/temporary.js Outdated
const temporaryTypePolicies = {
StoreConfig: {
fields: {
// This field is available in Magento 2.4.2 we need to support older versions, so env var is used here
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we support n now, instead of n-1, do we need to mock this? By the time this code is released in PWA Studio, 2.4.2 should be released so I think this is fine to remove.

sirugh
sirugh reviewed Dec 18, 2020
View reviewed changes
packages/peregrine/lib/talons/MiniCart/miniCart.gql.js Outdated
// We disable linting for local fields because there is no way to add them to
// the fetched schema.
// https://github.com/apollographql/eslint-plugin-graphql/issues/99
/* eslint-disable graphql/template-strings */
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment here re: re-enabling this lint rule, although you may be able to just remove @client.

@eug123 eug123 requested review from revanth0212 and sirugh January 7, 2021 21:46
revanth0212
revanth0212 previously approved these changes Jan 7, 2021
View reviewed changes
@revanth0212
Copy link
Contributor

Wonderful work @eug123, Approved 👍 .

@dpatil-magento
Copy link
Contributor

@eug123 PR looks good. Just an observation where product image thumbnail does not update in Product Edit dialogue on cart page.
@schensley @tkacheva Please let us know if this needs to be fixed.

image

@eug123
Copy link
Contributor Author

eug123 commented Jan 21, 2021

@dpatil-magento, I've discussed this observation with @tkacheva and she is going to open separate ticket addressing this issue.
Thank you

@eug123 eug123 closed this Jan 21, 2021
@eug123 eug123 reopened this Jan 21, 2021
@eug123 eug123 closed this Jan 21, 2021
@eug123 eug123 mentioned this pull request Jan 21, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@revanth0212 revanth0212 revanth0212 left review comments

@anthoula anthoula anthoula left review comments

@sirugh sirugh Awaiting requested review from sirugh

Assignees

@anthoula anthoula

@sirugh sirugh

@revanth0212 revanth0212

Labels
pkg:peregrine pkg:pwa-buildpack pkg:venia-ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@eug123 @PWAStudioBot @revanth0212 @dpatil-magento @anthoula @sirugh