asmyshlyaev177 / react-horizontal-scrolling-menu

Horizontal scrolling menu component for React.
https://www.npmjs.com/package/react-horizontal-scrolling-menu
MIT License
758 stars 98 forks source link

onLastItemVisible Function is not getting called when last item is visible #123

Closed bhargavavarma closed 3 years ago

bhargavavarma commented 3 years ago

Description

onLastItemVisible Function is not getting called when last item is visible on screen

Steps to reproduce

click on this https://codesandbox.io/s/keen-mestorf-swy2m?file=/src/App.js to reproduce in code sandbox.

Expected

When last item is visible in the screen, need to call onLastItemVisible Function

Actual

while scrolling items to right using right arrow onLastItemVisible Function is not getting called, for example last item('25') when it is visible onLastItemVisible is not getting called, but again when you click again on right arrow, it was getting called.

Additional Information

versions

Ganeshkaredla commented 3 years ago

@asmyshlyaev177

Here we had a use case instead of getting a callback for lastItemVisible we need to call that before 3 or 4 items from the last

so as per the code in scrollMenu file

//original line in `checkFirstLastItemVisibility` method

 lastItemVisible = visibleItems.includes(menuItems.slice(-1)[0]);

 //modified line 

 lastItemVisible = visibleItems.includes(menuItems.slice( -4 )[0]);

Here let's say 4 is our customItem where we need to trigger call back, is it ok to change here? Or need to do changes in any other places?

asmyshlyaev177 commented 3 years ago

@asmyshlyaev177

Here we had a use case instead of getting a callback for lastItemVisible we need to call that before 3 or 4 items from the last

so as per the code in scrollMenu file

//original line in `checkFirstLastItemVisibility` method

 lastItemVisible = visibleItems.includes(menuItems.slice(-1)[0]);

 //modified line 

 lastItemVisible = visibleItems.includes(menuItems.slice( -4 )[0]);

Here let's say 4 is our customItem where we need to trigger call back, is it ok to change here? Or need to do changes in any other places?

menuItems.slice(-1)[0] it's mean last item in array, e.g. [1,2,3,4,5,6,7].slice(-1)[0] // 7 With menuItems.slice( -4 )[0] you will get [1,2,3,4,5,6,7].slice(-1)[0] // 4

Need to fix this bug, but i have a lot of work and this project got too complex. Need cut out some features and rewrite it.

Ganeshkaredla commented 3 years ago

@asmyshlyaev177

Here we had added a prop customLastItemVisibleNumber : number if we want callBack for lastItemVisible before it reaches last item we had used this by forking your repo

you can see how it works as per below snippet

//original code 
lastItemVisible = visibleItems.includes(menuItems.slice(-1)[0]);

//Change we had done here by expecting `customLastItemVisibleNumber` as a prop for our use case

 if (customLastItemVisibleNumber) {
        lastItemVisible = visibleItems.includes(
          menuItems.slice(-customLastItemVisibleNumber)[0],
        );
      } else lastItemVisible = visibleItems.includes(menuItems.slice(-1)[0]);

     /*
     Example: [1,2,3,4,5,6,7,8,9,10] 
     - if we send `customLastItemVisibleNumber = { 4 }` as a prop
     - we will get callBack for `lastItemVisible` when array item `6` visible

     */ 

Here my doubt is this only change is enough to get my expected output or we need to change the same in different places in that scrollMenu file ?

asmyshlyaev177 commented 3 years ago

@Ganeshkaredla You can run use your fork with npm link and see it in real time with your app.

Ganeshkaredla commented 3 years ago

@asmyshlyaev177

Can you solve this issue?

He provided a code sandbox as well to reproduce the issue, I have the same.

When the last item gets visible we are not getting a callback.

Can you fix this issue ? or else just give us some leads on how to solve this issue so that we can do changes and raise a PR

We had tried by forking your repo but we got some issues by adding a prop as we discussed in above discussion

But we found scrollBy and getVisibleItems() and transistion are interlinked so it dosen't solve our use case. and got some other scrolling related issues that are not expected.

What we need is when the last item gets visible we need a callback immediately.

If you give us any leads it will be very helpful for us to fix this issue.

Thanks in Advance.

asmyshlyaev177 commented 3 years ago

Can you try version 2?