Skip to content

fix: restore tooltip display when PopupMenuHandler is unavailable #970

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.

Sign up for GitHub

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

Jump to bottom
Merged
tbouffard merged 3 commits into from
Dec 10, 2025
Merged

fix: restore tooltip display when PopupMenuHandler is unavailable #970

tbouffard merged 3 commits into from
Dec 10, 2025

Conversation

@chrisob194
Copy link

@chrisob194 chrisob194 commented Dec 9, 2025 •
edited by coderabbitai bot
Loading

Overview

As described in issue #961 , the current version does not display any tooltip when the plugin PopupMenuHandler is not added to the list of the plugins. This is a common case when BaseGraph is used over Graph (as suggested in the documentation, BaseGraph should always be the choice for production).

The solution now handle this case correctly: when popupMenuHandler is not available (undefined) or the menu is not showing, then the tooltip will be displayed.

!popupMenuHandler?.isMenuShowing()

To verify that the solution works, use the story "Wires" which has been edited in order to not include the PopupMenuHandler. Tooltips on vertices and edges should now be displayed correctly.

Summary by CodeRabbit

  • Bug Fixes

    • Tooltips now appear in additional cases where they were previously suppressed, improving discoverability.

  • Chores

    • Removed registration of an internal popup menu plugin from a demo/story, cleaning up example setup.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 9, 2025 •
edited
Loading

Caution

Review failed

The pull request is closed.

Walkthrough

Use optional chaining when checking PopupMenuHandler in TooltipHandler so tooltips can show when PopupMenuHandler is absent or not showing; remove PopupMenuHandler from the plugins array in the Wires story.

Changes

Cohort / File(s) Change Summary
TooltipHandler logic fix
packages/core/src/view/plugins/TooltipHandler.ts		 Replaced explicit truthy check with optional chaining in the tooltip guard (!popupMenuHandler?.isMenuShowing()), allowing tooltips when PopupMenuHandler is absent or its isMenuShowing() is false.
Wires story plugin configuration
packages/html/stories/Wires.stories.ts		 Removed PopupMenuHandler from the Graph plugins array in the Wires story.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Check TooltipHandler reset/display logic to ensure optional chaining covers absent plugin and preserves previous behavior when plugin exists.
  • Verify the Wires story renders as expected without registering PopupMenuHandler and that no other stories rely on it.

Possibly related PRs

Suggested labels

bug

Suggested reviewers

  • tbouffard

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the specific fix addressing the TooltipHandler dependency on PopupMenuHandler, following Conventional Commits format.
Description check ✅ Passed The description covers the problem, solution, how it was verified, and follows the template with all key elements including issue reference and testing approach.
Linked Issues check ✅ Passed The PR implements the exact solution proposed in issue #961: using optional chaining to handle the absence of PopupMenuHandler, allowing tooltips to display correctly regardless of PopupMenuHandler presence.
Out of Scope Changes check ✅ Passed All changes directly address the bug in issue #961: TooltipHandler fix for optional PopupMenuHandler check and removal of PopupMenuHandler from Wires.stories.ts for verification. No unrelated changes detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fe66ca8 and 3a49730.

📒 Files selected for processing (1)
  • packages/html/stories/Wires.stories.ts (0 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chrisob194 chrisob194 closed this Dec 10, 2025
@chrisob194 chrisob194 reopened this Dec 10, 2025
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 10, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@tbouffard tbouffard changed the title Fix(TooltipHandler): the tooltip plugin always requires the popup menu plugin in order to work fix: restore tooltip display when PopupMenuHandler is unavailable Dec 10, 2025
@tbouffard tbouffard added the bug Something isn't working label Dec 10, 2025
tbouffard
tbouffard approved these changes Dec 10, 2025
View reviewed changes
Copy link
Member

@tbouffard tbouffard left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution

@tbouffard tbouffard merged commit 87df189 into maxGraph:main Dec 10, 2025
7 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Dec 17, 2025
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tbouffard tbouffard tbouffard approved these changes

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Use of PopupMenuHandler in TooltipHandler

2 participants

@chrisob194 @tbouffard