103 rucio next page

Fixed pagination in Rucio search results:

esap-general#103 (closed)

requested review from @swinbank

assigned to @kliffen

Thanks for this!

My immediate reaction is that I don't really know what's happening here. That's fine — I'm happy to admit my ignorance — but it suggests that perhaps a bit more explanation (a comment, or whatever) would help.

My other reaction: I'm sure you've deployed & tested this locally, but given the current ASTRON enthusiasm for CI/CD I'm wondering if there's a way we can deploy this branch to sdc-dev.astron.nl for testing before merging? That would be a neat trick. @kliffen or @lautenbach might know...

added 1 commit

• be550ac2 - better code clean up and comment

Compare with previous version

resolved all threads

approved this merge request

resolved all threads

added 1 commit

• da35043d - Reformatting + better event handling

Compare with previous version

I did quite a rigorous refactoring. The useEffect was not needed, because it would trigger the first query to be executed twice when you click submit. Hence I moved it to a regular function which only gets triggered on a click of the pagination.

I also discovered a bug in the Paginate.js, where it would fire an event with undefined text for the current page since the element is disabled. I put a workaround for it now, since I am not sure if other components rely on this behavior.

Additionally I moved the "failure" states earlier in the component, which perform a small check (for loading, error etc.) and then render their respective component and then leave the complex constructing of the table at the end. Added also comments to make it more clear what each part renders.

requested review from @lautenbach

approved this merge request

added 1 commit

• dae69069 - Rename event to clickEvent

Compare with previous version

approved this merge request

resolved all threads

merged

mentioned in commit 6853ff42

mentioned in issue esap-general#153 (closed)