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

refactor: use native swiper instead of ngx-swiper-wrapper #552

Merged
merged 1 commit into from
Feb 24, 2021

Conversation

dhhyi
Copy link
Collaborator

@dhhyi dhhyi commented Feb 16, 2021

PR Type

[x] Refactoring (no functional changes, no API changes)

What Is the Current Behavior?

ngx-swiper-wrapper is used in the PWA but it will not be supported any longer (see migration note for link).

What Is the New Behavior?

  • native swiper usage
  • swiper only imported in page modules which reduces main bundle size.

Does this PR Introduce a Breaking Change?

[ ] Yes
[x] No

Other Information

@dhhyi dhhyi added the enhancement Enhancement to an existing feature label Feb 16, 2021
@dhhyi dhhyi added this to the 0.28 milestone Feb 16, 2021
@dhhyi dhhyi self-assigned this Feb 16, 2021
@shauke shauke self-requested a review February 17, 2021 10:25
@dhhyi dhhyi requested a review from suschneider February 17, 2021 14:17
Copy link
Collaborator

@shauke shauke left a comment

Choose a reason for hiding this comment

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

Before
image

After
image

The controls for click slide switching are missing. Swiping with mouse drag & drop works though (meaning the Swiper is initialized).

The same is with the product links (see "You may also like")
Before
https://intershoppwa.azurewebsites.net/Smart-Home/Google-Home-sku201807171-catHome-Entertainment.SmartHome
After
http://pwa-gh-review-552-universal-b2c.azurewebsites.net/Smart-Home/Google-Home-sku201807171-catHome-Entertainment.SmartHome

src/app/core/appearance.module.ts Show resolved Hide resolved
@dhhyi dhhyi assigned shauke and suschneider and unassigned dhhyi Feb 18, 2021
@dhhyi dhhyi force-pushed the refactor/use-native-swiper branch 2 times, most recently from a2b507f to c37f9d6 Compare February 19, 2021 13:34
@dhhyi dhhyi self-assigned this Feb 19, 2021
@dhhyi dhhyi requested a review from shauke February 19, 2021 15:33
@shauke shauke unassigned dhhyi and shauke Feb 22, 2021
@suschneider suschneider force-pushed the refactor/use-native-swiper branch 2 times, most recently from 38e81b8 to b219546 Compare February 24, 2021 13:19
suschneider
suschneider previously approved these changes Feb 24, 2021
@suschneider suschneider merged commit f596d0b into develop Feb 24, 2021
@suschneider suschneider deleted the refactor/use-native-swiper branch February 24, 2021 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants