Replace jBox tooltips with Tippy.js tooltips, remove jBox assets and references #4582
Conversation
WalkthroughSwaps tooltip library from jBox to tippy.js. Updates package.json dependencies, removes jBox CSS import, sets global tippy defaults, migrates tooltip initialization in GUI and OSD modules to tippy-based creation using title attributes, and replaces jBox-specific styles with a custom tippy theme in main.less. Changes
Sequence Diagram(s)sequenceDiagram
participant DOM as Document
participant GUI as gui.js
participant Tippy as tippy.js
Note over DOM,GUI: Page load
DOM->>GUI: content_ready()
GUI->>DOM: Query ".cf_tip, .cf_tip_wide"
loop for each element
GUI->>GUI: if (!element._tippy)
GUI->>Tippy: tippy(element, { content: title, theme: "custom" })
GUI->>DOM: element.removeAttribute("title")
end
sequenceDiagram
participant OSD as osd.js
participant DOM as .osd_tip elements
participant Tippy as tippy.js
Note over OSD: Re-render/refresh OSD tab
OSD->>DOM: Iterate stored elements
OSD->>Tippy: element._tippy?.destroy()
OSD->>DOM: Collect $(".osd_tip").toArray()
loop for each element
OSD->>Tippy: if (!element._tippy) tippy(element, { content: title, theme: "custom" })
OSD->>DOM: element.removeAttribute("title")
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
|
Preview URL: https://353d3840.betaflight-configurator.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/components/init.js (1)
43-53: Default tooltip props: tighten security and parity.
- Consider defaulting to allowHTML: false and enabling per-instance only when needed.
- Match previous jBox widths via a default maxWidth.
Apply:
tippy.setDefaultProps({ - allowHTML: true, + allowHTML: false, appendTo: () => document.body, delay: 100, interactive: true, placement: "right", theme: "custom", + maxWidth: 180, });If some tooltips require HTML, pass allowHTML: true in those specific tippy() calls.
src/js/tabs/osd.js (1)
3940-3955: Clean up on tab exit to prevent leaks.Destroy OSD tooltips in osd.cleanup().
Add:
osd.cleanup = function (callback) { // unbind "global" events $(document).unbind("keypress"); $(document).off("click", "span.progressLabel a"); + + // destroy OSD tooltips + if (OSD?.data?.tooltips) { + for (const el of OSD.data.tooltips) { + el?._tippy?.destroy(); + } + OSD.data.tooltips = []; + }src/js/gui.js (2)
293-305: Restore “wide vs normal” widths previously provided by jBox.jBox had separate max widths (Tooltip vs Widetip). Mirror that with per-element options.
Apply:
- $(".cf_tip, .cf_tip_wide").each((_, element) => { + $(".cf_tip, .cf_tip_wide").each((_, element) => { const jQueryElement = $(element); const attrTitle = jQueryElement.attr("title"); - if (attrTitle && !element._tippy) { - tippy(element, { - content: attrTitle, - }); + if (attrTitle && (!element._tippy || element._tippy.state?.isDestroyed)) { + const opts = { content: attrTitle, maxWidth: 180 }; + if (jQueryElement.hasClass("cf_tip_wide")) opts.maxWidth = 300; + tippy(element, opts); jQueryElement.removeAttr("title"); } });
293-305: Minor: avoid nesting jQuery ready inside content_ready.content_ready is called after the view is mounted; the extra $(function(){}) is unnecessary.
- // Create tooltips once page is "ready" - $(function () { - $(".cf_tip, .cf_tip_wide").each(…) - }); + // Create tooltips for the current view + $(".cf_tip, .cf_tip_wide").each(…)src/css/main.less (1)
1634-1647: Theme additions good; add top/bottom arrows and width parity.
- Arrow borders for top/bottom placements to match the border color.
- Default/wide max widths to replace jBox-Tooltip/Widetip behavior.
Append near the theme:
.tippy-box[data-theme~="custom"] { background: var(--surface-300); border: 2px solid var(--primary-500); border-radius: 0.5rem; color: var(--text); + max-width: 180px; /* parity with former jBox-Tooltip */ } .tippy-box[data-theme~="custom"][data-placement^="right"] > .tippy-arrow:before { border-right-color: var(--primary-500); left: -9px; } .tippy-box[data-theme~="custom"][data-placement^="left"] > .tippy-arrow:before { border-left-color: var(--primary-500); right: -9px; } +.tippy-box[data-theme~="custom"][data-placement^="top"] > .tippy-arrow:before { + border-top-color: var(--primary-500); + bottom: -9px; +} +.tippy-box[data-theme~="custom"][data-placement^="bottom"] > .tippy-arrow:before { + border-bottom-color: var(--primary-500); + top: -9px; +} +/* optional: wide variant parity with former jBox-Widetip */ +.tippy-box[data-theme~="custom"][data-theme~="wide"] { + max-width: 300px; +}If you prefer pure CSS for wide tips, also pass theme: "custom wide" when creating tippy for .cf_tip_wide.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
package.json(1 hunks)src/components/init.js(2 hunks)src/css/main.less(1 hunks)src/js/browserMain.js(0 hunks)src/js/gui.js(2 hunks)src/js/tabs/osd.js(2 hunks)
💤 Files with no reviewable changes (1)
- src/js/browserMain.js
🔇 Additional comments (5)
package.json (1)
72-72: jBox removal confirmed. No leftover imports, CSS, or references found.src/components/init.js (1)
9-11: CSS import placement OK.Importing tippy.css here centralizes styling; no action needed.
src/js/tabs/osd.js (2)
13-13: Library switch import LGTM.
3940-3955: Destroyed tooltips are not re-initialized (stale _tippy guard).element._tippy remains after destroy(), so the subsequent “!element._tippy” check skips re-creation, leaving tooltips missing after refreshes. Clear the reference or check isDestroyed.
Use either approach:
- for (const element of OSD.data.tooltips) { - element._tippy?.destroy(); - } + for (const element of OSD.data.tooltips) { + if (element._tippy) { + element._tippy.destroy(); + element._tippy = undefined; // ensure re-init on next pass + } + }Or:
- if (attrTitle && !element._tippy) { + if (attrTitle && (!element._tippy || element._tippy.state?.isDestroyed)) { tippy(element, { content: attrTitle }); jQueryElement.removeAttr("title"); }Likely an incorrect or invalid review comment.
src/js/gui.js (1)
4-4: Import switch to tippy OK.
This PR completes the request @haslinghuis made in #4460 (comment) to remove jBox from the project by using the Tippy.js tooltip implementation (after an attempt to use Floating Vue was found to have quirks in #4552).
Notes:
Related:
Summary by CodeRabbit