Skip to content

feat: on_hover - #111

Open
JosephBARBIERDARNAL wants to merge 7 commits into
mainfrom
on_hover
Open

feat: on_hover#111
JosephBARBIERDARNAL wants to merge 7 commits into
mainfrom
on_hover

Conversation

@JosephBARBIERDARNAL

Copy link
Copy Markdown
Contributor

No description provided.

@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@JosephBARBIERDARNAL, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 9 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 981c90dd-93d9-4609-bbda-34f5c7612273

📥 Commits

Reviewing files that changed from the base of the PR and between e7a1027 and bcfb280.

⛔ Files ignored due to path filters (1)
  • ninejs/static/PlotParser.min.js is excluded by !**/*.min.js
📒 Files selected for processing (40)
  • docs/iframes/animation-art.html
  • docs/iframes/animation-hover.html
  • docs/iframes/animation-line.html
  • docs/iframes/animation.html
  • docs/iframes/area-chart.html
  • docs/iframes/area-hover-to-show.html
  • docs/iframes/bar.html
  • docs/iframes/climate-vulnerability-co2.html
  • docs/iframes/coal-production.html
  • docs/iframes/effects-confetti.html
  • docs/iframes/europe-co2.html
  • docs/iframes/facet_wrap.html
  • docs/iframes/household-wealth.html
  • docs/iframes/hover-nearest.html
  • docs/iframes/javascript-animate.html
  • docs/iframes/javascript-click-color.html
  • docs/iframes/javascript-hello.html
  • docs/iframes/linked-map-bars.html
  • docs/iframes/map-belgium-unemployment.html
  • docs/iframes/matplotlib-annotation.html
  • docs/iframes/mondrian.html
  • docs/iframes/mtcars-linked-panels.html
  • docs/iframes/on-click-custom-alert.html
  • docs/iframes/on-click-new-window.html
  • docs/iframes/on-click-stroke.html
  • docs/iframes/quickstart2.html
  • docs/iframes/sao-paulo-hdi.html
  • docs/iframes/south-france-restaurants.html
  • docs/iframes/tooltip-after-stat.html
  • docs/iframes/tooltip-custom-css.html
  • docs/iframes/tooltip-custom-css2.html
  • docs/iframes/tooltip-default.html
  • docs/iframes/tooltip-html-injection.html
  • docs/iframes/zoom.html
  • justfile
  • ninejs/static/PlotParserHover.js
  • ninejs/static/PlotParserInit.js
  • ninejs/static/template.html
  • tests/test-javascript/PlotParser.test.js
  • tests/test-python/test_utils.py
📝 Walkthrough

Walkthrough

Hover handlers are added across tooltip configuration, plot registration, client-side JavaScript wiring, and HTML rendering. Interactive mappings now recognize on_hover, while documentation, stale-JavaScript checks, and formatting configuration reflect the expanded behavior.

Changes

Hover Handler Support

Layer / File(s) Summary
Generic handler and tooltip configuration plumbing
ninejs/utils.py
Tooltip configurations now carry normalized hover handlers across row, grouped, and merged data; shared utilities generate handler IDs and JavaScript for both handler types.
Plot registration and HTML handler injection
ninejs/main.py, ninejs/static/template.html
Interactive plots collect on_hover mappings, include hover handlers in tooltip payloads, and inject generated hover-handler scripts into exported HTML.
Client-side hover handler attachment
ninejs/static/PlotParserHover.js, ninejs/static/PlotParserInit.js
Hover handler references are resolved and attached to node mouseover events through the new handler-based APIs.
Documentation, checks, and formatting support
docs/javascript-reference.md, justfile, .prettierignore, README.md, ninejs/main.py
The JavaScript reference documents hover-handler APIs, just check runs stale-JavaScript validation, the README mentions click/hover events, and formatting excludes the HTML template.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Ggplot
  participant InteractivePlot
  participant TooltipConfig
  participant PlotParserInit
  participant PlotParserHover
  participant HTMLTemplate
  Ggplot->>InteractivePlot: provide on_hover mapping
  InteractivePlot->>TooltipConfig: register hover_handlers
  TooltipConfig->>InteractivePlot: return hover handler payload
  InteractivePlot->>HTMLTemplate: provide hover handler JavaScript
  PlotParserInit->>PlotParserHover: pass hoverHandlers
  PlotParserHover->>PlotParserHover: resolve and attach mouseover handler
  HTMLTemplate->>HTMLTemplate: render hover handler script
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No meaningful description was provided, so the change intent cannot be assessed from it. Add a short description of the hover-handler changes and affected files.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main change by indicating a hover/on_hover feature.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch on_hover

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
ninejs/main.py (1)

200-218: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Wire hover_handlers into the hover runtime. ninejs/static/PlotParserHover.js only resolves globalThis.ninejs.clickHandlers, so the globalThis.ninejs.hoverHandlers block emitted from _extract_handler_javascript(..., "hover") is never used and on_hover callbacks won’t fire.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ninejs/main.py` around lines 200 - 218, Update the hover runtime in
PlotParserHover.js to resolve callbacks from globalThis.ninejs.hoverHandlers,
matching the hover handler block generated by _set_html via
_extract_handler_javascript(..., "hover"). Keep click handling bound to
globalThis.ninejs.clickHandlers and ensure resolved hover callbacks invoke
on_hover.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@README.md`:
- Line 14: Update the README description to hyphenate the event modifiers,
changing “on click/hover events” to “on-click/on-hover events” while leaving the
surrounding wording unchanged.

---

Outside diff comments:
In `@ninejs/main.py`:
- Around line 200-218: Update the hover runtime in PlotParserHover.js to resolve
callbacks from globalThis.ninejs.hoverHandlers, matching the hover handler block
generated by _set_html via _extract_handler_javascript(..., "hover"). Keep click
handling bound to globalThis.ninejs.clickHandlers and ensure resolved hover
callbacks invoke on_hover.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fd93dd7c-c33b-4c5e-a83d-26312f79b527

📥 Commits

Reviewing files that changed from the base of the PR and between ba768d9 and 801e0c9.

📒 Files selected for processing (5)
  • .prettierignore
  • README.md
  • ninejs/main.py
  • ninejs/static/template.html
  • ninejs/utils.py

Comment thread README.md

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ninejs/static/PlotParserHover.js`:
- Around line 80-81: Fix getHoverHandler to look up the requested id in
hover_handlers using its hover-handler parameter, then return the function or
null as currently intended; remove the out-of-scope click_handlers/click_handler
references copied from getClickHandler.
- Around line 393-394: Correct the misspelled setHoverEffectHanlder call to
setHoverEffectHandler, then update the hover setup around setHoverEffect and
setHoverEffectHandler so both behaviors execute through the existing mouseover
callback without registering competing mouseover listeners on plot_element.

In `@ninejs/static/PlotParserInit.js`:
- Around line 109-118: Update the hover configuration initialization near the
clickHandlers setup to populate hover_config.hoverHandlers from
config_data["hover_handlers"]. Preserve the existing setHoverEffectHandler and
setHoverEffect flows so they receive the configured hover callbacks.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3c4154a6-a9b4-4c9f-8829-d2e1294e980a

📥 Commits

Reviewing files that changed from the base of the PR and between 801e0c9 and e7a1027.

⛔ Files ignored due to path filters (1)
  • ninejs/static/PlotParser.min.js is excluded by !**/*.min.js
📒 Files selected for processing (4)
  • docs/javascript-reference.md
  • justfile
  • ninejs/static/PlotParserHover.js
  • ninejs/static/PlotParserInit.js

Comment thread ninejs/static/PlotParserHover.js Outdated
Comment thread ninejs/static/PlotParserHover.js Outdated
Comment thread ninejs/static/PlotParserInit.js
@JosephBARBIERDARNAL JosephBARBIERDARNAL linked an issue Jul 11, 2026 that may be closed by this pull request
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.

feat: on_hover

1 participant