Filtering from events cells in pages other than first page now works! #1258
Conversation
|
Use Run test server using develop.opencast.org as backend: Specify a different backend like stable.opencast.org: It may take a few seconds for the interface to spin up. |
|
This pull request is deployed at test.admin-interface.opencast.org/1258/2025-05-14_07-19-00/ . |
There was a problem hiding this comment.
Works as expected. My knowledge to the code is limited. Maybe someone else can look into the code, too. For me it's fine.
Good job, thank you @ferishili
|
As an aside, this PR also nicely illustrates that we have code duplication in our table cells which we should get rid of at some point :D. Not necessarily in this PR though, no worries. |
|
You are 100% right about the code duplication! And I can bring this to the /Util and reuse them! |
|
If you are convinced with having the debounce and my proposed way! I can make it reusable |
I did not think about that users double- or even multi-clicking would trigger multiple backend requests. I agree that ideally we want to prevent this from happening. I feel like preventing double clicks should be easier to solve though. I worry that have this much code just for debouncing makes the component as a whole unnecessarily harder to understand. I don't know of a better solution off the top my head though.
I don't think this really achieves that. If understand the code correctly, we first wait 500ms and only then we fetch events and load them into the table. So React is only doing its thing after waiting for 500ms for no reason. Am I getting this wrong?
Yeah, that is very much a matter of design opinions :D. Personally, I don't like introducing artificial lag when avoidable, due to lag making the app feel less responsive. But I'm likely also way more sensitive about lag then the average user ^^. Suggestion: All this talk about debouncing is only tangentially related to the issue this PR is supposed to solve. I think it would be better to factor debouncing out of this PR and continue the discussion about debouncing in another PR or issue. |
|
In this case, and for the favor of keeping the code base simple, we should close this and go for the PR: #1266 from Carolina, which is a simpler version of this and resolves the main problem by adding |
This PR fixes #1229,
Applying a debouncing mechanism when the filtering should be applied in order to fetch the events and load them into the table. Using debounce helps reduce unwanted redundent calls.
In addition, we use useState and useEffect in order to use the advantage of async and watcher properties!
The main problem also was the fact that by applying filters no matter which page the table is set to, it has to be set to first page, so here we use
goToPage(0)TESTING
Please make sure to test all event cells that are filterable, by clicking on the values! don't hesitate to ask if you have any question. Thanks in advance!