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

Feature/322 support infinite scrolling #342

Merged
merged 35 commits into from
Mar 25, 2024

Conversation

g-saracca
Copy link
Contributor

@g-saracca g-saracca commented Mar 13, 2024

What this PR does / why we need it:

Support inifinite scrolling in the collections page.

Which issue(s) this PR closes:

Special notes for your reviewer:

At the moment I replaced the original pagination with buttons in the collections section by this infinite scroll, the final goal would be to add a toggle button to be able to switch between common pagination and infinite scroll.
All the stories are done, component test and I added cases to the e2e test, in this last mentioned I made a skip of the tests for the usual pagination since at the moment it is not being rendered, when we add the toggle, we will have to apply them again.

We used the react-infinite-scroll-hook library which is super lightweight, (see https://bundlephobia.com/package/[email protected]).
I also added the classnames library to be able to assign dynamic css classes, this is going to be super useful for many cases in my opinion.

Suggestions on how to test this:

Install the necessary dependencies using npm install.
Start Storybook with npm run storybook.
Visit the Storybook and check the DatasetsListWithInfiniteScroll Stories inside sections.
There are many scenarios for the infinite scrolling feature.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Infinite scrolling enabled

Is there a release notes update needed for this change?:

Additional documentation:

Storybook
Storybook with all states

@g-saracca g-saracca added Size: 10 A percentage of a sprint. 7 hours. pm.GREI-d-2.7.1 NIH, yr2, aim7, task1: R&D UI modules for creating datasets and supporting publishing workflows pm.GREI-d-2.7.2 NIH, yr2, aim7, task2: Implement UI modules for creating datasets and publishing workflows UI Tasks related to the user interface (UI) or frontend development labels Mar 13, 2024
@coveralls
Copy link

coveralls commented Mar 13, 2024

Coverage Status

coverage: 97.26% (+0.08%) from 97.179%
when pulling 8cb9d10 on feature/322-support-infinite-scrolling
into a9d62da on develop.

@MellyGray MellyGray self-assigned this Mar 13, 2024
MellyGray
MellyGray previously approved these changes Mar 14, 2024
Copy link
Contributor

@MellyGray MellyGray left a comment

Choose a reason for hiding this comment

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

LGTM!

@g-saracca
Copy link
Contributor Author

All the recommended changes are ready. Thanks a lot, they were very useful! I just didn't do the one to implement the getDatasetsWithCount in the view with normal pagination in order not to extend too much this issue.

@GPortas GPortas self-assigned this Mar 21, 2024
Copy link
Contributor

@GPortas GPortas left a comment

Choose a reason for hiding this comment

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

This looks really good! @GermanSaracca

I think we should add a section about the infinite scrolling (mentioning next steps like the toggle button or configuration variable) on the Collection Page in the README section for changes to the original functionality: https:/IQSS/dataverse-frontend?tab=readme-ov-file#changes-in-functionality-behavior

I think we should make the results component longer, since it shows about 5 results when the original version shows 10.

infinitescrolling

Original JSF:

jsf

@g-saracca
Copy link
Contributor Author

Hi @GPortas , thanks for the feedback!

  • Container height:
    Making the results container longer will create a doble scrollbar, one on the whole page and one on the results container. At this point, the focus on the height of the container is based on the height of the viewport (60% of its height) to avoid this double scrollbar behaviour. Anyway, when we start adding more elements in the collections page we will have to check how it behaves and adjust it.
  • Results label:
    The case of the message about the results, is that the container itself already has 10 items inside, so thats why "10 of ...". Although you are right, the user didn't see them yet, they are just in the list, I'm thinking we could simplify the text of the results to something like "10 out of 200 datasets" or something like "10 out of 200 datasets displayed".

I'm all ears, let me know what you think.

@g-saracca g-saracca assigned GPortas and unassigned g-saracca Mar 22, 2024
@g-saracca
Copy link
Contributor Author

g-saracca commented Mar 22, 2024

@GPortas I have updated the storybook link from the description in case you have tested from there, it was outdated from another chromatic deploy.
Please check this one

@g-saracca g-saracca assigned g-saracca and unassigned GPortas Mar 22, 2024
@GPortas GPortas self-assigned this Mar 22, 2024
@g-saracca g-saracca assigned GPortas and unassigned g-saracca Mar 25, 2024
@GPortas GPortas merged commit 1ce4113 into develop Mar 25, 2024
14 checks passed
@GPortas GPortas deleted the feature/322-support-infinite-scrolling branch March 25, 2024 13:47
@GPortas GPortas removed their assignment Mar 25, 2024
jayanthkomarraju pushed a commit to jayanthkomarraju/dataverse-frontend that referenced this pull request May 31, 2024
…rolling

Feature/322 support infinite scrolling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pm.GREI-d-2.7.1 NIH, yr2, aim7, task1: R&D UI modules for creating datasets and supporting publishing workflows pm.GREI-d-2.7.2 NIH, yr2, aim7, task2: Implement UI modules for creating datasets and publishing workflows Size: 10 A percentage of a sprint. 7 hours. SPA: Collection Page UI Tasks related to the user interface (UI) or frontend development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants