-
Notifications
You must be signed in to change notification settings - Fork 170
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
Resolve loading indicator issues in grading table #3064
Conversation
Current implementation uses the outdated api.showLoadingOverlay(), which causes issues in loading as the loading logo disappears even before the data arrives. This commit uses the current method of using the "loading" field in the AGGrid component, which solves the loading mismatch issue.
ready for review! |
Pull Request Test Coverage Report for Build 11567897786Details
💛 - Coveralls |
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.
Did a preliminary review of this PR, and it looks great! Thanks for the improvements!
Just to clarify but from what I see this does not seem to fully resolve #3048, does it?
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.
Also pushed some cosmetic changes for better UI and code quality, LGTM, thanks!
Hi sayo, apologies for the late reply - it does solve refresh issues for next page / previous page / filters, but not for the dropdown menus for page size or grouping. This is because there wasnt an API to disable them - and I thought it wouldn't be an issue since you couldn't spam backend requests through the dropdown menus easily. Thank you for the review! |
Resolves #3048
Good to do before tackling #2814 so that loading can be more explicity indicated.
This PR resolves the loading indicator bug in the grading table, whereby a request for a new submissions page does not properly get reflected as "loading" until backend data arrives. This problem likely arised due to the use of a deprecated API from AGGrid.
To accompany this change, additional measures have been added to prevent users from spamming new requests while awaiting for data - this disabling is now made possible by the proper loading state.
Description
Type of change
How to test
priv/repo/seeds.exs
, example 20000 students and 100 questions. It should be enough such that backend fetching takes at least 1 second.Checklist