Skip to content

refactor(spx-gui): replace naive-ui dropdown and tooltip popups#3032

Draft
cn0809 wants to merge 1 commit intogoplus:devfrom
cn0809:naive-ui
Draft

refactor(spx-gui): replace naive-ui dropdown and tooltip popups#3032
cn0809 wants to merge 1 commit intogoplus:devfrom
cn0809:naive-ui

Conversation

@cn0809
Copy link
Copy Markdown
Collaborator

@cn0809 cn0809 commented Apr 13, 2026

close: #3015

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request replaces the Naive UI-based UIDropdown and UITooltip components with a custom implementation using @floating-ui/dom and a new internal popup management system. It includes updates to UIConfigProvider and UIModal to support this foundation, along with simplified class merging across multiple UI components. Review feedback identifies a logic error in the resolveMaybeRef utility and a functional issue where nested teleported popups may cause parent dropdowns to close prematurely.

Comment on lines +118 to +123
function resolveMaybeRef<T>(value: MaybeRefOrGetter<T> | null | undefined): T | null
function resolveMaybeRef<T>(value: MaybeRefOrGetter<T> | null | undefined, fallback: T): T
function resolveMaybeRef<T>(value: MaybeRefOrGetter<T> | null | undefined, fallback?: T) {
if (value == null) return fallback ?? null
return toValue(value) as T
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The resolveMaybeRef function does not correctly apply the fallback when the provided MaybeRefOrGetter resolves to null or undefined. It only checks if the value (the ref or getter itself) is nullish. If a ref is passed but its current value is undefined, the fallback will be ignored, which likely isn't the intended behavior for default values like placement or offset.

Suggested change
function resolveMaybeRef<T>(value: MaybeRefOrGetter<T> | null | undefined): T | null
function resolveMaybeRef<T>(value: MaybeRefOrGetter<T> | null | undefined, fallback: T): T
function resolveMaybeRef<T>(value: MaybeRefOrGetter<T> | null | undefined, fallback?: T) {
if (value == null) return fallback ?? null
return toValue(value) as T
}
function resolveMaybeRef<T>(value: MaybeRefOrGetter<T> | null | undefined): T | null
function resolveMaybeRef<T>(value: MaybeRefOrGetter<T> | null | undefined, fallback: T): T
function resolveMaybeRef<T>(value: MaybeRefOrGetter<T> | null | undefined, fallback?: T) {
const resolved = value == null ? undefined : toValue(value)
return (resolved ?? fallback) ?? null
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace Naive UI popup primitives (dropdown / tooltip)

1 participant