refactor(spx-gui): replace naive-ui message infrastructure#3064
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces the Naive UI message provider with a custom implementation, introducing UIMessageProvider and UIMessageItem components. The feedback identifies several opportunities for code simplification, such as removing a redundant computed property for icon types, an unnecessary async keyword, and extraneous forceReflow calls in the transition hooks. Additionally, a bug was noted where the height transition in UIMessageItem will not animate correctly without adding max-height to the CSS transition-property.
There was a problem hiding this comment.
Well-structured refactor removing the Naive UI message dependency and replacing it with a focused custom implementation. The test suite is thorough and the animation technique is correctly commented. A few items worth addressing:
Summary of findings below — ranging from a documentation inaccuracy about how the animation actually works, to a missing public export, to minor accessibility and resource-management concerns.
| content, | ||
| visible: true | ||
| } | ||
| messages.value.push(message) |
There was a problem hiding this comment.
No upper bound on concurrent messages. messages.value grows without a cap. The withLoading path (duration = 0) never auto-hides, so a Promise that never settles leaves a permanent entry. A tight loop calling any messageApi.* method can also fill the list before timers fire. Consider limiting to e.g. 5–10 visible messages (dropping the oldest or ignoring new ones) to guard against accidental runaway callers in a long-lived editor session.
There was a problem hiding this comment.
-
withLoading 对应的是任务生命周期,如果 Promise 一直不结束,loading 一直存在在语义上也是合理的
-
普通消息本身会自动消失,目前也没遇到消息堆积导致的实际问题,暂时先不加吧;如果后续要加上限,需要先明确处理策略:
- withLoading 这类消息是否应该参与上限控制
- 超限时是 丢最旧的,还是 忽略最新的
- 正在 leave 的消息算不算在上限里
- 被上限挤掉的消息是否需要补事件、日志或别的可观测性
There was a problem hiding this comment.
Pull request overview
This PR continues the Naive UI cleanup in spx-gui by replacing the existing message feedback infrastructure (NMessageProvider/useMessage) with an in-house UIMessageProvider + injected message API, and documenting/demoing it in the UI design docs.
Changes:
- Replaced the Naive UI–backed
useMessageimplementation with an injectedMessageApiprovided byUIMessageProvider. - Added a new teleported message stack (
UIMessageItem+TransitionGroup) and regression tests for the new message API. - Updated UI design docs and UI README to reflect the new
UIMessageprimitive.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| spx-gui/src/pages/docs/ui-design/index.vue | Adds a UIMessage showcase section and demo handlers. |
| spx-gui/src/components/ui/message/index.ts | Switches the public exports to the new provider + injected API. |
| spx-gui/src/components/ui/message/index.test.ts | Adds regression coverage for the new useMessage behavior and events. |
| spx-gui/src/components/ui/message/UIMessageProvider.vue | Implements the new message provider, API injection, and teleported stack rendering. |
| spx-gui/src/components/ui/message/UIMessageItem.vue | Adds a message item component with enter/leave animations. |
| spx-gui/src/components/ui/README.md | Removes the outdated Naive UI message-provider note. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ions to TransitionGroup
| import * as projectApis from '@/apis/project' | ||
| import { useModalEvents } from '@/components/ui/modal/UIModalProvider.vue' | ||
| import { useMessageEvents } from '../ui/message/UIMessageProvider.vue' | ||
| import { useMessageEvents } from '../ui/message' |
There was a problem hiding this comment.
这里直接从 @/components/ui import 更合适
update: #3018