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

Scroller: dynamic size for itemSize #2290 #16648

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

Max0nyshchenko
Copy link
Contributor

@Max0nyshchenko Max0nyshchenko commented Oct 27, 2024

This is the example PR to the discussion in https://github.com/orgs/primefaces/discussions/2290

These changes need to be further optimized, which I can do, if you are going to accepts the changes, but they already working and you can see and check them out in the docs examples that has been added (flexible-items-height-doc and flexible-grid-doc). Or you can finish this PR by yourself, feel free to use my changes in any way you want. The main thing is to have the support for flexible itemSize for different items.

I need these changes so that I can implement expandable rows in p-table with virtualization later, which is required for one of my projects.
So I'm wondering if you consider these changes significant or of the kind that would be welcomed.
If you give me the green light I'll work on the final version with improved performance.

Copy link

vercel bot commented Oct 27, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Updated (UTC)
primeng ⬜️ Ignored (Inspect) Visit Preview Oct 27, 2024 3:00pm
primeng-v18 ⬜️ Ignored (Inspect) Visit Preview Oct 27, 2024 3:00pm

@DanniKach
Copy link

Oh, thanks a lot @Max0nyshchenko.
We really researched a lot of topics before understood that such functionality is unavailable in current version of library, so it is impossible to implement expanded items with infinite scroll. This is required functionality for our application, so hope it will be approved and merged as soon as possible. We are waiting for it.

@Max0nyshchenko
Copy link
Contributor Author

@cetincakiroglu what do you think? Could you take a look?

@mertsincan
Copy link
Member

Hi @Max0nyshchenko,
Thanks a lot for your contribution! This seems like a draft. Are you still working on it?

@mertsincan mertsincan added Resolution: Needs Revision The pull request can't be merged. Conflicts need to be corrected or documentation and code updated. Resolution: Help Wanted Issue or pull request requires extra help and feedback labels Nov 18, 2024
@Max0nyshchenko
Copy link
Contributor Author

Hi @mertsincan ,
You're welcome. It is a draft cause I need to know if this kind of changes will be accepted so I can see if it makes sense for me to finish it.
The only thing left to finish is performance optimization. Currently, the recalculateSize function is called on every scroll event, it is computationally-intensive function and can take some time to execute in large data sets.
I'm thinking of introducing caching of recalculateSize results and scroll event debouncing (or maybe using requestAnimationFrame instead of scroll event) for optimization purposes. But maybe you have better suggestions?

So 2 questions:
1 - Will these changes be welcomed (maybe you don't approve of an interface or don't want to have such feature at all)?
2 - Any ideas for optimization?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Help Wanted Issue or pull request requires extra help and feedback Resolution: Needs Revision The pull request can't be merged. Conflicts need to be corrected or documentation and code updated.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants