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

Refactored the Sliders to use the new react-id-swiper version #571

Merged
merged 19 commits into from
Mar 28, 2019

Conversation

devbucket
Copy link
Contributor

Description

Updated the react-id-swiper to the newest version and refactored all sliders to be compatible with the new API. This caused react and react-dom to be updated to the newest versions to provide the new hooks API which is needed for react-id-swiper.

Type of change

Please add an "x" into the option that is relevant:

  • [ x ] Bug Fix 🐛 (non-breaking change which fixes an issue)
  • [ ] New Feature 🚀 (non-breaking change which adds functionality)
  • [ ] Breaking Change 💥 (fix or feature that would cause existing functionality to not work as expected)
  • [ x ] Polish 💅 (Just some cleanups)
  • [ ] Docs 📝 (Changes in the documentations)
  • [ ] Internal 🏠 Only relates to internal processes.

How to test it

Please describe here any specialty that the tester should be aware of.

@devbucket devbucket added the bug Something isn't working label Mar 15, 2019
@devbucket devbucket self-assigned this Mar 15, 2019
@coveralls
Copy link

coveralls commented Mar 15, 2019

Pull Request Test Coverage Report for Build 2186

  • 11 of 11 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.04%) to 49.814%

Totals Coverage Status
Change from base Build 2184: -0.04%
Covered Lines: 5505
Relevant Lines: 10452

💛 - Coveralls

fkloes
fkloes previously requested changes Mar 21, 2019
libraries/common/components/Swiper/index.jsx Outdated Show resolved Hide resolved
libraries/common/components/Swiper/index.jsx Outdated Show resolved Hide resolved
libraries/common/components/Swiper/index.jsx Outdated Show resolved Hide resolved
themes/theme-gmd/widgets/ImageSlider/spec.jsx Outdated Show resolved Hide resolved
@SG-Noxoreos
Copy link
Contributor

Things that I noticed while testing:

  • PDP Image-gallery panning after zoom seems not to work properly. Instead it switches to the next/prev image when trying to pan horizontally. Vertically nothing happens.
  • PDP Images are swipeable, even if only one image is available (no swiping was possible before)
  • When zooming in and out a lot in the PDP image-gallery, the zoom stops working until the gallery is reopened on that PDP view

@SG-Noxoreos SG-Noxoreos changed the base branch from v6.X to v6.4.0 March 27, 2019 14:35
@SG-Noxoreos SG-Noxoreos dismissed fkloes’s stale review March 28, 2019 11:15

Won't re-review. Taken over by @SG-Noxoreos

@SG-Noxoreos SG-Noxoreos self-requested a review March 28, 2019 11:20
Copy link
Contributor

@SG-Noxoreos SG-Noxoreos left a comment

Choose a reason for hiding this comment

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

All tested and no more issues found.

@SG-Noxoreos SG-Noxoreos merged commit 7f205c2 into v6.4.0 Mar 28, 2019
@SG-Noxoreos SG-Noxoreos deleted the PWA-1198-repalce-slider-implementations branch March 28, 2019 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants