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

Bugfix/product image loader #2925

Merged
merged 30 commits into from
May 28, 2019
Merged

Bugfix/product image loader #2925

merged 30 commits into from
May 28, 2019

Conversation

pspaczek
Copy link
Collaborator

Short description and why it's useful

I extract to the new component images loading strategy implemented in Product Gallery Carousel and use it on Producta Gellery Carousel and Product Gallery Zoom Carousel components.

Which environment this relates to

Check your case. In case of any doubts please read about Release Cycle

  • Test version (https://test.storefrontcloud.io) - this is a new feature or improvement for Vue Storefront. I've created branch from develop branch and want to merge it back to develop
  • RC version (https://next.storefrontcloud.io) - this is a stabilisation fix for Release Candidate of Vue Storefront. I've created branch from release branch and want to merge it back to release
  • Stable version (https://demo.storefrontcloud.io) - this is an important fix for current stable version. I've created branch from hotfix or master branch and want to merge it back to hotfix

Upgrade Notes and Changelog

  • No upgrade steps required (100% backward compatibility and no breaking changes)
  • I've updated the Upgrade notes and Changelog on how to port existing VS sites with this new feature

IMPORTANT NOTICE - Remember to update CHANGELOG.md with description of your change

Contribution and currently important rules acceptance

@pkarw pkarw requested review from patzick and DaanKouters May 20, 2019 17:56
Copy link
Collaborator

@DaanKouters DaanKouters left a comment

Choose a reason for hiding this comment

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

Nice, good improvement

pkarw
pkarw previously requested changes May 21, 2019
Copy link
Collaborator

@pkarw pkarw left a comment

Choose a reason for hiding this comment

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

Please do check the usage of !! operator

@pkarw pkarw added this to the 1.10.0-rc.1 milestone May 22, 2019
@pkarw pkarw added the QA - Ready for tests This is notification for testers, that improvement is ready to be tested and verified. label May 22, 2019
Copy link
Collaborator

@patzick patzick left a comment

Choose a reason for hiding this comment

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

Thanks, mostly things we've discussed.

After improvements QA team should checkout this branch and test images on category, homepage and product pages locally before we will merge that in (notice to @pkarw :) )

@pspaczek
Copy link
Collaborator Author

@patzick

  • I removed hidden props from component and use v-show on parent component
  • Used $listeners to stop using '.native' event
  • I set default for all props and removed required
  • Cheged object name to 'image'

I used https://css-tricks.com/aspect-ratio-boxes/ to maintaining image size. I used to this mathematic formula {height% / ( width / 100 )}

@patzick patzick mentioned this pull request May 27, 2019
5 tasks
@patzick patzick requested a review from pkarw May 28, 2019 15:03
@patzick patzick dismissed pkarw’s stale review May 28, 2019 15:04

outdated review

@patzick patzick merged commit d347fc6 into vuestorefront:develop May 28, 2019
@patzick
Copy link
Collaborator

patzick commented May 28, 2019

Merged and ready for tests

@pspaczek pspaczek deleted the bugfix/ProductImage branch May 30, 2019 10:56
@alinadivante
Copy link
Collaborator

@przspa You forgot about pictures in checkout :)
image

@patzick
Copy link
Collaborator

patzick commented Jun 3, 2019

Related to #2863 - @przspa please take a look :)

@pspaczek
Copy link
Collaborator Author

pspaczek commented Jun 6, 2019

@przspa You forgot about pictures in checkout :)
image #3013 fixed my mistake

@alinadivante alinadivante added QA approved on branch Testers will add this label after positive check on specific branch. and removed QA - Ready for tests This is notification for testers, that improvement is ready to be tested and verified. labels Jun 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA approved on branch Testers will add this label after positive check on specific branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants