-
Notifications
You must be signed in to change notification settings - Fork 32
✨ [Frontend] Feature: Saving pipeline #8054
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
✨ [Frontend] Feature: Saving pipeline #8054
Conversation
giancarloromeo
left a comment
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.
Nice
matusdrobuliak66
left a comment
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.
Nice improvement 👍 thanks
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 adds a visual "Saving..." indicator to the navigation bar by tracking a new savePending flag on the Study model. It introduces a periodic diff check in the StudyEditor to update that flag and binds it to a new icon in the NavigationBar.
- Adds a
savePendingproperty toStudyand ignores it during serialization. - Introduces
DIFF_CHECK_INTERVALand a saving timer inStudyEditorto setsavePending. - Binds
study.changeSavePendingevents inNavigationBarto toggle a “Saving…” icon.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| services/static-webserver/client/source/class/osparc/navigation/NavigationBar.js | Bind savePending to a new "saving-study-icon" control and register the icon in left-items |
| services/static-webserver/client/source/class/osparc/desktop/StudyEditor.js | Define DIFF_CHECK_INTERVAL, start/stop a saving timer to call setSavePending, integrate into lifecycle |
| services/static-webserver/client/source/class/osparc/data/model/Study.js | Declare savePending property with change event and exclude it from serialization |
Comments suppressed due to low confidence (2)
services/static-webserver/client/source/class/osparc/desktop/StudyEditor.js:108
- [nitpick] The name
DIFF_CHECK_INTERVALis generic—consider renaming it to something likeSAVE_PENDING_CHECK_INTERVALto clarify its specific role alongsideAUTO_SAVE_INTERVAL.
DIFF_CHECK_INTERVAL: 300,
services/static-webserver/client/source/class/osparc/desktop/StudyEditor.js:231
- There are no tests covering the new saving indicator or the logic in
__startSavingTimer. Adding unit or integration tests would help ensure this behavior works correctly and prevent regressions.
this.__startSavingTimer();
services/static-webserver/client/source/class/osparc/navigation/NavigationBar.js
Outdated
Show resolved
Hide resolved
services/static-webserver/client/source/class/osparc/navigation/NavigationBar.js
Outdated
Show resolved
Hide resolved
services/static-webserver/client/source/class/osparc/desktop/StudyEditor.js
Show resolved
Hide resolved
services/static-webserver/client/source/class/osparc/desktop/StudyEditor.js
Show resolved
Hide resolved
services/static-webserver/client/source/class/osparc/data/model/Study.js
Show resolved
Hide resolved
GitHK
left a comment
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.
very nice addition
sanderegg
left a comment
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.
very nice! Maybe check the copilot review
…core into feature/saving-study
…core into feature/saving-study
|
@mergify queue |
🟠 Waiting for conditions to match
|
|



What do these changes do?
This PR bring a small icon to the navigation bar that shows users when a study is being saved.
This makes sense only when users are dealing with pipelines, that's why it won't be enabled in
standaloneorappui modes.Reminder of how the save pipeline works:
Related issue/s
How to test
Dev-ops