feat: offset the `initialItemCount` with the `initialTopMostItem`

[jord-nijhuis]

Currently, when using initialItemCount, it would always render the first N items (starting at 0). With this change, it will still render N items, but start at the offset provided by initialTopMostItem.

This PR contains the work performed by @TrueCarry for issue #361 plus some changes based on the feedback provided there.

There is one issue however: due to the filter for scrolledToInitialItem, the rendered items will always have a visibility: hidden (unless the offset is 0, thus recreating the old behaviour). This would mean that, while the correct section will be rendered, it will be hidden. https://github.com/petyosi/react-virtuoso/blob/16b418b8fae443b59ad379850b9b7d2be2c69e61/src/initialTopMostItemIndexSystem.ts#L22-L30

I have added a test case to test this, but I am not quite sure how to resolve this without breaking some of the other tests (this is my first time looking at the source code for Virtuoso). As such, some feedback would be greatly appreciated.

[petyosi]

Question - why do you use this? if for SEO purposes, do you know if the hidden display will affect you?

In general, I think that what you're doing brings no trouble, so I will merge it unless I discover some problem during testing. The initial item count needs improvement, the items should not be erased (causes a brief blink), but this is not directly related to what you've changed.

Also, can you check the conflicts?

[jord-nijhuis]

My main reason for this change is serving the requested content to the end-user as quickly as possible (i.e. showing the requested item pre-rendered, and loading all the other items afterwards). SEO is something I would like to focus on in the long run. I do believe that in the past display: none had no effect on SEO, but I think that nowadays it does at least have some effect on the rankings.

Regarding the conflicts: I may be blind, but GitHub says that "This branch has no conflicts with the base branch". Am I looking at the wrong thing?

Thank you for your swift response, and do tell if I need to change anything!

[petyosi]

Hm, I don't know if this will help much with your goal given https://github.com/petyosi/react-virtuoso/issues/763. Have you seen that? You should be affected by it.

Also, I don't know why I see this. Not the biggest deal though, can merge it anyway.

[jord-nijhuis]

Hm, I don't know if this will help much with your goal given #763. Have you seen that? You should be affected by it.

Also, I don't know why I see this. Not the biggest deal though, can merge it anyway.

Ah, I am using Next.js, so that is perhaps why I have not encountered the bug described in #763.

I have tried rebasing my branch onto master (thus the force push), has this resolved the conflict message for you?

[petyosi]

All good with the force push!

I think the problem should still be there, even with NextJS (several people reporting it in the comments). Probably happening too fast and you don't see it.

Anyway, what you've done brings no harm, and will be used when I eventually solve #763. Will review this week and update you accordingly.

[github-actions[bot]]

:tada: This PR is included in version 4.5.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: