React Flicking: issues related to needPanel event and decreasing number of panels

[gamtiq]

trafficstars

Description

I’ve encountered a couple of issues by using React Flicking.

1. Calling focus method for a panel may trigger needPanel event.
2. Decreasing number of panels trigger “Maximum update depth exceeded” React’s error.

What are the preferable ways to bypass those issues?

Steps to check or reproduce

You can reproduce both issues by using the following code sandbox: https://codesandbox.io/s/react-flicking-issues-mb8xr?file=/src/App.tsx Move by mouse to panel #4. Then you will see that new panels are added and moved (issue 1). At the end (after panel #18 is added) you will see React’s error (issue 2).

[WoodNeck]

Hello, @gamtiq! This looks like the componentDidUpdate is called endlessly, and it's quite hard to say what is preferable because I don't know what kind of UX exactly you're trying to create.

So, for the needPanel issue, the possible bypasses are:

• Check isTrusted property. It's only true when the user actually interacted with the carousel.
• Create bigger panels to prevent needPanel event to be called.
• Remove focus if doesn't necessary

For the second issue, don't call setState inside componentDidUpdate. I think it can be handled in needPanel event.

[gamtiq]

Thank you for the suggestions!

Regarding UX that I try recreate. Length of list of items/panels isn't known beforehand. On needPanel event placeholders are created. After that loading of new items is triggered. When new items are loaded placeholders are replaced and first loaded panel is focused. If there are no more new items placeholders should be deleted and lastIndex should be set.

I've checked isTrusted field of needPanel event. If the field is used to prevent modifying state, no more needPanel events are triggered. See the corresponding sandbox: https://codesandbox.io/s/react-flicking-issues-istrusted-080b9?file=/src/App.tsx It's impossible to cause adding of new panels after panel #6 is created because needPanel event isn't triggered anymore.

[WoodNeck]

Well, in your case I think the main issue is placeholder panels are too small so that if you call focus to the first panel added needPanel is triggered inevitably.

When focused to the first added panel (#5), needPanel is called and #7 and later should be added in this case.

So in this case, I think you can try to load more panels at once if possible. Like, 4 at once in your demo.

[gamtiq]

Unfortunately making placeholders bigger or requesting more panels doesn't fix needPanel issue because loaded panels can be much smaller and can be received in lesser quantity (for example 0 or 1) than was requested. As a result there are no more needPanel events after placeholders are changed by real panels. See related code sandbox: https://codesandbox.io/s/react-flicking-issues-big-placeholder-l7r23 Moreover there is a strange error this.flicking is null that is caused by needPanel event handler when check for eventData.isTrusted is omitted. Try to comment the corresponding if block, move by mouse to the left and wait until panels are loaded. Then drag a little the panel #5 or #6 to the right side of flicking viewport.

The only way I've managed to bypass needPanel issues is to use specific hanger and anchor that fix first placeholder and loaded panel near right side of flicking viewport. The corresponding sandbox: https://codesandbox.io/s/react-flicking-issues-bypass-auto-needpanel-xlpwo But such a case also has its own shortcomings and does not always applicable.

[WoodNeck]

I think Flicking should be improved to settle this issue. Maybe needPanel event should be triggered again, or "Add indicator method/getter for checking whether Flicking is at the boundary" in our roadmap should be done first.

Either way, I'll leave a comment here after making it :)

[stale[bot]]

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일간 활동이 없다면 자동으로 닫힐 예정입니다. 프로젝트 개선에 기여해주셔서 감사합니다.