-
Notifications
You must be signed in to change notification settings - Fork 34
🐛 [Frontend] Fix: Runs listing #8049
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
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.
Pull Request Overview
This PR centralizes server pagination limits and fixes the Runs table to fetch all pages when scrolling, replacing the previous single-page-only behavior. It also introduces manual reload controls and removes the automatic refresh interval.
- Refactored both SubRunsTableModel and RunsTableModel to use a shared
SERVER_MAX_LIMIT
and corrected multi-page fetch logic. - Updated RunsBrowser to add a manual "Reload" button and removed the auto-refresh interval.
- Added image caching in
ImageButtonRenderer
and tweaked notification timeouts in the theme.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
ImageButtonRenderer.js | Added async image-to-data-URI conversion with in-memory cache |
Appearance.js | Adjusted showTimeout and hideTimeout values |
SubRunsTableModel.js | Switched to central SERVER_MAX_LIMIT and updated pagination |
RunsTableModel.js | Adopted central limit and fixed pagination loop |
RunsBrowser.js | Introduced manual reload button; removed auto-refresh |
ActivityCenterWindow.js | Removed obsolete runsBrowser.stopInterval() call |
Comments suppressed due to low confidence (4)
services/static-webserver/client/source/class/osparc/jobs/SubRunsTableModel.js:133
- Consider adding unit tests for the pagination splitting logic to verify multiple fetch requests are triggered correctly when requesting more rows than
SERVER_MAX_LIMIT
.
const nRequests = Math.ceil(reqLimit / serverMaxLimit);
services/static-webserver/client/source/class/osparc/jobs/RunsTableModel.js:83
- [nitpick] Initializing
__includeChildren
tonull
may introduce ambiguity in downstream checks; consider using an explicit boolean default (e.g.,false
) unlessnull
has distinct semantics.
__includeChildren: null,
services/static-webserver/client/source/class/osparc/ui/table/cellrenderer/ImageButtonRenderer.js:57
- [nitpick] Converting each icon to a data URI on first render may delay cell display; consider preloading icons or leveraging the browser's caching mechanisms or service worker to reduce render latency.
fetch(iconUrl)
services/static-webserver/client/source/class/osparc/jobs/ActivityCenterWindow.js:75
- Since the auto-refresh interval was removed from
RunsBrowser
, verify thatsubRunsBrowser.stopInterval()
still references an existing method or remove this call to avoid potentialundefined
errors.
subRunsBrowser.stopInterval();
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.
It looks like you are loading and keeping all pages or rather have a rolling window?
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.
👍
For testing purposes, I cloned the content of a comp_runs 900 times, that's why it seems what I show doesn't change. You can see a minor change when the scroll bar hits the bottom. |
@mergify queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at 0eb2563 |
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.
Thanks @odeimaiz
|
What do these changes do?
Due to a bug in the Runs table, when the user was scrolling only the first page was being fetched (see video in bug-issue). This PR fixes it. The small delay for requesting the next page (requested by @matusdrobuliak66) will come in a follow-up PR.
Fixed:

Related issue/s
How to test
Dev-ops