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

Add Optional InfiniteList Support to AgendaList #2270

Merged
merged 13 commits into from
Jul 5, 2023

Conversation

yakirza17
Copy link
Contributor

@yakirza17 yakirza17 commented Jun 28, 2023

Introduces an experimental prop infiniteListProps to AgendaList. When defined, AgendaList switches from SectionList to InfiniteList. Be aware, as an experimental feature, it might not support all existing APIs.

Additionally, enhances InfiniteList to accept a layoutProvider for detailed item size control.

@yakirza17 yakirza17 changed the title [WIP] Start working on migration of the agenda list Migrate AgendaList to InfiniteList Jul 2, 2023
@yakirza17 yakirza17 changed the title Migrate AgendaList to InfiniteList Add Optional InfiniteList Support to AgendaList Jul 2, 2023
@yakirza17 yakirza17 marked this pull request as ready for review July 2, 2023 07:13
(index) => data[index]?.isTitle ? 'title': 'page',
(type, dim) => {
dim.width = constants.screenWidth;
dim.height = type === 'title' ? 60 : 120;
Copy link
Contributor Author

@yakirza17 yakirza17 Jul 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should see what the right size to pass, RecyclerListView doesn't support calculating item height

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ethanshar @Inbal-Tish please give your thoughts

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current example, it should be 80 instead of 120, but anyways you'll have to let the user pass the item's height to set it there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created infiniteListProps in the oldAgendaList, with an option to define these values (and load the new component in case it pass - even an empty object)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Inbal-Tish It's ok to put values like that? will it work in any view? tablet view? landscape?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will stay the same unless you change it when the orientation change. Regarding tablets, it depends on the design of course... Once you give the user the option to set these values it can be dynamic. 'pageWidth' and 'pageHeight' covers that

@Inbal-Tish
Copy link
Collaborator

@yakirza17 @chenei Overall looks good. Left a few comments

@yakirza17 yakirza17 requested a review from Inbal-Tish July 3, 2023 07:09
@Inbal-Tish
Copy link
Collaborator

@yakirza17 Can you add a code snippet for the AgendaList so I can see on what example you're working on?

@yakirza17
Copy link
Contributor Author

@Inbal-Tish, add infiniteListProps={{}} in the AgendaList on example/src/screens/expandableCalendarScreen.tsx
I also test it with our real-use repo (by changing the node modules code)

@Inbal-Tish Inbal-Tish merged commit 72f9b5d into master Jul 5, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants