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

Fix on 3.9.1 made the whole page freeze (relative issue: #616) #624

Closed
kazvaggos opened this issue Feb 11, 2022 · 9 comments
Closed

Fix on 3.9.1 made the whole page freeze (relative issue: #616) #624

kazvaggos opened this issue Feb 11, 2022 · 9 comments
Assignees
Labels
🔖3.x v3.x~ related issue 🐛Bug Bugs 📦Vue2 @egjs/vue-flicking related issue stale

Comments

@kazvaggos
Copy link

kazvaggos commented Feb 11, 2022

"@egjs/vue-flicking": "3.8.1"

Description

Hello team. Thanks a lot for the fix!
However, this fix introduced a much bigger bug. When the scenario of the #616 happen, then the whole page freezes due to some endless loop happening next.
After debugging it, we noticed that panel0OnFirstScreen equals to NaN due to 0/0 division because all the variables on the right hand side equal to 0. This makes the cloneCount to equal to NaN, creating a domino effect afterwards with endless loops.
image

A suggested fix that seemed to resolve the domino effect is to add a fallback on cloneCount, but we are not sure if this is the correct one.
image

We were forced to downgrade to "@egjs/vue-flicking": "3.8.0" with egjs-flicking: 3.9.0, because the error that was thrown in the previous version saves as from the whole page to freeze.

@kazvaggos kazvaggos changed the title Fix on 3.9.1 made the whole page freeze (relative issue: ) Fix on 3.9.1 made the whole page freeze (relative issue: #616) Feb 11, 2022
@WoodNeck
Copy link
Member

Hello @kazvaggos!
It's my fault that I was a bit haste to release #619. I should add some more tests or so.
I'll fix this right away :)

@WoodNeck WoodNeck self-assigned this Feb 14, 2022
@WoodNeck WoodNeck added 🔖3.x v3.x~ related issue 🐛Bug Bugs 📦Vue2 @egjs/vue-flicking related issue labels Feb 14, 2022
@WoodNeck
Copy link
Member

Can you try this version? 3.8.2-beta.0
You can install it with

npm i @egjs/vue-flicking@beta-3

@kazvaggos
Copy link
Author

kazvaggos commented Feb 14, 2022

Hello @WoodNeck ! Thank you for the immediate response.
After upgrading to @egjs/vue-flicking@beta-3 the page doesn't freeze anymore. However, when the scenario in #616 happens, the carousel becomes unresponsive. It only shows the last photo and swiping between photos does not work at all. It is like the carousel has not initialized at all.

@WoodNeck
Copy link
Member

@kazvaggos
I think I should take a look at actually what's happening in this case.
Can you provide me with a way to reproduce that issue?
Any of the following methods are appreciated:

  • An explanation of how to reproduce the issue
  • A simple demo with a Codepen
  • URL to your website page, if it's public

@kazvaggos
Copy link
Author

Hello @WoodNeck .
I created this pen that simulates the scenario #619
By clicking the button, the flicking component mounts, but sometimes doesn't mount correctly and it is inactive showing only the last photo.
Most probably, it will happen using safari browser. But with chrome, can happen too.

@WoodNeck
Copy link
Member

@kazvaggos
That super helped! I found the cause and here's the thing:

When Flicking's initialized before the images are loaded, the panels inside Flicking have 0 sizes and end up stuck in one position.
And that makes Flicking show only the last photo.

Flicking offers resizeOnContentsReady to overcome these kinds of issues, but looks like it isn't compatible with frameworks like Vue in the old v3. Panel sizes are fixed correctly, but as cloned panels are not generated so other error still occurs.
Calling $forceUpdate can fix that, but v3 doesn't offer any correct timing for it, and we're not adding any more APIs in v3 as we're focusing on v4.

Check this demo, which I've slightly modified from your pen. Init Flicking and clicking force update after it will make the carousel work properly.

I'll try to fix this, but I suggest using v4 as all kinds of these issues are already fixed in it.

@WoodNeck
Copy link
Member

I've just released a new version (flicking 3.9.2, vue-flicking 3.8.2) that fixes this issue.
Please try that :)

@kazvaggos
Copy link
Author

Looks great! Thanks a lot @WoodNeck 💪

@stale
Copy link

stale bot commented Apr 16, 2022

This issue/PR has been automatically marked as stale because it has not had any update (including commits, comments, labels, milestones, etc) for 30 days. It will be closed automatically if no further update occurs in 7 day. Thank you for your contributions!

한글 이 이슈/PR은 commits, comments, labels, milestones 등 30일간 활동이 없어 자동으로 stale 상태로 전환되었습니다. 이후 7일간 활동이 없다면 자동으로 닫힐 예정입니다. 프로젝트 개선에 기여해주셔서 감사합니다.

@stale stale bot added the stale label Apr 16, 2022
@stale stale bot closed this as completed Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔖3.x v3.x~ related issue 🐛Bug Bugs 📦Vue2 @egjs/vue-flicking related issue stale
Projects
None yet
Development

No branches or pull requests

2 participants