Skip to content

fix(spx-gui): fix UIIcon tooltip trigger resolution & support SVG popup triggers#3197

Merged
nighca merged 3 commits into
goplus:devfrom
cn0809:dev
May 21, 2026
Merged

fix(spx-gui): fix UIIcon tooltip trigger resolution & support SVG popup triggers#3197
nighca merged 3 commits into
goplus:devfrom
cn0809:dev

Conversation

@cn0809
Copy link
Copy Markdown
Collaborator

@cn0809 cn0809 commented May 21, 2026

No description provided.

Copy link
Copy Markdown
Contributor

@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 generalizes the popup trigger logic by replacing HTMLElement with Element and Node types across several UI components and utilities. These changes allow the system to support SVG elements as triggers and correctly handle Vue development mode scenarios where a component's root might be a comment or text node. A new helper function, resolveElementFromComponentRoot, has been implemented to traverse the DOM and locate the first valid element when an anchor node is provided. Additionally, new unit tests have been introduced to verify these improvements. I have no feedback to provide as there were no review comments to evaluate.

@cn0809 cn0809 marked this pull request as ready for review May 21, 2026 05:23
Copy link
Copy Markdown
Contributor

@fennoai fennoai Bot left a comment

Choose a reason for hiding this comment

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

The type-widening from HTMLElement to Element is the correct minimal fix for SVG trigger support, and resolveElementFromComponentRoot is well-structured with good inline documentation. A few minor findings below.

Comment thread spx-gui/src/components/ui/popup/trigger.ts Outdated
Comment thread spx-gui/src/components/ui/popup/trigger.ts Outdated
Comment thread spx-gui/src/components/ui/popup/trigger.ts Outdated
Comment thread spx-gui/src/components/ui/popup/trigger.ts
Comment thread spx-gui/src/components/ui/popup/trigger.test.ts
Comment thread spx-gui/src/components/ui/popup/trigger.ts Outdated
@cn0809 cn0809 changed the title fix(spx-gui): support comment-rooted and SVG popup triggers fix(spx-gui): fix UIIcon tooltip trigger resolution & support SVG popup triggers May 21, 2026
@nighca nighca merged commit 0fdd4ea into goplus:dev May 21, 2026
4 checks passed
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.

2 participants