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-616] [Mini-Cart] Swatch images are stretched in mini-cart and has… #2457

Merged
merged 8 commits into from
Jun 8, 2020
Merged

[PWA-616] [Mini-Cart] Swatch images are stretched in mini-cart and has… #2457

merged 8 commits into from
Jun 8, 2020

Conversation

anthoula
Copy link
Contributor

@anthoula anthoula commented Jun 3, 2020 •
edited

Description

Ensure mini cart swatch images are not stretched and aligned to top of product info text

Related Issue

https://jira.corp.magento.com/browse/PWA-616

Acceptance

Verification Stakeholders

@jimbo
@dhaecker

Specification

Verification Steps

  1. Add a product to cart with a wide image
  2. Go to mini cart page and view thumbnail image is not stretched

Screenshots / Screen Captures (if appropriate)

Checklist

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

@anthoula
PWA-616: [Mini-Cart] Swatch images are stretched in mini-cart and has…
5d59d0f 
… more blank space in PDP.

- fix mini cart image thumbnail
@m2-community-project m2-community-project bot added this to Ready for Review in Pull Request Progress Jun 3, 2020
@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Jun 3, 2020 •
edited

Messages
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.
📖 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).
📖

Associated JIRA tickets: PWA-616.

Generated by 🚫 dangerJS against c8dcedb

@devops-pwa-codebuild
Copy link
Collaborator

devops-pwa-codebuild commented Jun 3, 2020 •
edited

Performance Test Results

The following fails have been reported by WebpageTest. These numbers indicates a possible performance issue with the PR which requires further manual testing to validate.

https://pr-2457.pwa-venia.com : LH Performance Expected 0.85 Actual 0.57, LH Best Practices Expected 1 Actual 0.92
https://pr-2457.pwa-venia.com/venia-tops.html : LH Performance Expected 0.75 Actual 0.36, LH Best Practices Expected 1 Actual 0.92
https://pr-2457.pwa-venia.com/valeria-two-layer-tank.html : LH Performance Expected 0.8 Actual 0.49, LH Accessibility Expected 0.9 Actual 0.89, LH Best Practices Expected 1 Actual 0.92

@davemacaulay davemacaulay added the version: Patch This changeset includes backwards compatible bug fixes. label Jun 4, 2020
@anthoula anthoula changed the title PWA-616: [Mini-Cart] Swatch images are stretched in mini-cart and has… [PWA-616] [Mini-Cart] Swatch images are stretched in mini-cart and has… Jun 4, 2020
davemacaulay
davemacaulay previously approved these changes Jun 4, 2020
View reviewed changes
jimbo
jimbo previously approved these changes Jun 4, 2020
View reviewed changes
Copy link
Contributor

@jimbo jimbo left a comment

Choose a reason for hiding this comment

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

Seems to solve the problem. 👍

It's unfortunate that the solution that's been agreed upon is to enforce an aspect ratio the images don't have, but we can revisit it.

@m2-community-project m2-community-project bot moved this from Ready for Review to Reviewer Approved in Pull Request Progress Jun 4, 2020
tjwiebell
tjwiebell previously approved these changes Jun 4, 2020
View reviewed changes
@anthoula
PWA-616: [Mini-Cart] Swatch images are stretched in mini-cart and has…
d6b678c 
… more blank space in PDP.

- fix mini cart image thumbnail with aspect ratio in cart, pdp, and search bar
@anthoula anthoula dismissed stale reviews from tjwiebell, jimbo, and davemacaulay via d6b678c June 5, 2020 14:55
anthoula
anthoula commented Jun 5, 2020
View reviewed changes
packages/venia-ui/lib/components/SearchBar/suggestedProduct.css
@@ -15,6 +15,8 @@
.thumbnail {
height: 75px;
max-height: 75px;
object-fit: contain;
object-position: center;
Copy link
Contributor Author

@anthoula anthoula Jun 5, 2020 •
edited

Choose a reason for hiding this comment

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

This is centered to align with vertically centered product text.

davemacaulay
davemacaulay previously approved these changes Jun 5, 2020
View reviewed changes
tjwiebell
tjwiebell previously approved these changes Jun 5, 2020
View reviewed changes
jimbo
jimbo previously approved these changes Jun 5, 2020
View reviewed changes
Copy link
Contributor

@jimbo jimbo left a comment

Choose a reason for hiding this comment

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

Good catch. 👍

@anthoula
PWA-616: [Mini-Cart] Swatch images are stretched in mini-cart and has…
ed22d57 
… more blank space in PDP.

- set thumbnail images to vertically center
@anthoula anthoula dismissed stale reviews from jimbo, tjwiebell, and davemacaulay via ed22d57 June 5, 2020 20:29
anthoula
anthoula commented Jun 5, 2020
View reviewed changes
packages/venia-ui/lib/components/CartPage/ProductListing/product.css
@@ -13,6 +13,9 @@
background-color: rgb(var(--venia-grey));
border: solid 1px rgb(var(--venia-border));
border-radius: 2px;
height: 100%;
object-fit: contain;
object-position: center;
Copy link
Contributor Author

@anthoula anthoula Jun 5, 2020 •
edited

Choose a reason for hiding this comment

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

Small change from UX: vertically center all images

Copy link
Contributor

Choose a reason for hiding this comment

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

That should work better. 👍

jimbo
jimbo approved these changes Jun 5, 2020
View reviewed changes
packages/venia-ui/lib/components/CartPage/ProductListing/product.css
@@ -13,6 +13,9 @@
background-color: rgb(var(--venia-grey));
border: solid 1px rgb(var(--venia-border));
border-radius: 2px;
height: 100%;
object-fit: contain;
object-position: center;
Copy link
Contributor

Choose a reason for hiding this comment

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

That should work better. 👍

@dhaecker
Copy link
Collaborator

dhaecker commented Jun 8, 2020

QA approved

@davemacaulay davemacaulay merged commit 21c9edf into magento:develop Jun 8, 2020
@m2-community-project m2-community-project bot moved this from Reviewer Approved to Done in Pull Request Progress Jun 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tjwiebell tjwiebell tjwiebell approved these changes

@davemacaulay davemacaulay davemacaulay approved these changes

@jimbo jimbo jimbo approved these changes

Assignees

@tjwiebell tjwiebell

@jimbo jimbo

Labels
pkg:venia-ui version: Patch This changeset includes backwards compatible bug fixes.
Projects
Pull Request Progress
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@anthoula @PWAStudioBot @devops-pwa-codebuild @dhaecker @tjwiebell @davemacaulay @jimbo