-
Notifications
You must be signed in to change notification settings - Fork 32
🐛 [Frontend] Conversations: fixes #8168
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 contains bug fixes for the Frontend Conversations feature. The changes address various issues including variable naming consistency, annotation serialization, conversation handling, and error handling improvements.
- Improves variable naming for clarity and consistency
- Fixes annotation serialization by ensuring all types include color attribute
- Enhances conversation UI handling for temporary conversation pages
- Updates operation names for consistency with backend expectations
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| WorkbenchUI.js | Renames variable from tempNodeUI to dashedNodeUI for better clarity |
| Annotation.js | Ensures all annotation types include color attribute in serialization |
| Conversations.js | Adds logic to handle temporary conversation pages and prevents errors when removing buttons |
| Workbench.js | Makes giveUniqueNameToNode method private by renaming to __giveUniqueNameToNode |
| StudyUI.js | Changes operation from "delete" to "remove" for consistency |
| Study.js | Improves error handling by restructuring conditional logic for node updates |
| Node.js | Changes operation from "delete" to "remove" for consistency |
| Conversation.js | Fixes message display logic and adds null check for load more button |
services/static-webserver/client/source/class/osparc/conversation/Conversation.js
Show resolved
Hide resolved
|
@mergify queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at 9845835 |
pcrespov
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.
👍
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.
I guess that the backed should invert the order of the messages. It looks strange that the latest is on top
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.
👍
Done 👍 |
|
@mergify requeue |
☑️ This pull request is already queued |
|



What do these changes do?
Fixes:
Related issue/s
How to test
Dev-ops