-
Notifications
You must be signed in to change notification settings - Fork 16
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
405 Infinite Scrolling on Dataset Files #415
405 Infinite Scrolling on Dataset Files #415
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks for going over the code with me, just small comment below. Also it is not in the list of changed files, but in the pagination text, I think the word "viewed" sounds better than "seen".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, approved! (FYI about the failing Github action - I ran the component test a second time because I think there was a glitch in the first run of the github action. The component test succeeded on the second run, but then the coverage report failed, but that is only because there was already a coverage report from the first test run. So I think it is all good.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome. Congratulations for the great work on this! @GermanSaracca
I only found this detail about the scroll controller bar. The bar starts above the content of the files tab, instead of starting with the first file. Do you think this is a correct behavior?
I am wondering if the bar should start with the first file instead.
scrolltest.mov
That is indeed the expected behaviour as the scrollable container wraps the criteria form , the table and the rest of elements that may appear (as file selection count info, and download size info). |
What this PR does / why we need it:
Adds infinite scrolling to the files list in the files tab from a dataset.
Which issue(s) this PR closes:
Closes #405
Special notes for your reviewer:
I have created a configuration file to choose between normal pagination with buttons or infinite scroll.
Many components were reused and adapted with some new prop, others I preferred to make an alternative version specifically for this feature.
Many changes were necessary for the file selection but everything still maintains the same flows.
I also took the opportunity to improve the styles of the criteria form, some elements were collapsing or overflowing in mobile.
Suggestions on how to test this:
Step 1: Run the Development Environment
npm i
.cd packages/design-system && npm run build
.cd ../../
..env
file similar to.env.example
, with the variableVITE_DATAVERSE_BACKEND_URL=http://localhost:8000
.cd dev-env
../run-env.sh unstable
.Step 2: Test the Files scrollable Table
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Yes.
How sticky positioning of all elements works with the infinite scrolling 👇🏼
Criteria Form layout fixes & table header action buttons only icons for mobile 👇🏼 :
Old:
New:
Is there a release notes update needed for this change?:
No.
Additional documentation:
No.