search

naver / egjs-flicking

🎠 ♻️ Everyday 30 million people experience. It's reliable, flexible and extendable carousel.
https://naver.github.io/egjs-flicking/
MIT License
2.78k stars 129 forks source link

Not working with remix run #791

Closed TeoAvignon closed 1 year ago

TeoAvignon commented 1 year ago

Description

Hello !

First of all, your library looks awesome, thanks for it ! I am using Remix Run (SSR) and I can not make your component work. Here is a codesandbox of the issue

Steps to check or reproduce

https://codesandbox.io/p/sandbox/frosty-platform-v5tbcm?file=%2Fapp%2Froutes%2Findex.tsx&selection=%5B%7B%22endColumn%22%3A11%2C%22endLineNumber%22%3A10%2C%22startColumn%22%3A39%2C%22startLineNumber%22%3A9%7D%5D

malangfox commented 1 year ago

Hello @TeoAvignon.

We are so glad that you like our component. We took a look at the demo you left, and there are two points that might seem like issue you reported. You can check and compare the fixed demo to the previous one.

  1. The horizontal length of the Panel is not set. I set a horizontal size style to each Panel that fits the firstPanelSize you were hoping for, and it now works fine.

  2. circular option was not working due to the above issue. circular option in Flicking doesn't work if the sum of the panel sizes is too small, you can check out this document.

I hope the modified demo has the behavior you want. If you have additional questions or unresolved issues, please feel free to leave them.

TeoAvignon commented 1 year ago

Hello @malangfox ,

Thank you very much for your quick answer !

Effectively, your demo seems to work pretty well ! Here is my feedback:

  1. For the point 2, It is well documented indeed.

  2. However, for the 1st point, I just followed the basic demo present in your documentation. It seems that there is no warning about a minimum size to reach to make the whole thing work. Maybe adding a comment (similar to the one you have for the point 2) would help other people not to encounter the same issue as mine.

malangfox commented 1 year ago

@TeoAvignon We've considered adding a warning in this situation, but we've noticed new warning may show for existing implementations that follow Flicking's internal policy of enabling or disabling circular based on the horizontal width.

So instead of adding a warning, we'd like to improve the demo in the documentation so that users don't encounter issues when they follow the demo.

We'd love to know which demo you referenced when implementing the circular option.

TeoAvignon commented 1 year ago

So nice to hear !

To be honest, I was just playing around with circular option to check if my issue was coming from a prop or not and I forgot to remove it when sending the demo to you.

When trying your lib, I first copied/pasted your first demo. I think it should be the place !

Btw, what I first loved about your lib is the demo you have on your home page ! It took me a bit of time to find it. I think it could a plus to have this example in your demo page where I expected it. It would also be a nice transition for the user to jump to the plugin page.