-
-
Notifications
You must be signed in to change notification settings - Fork 988
Replace jBox tooltips with Floating Vue tooltips, remove jBox assets and references #4552
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.
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
Conversation
""" WalkthroughThe changes replace the tooltip library Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DOM Element
participant JS (gui.js/osd.js)
participant FloatingVue
User->>DOM Element: Hover/focus on .cf_tip/.cf_tip_wide/.osd_tip
JS (gui.js/osd.js)->>FloatingVue: createTooltip(element, content, theme)
FloatingVue-->>DOM Element: Show tooltip with custom theme
User->>DOM Element: Move away or blur
JS (gui.js/osd.js)->>FloatingVue: destroyTooltip(element)
FloatingVue-->>DOM Element: Remove tooltip
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Suggested labels
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
The top issue of multiple tool tips showing at once seems like a case of what I described in my notes above where Floating Vue sometimes fails to hide tooltips. I can search their issue tracker to see if this is discussed, but don't see how it could be a problem with this PR because things usually work correctly and there is nothing timing-sensitive in the changes I made. The bottom issue where there seems to be a little bubble next to the tooltip triangle is something I have not seen. If you know how to reproduce that, it could be helpful. Again, I can search the issue tracker to see if this is mentioned, but do not think it is a problem that can be fixed in this PR. I do not have the time to try to debug and fix the Floating Vue library right now. |
Yeah you asked me for something to work on - but it turns out to be definite more complicated then I thought :) |
@haslinghuis, the issue of tooltips sometimes not hiding seems a lot like Akryum/floating-vue#1081. Unfortunately, we need |
@haslinghuis, please try again, I think I have addressed the issue causing empty tooltips on the Motors tab in a commit I just pushed. (That tab invokes |
…for scenarios that invoke it multiple times
|
Preview URL: https://9b506dcb.betaflight-configurator.pages.dev |
Superseded by #4582 |
This PR completes the request @haslinghuis made in #4460 (comment) to remove jBox from the project by using the Floating Vue tooltip implementation.
Notes:
Related:
Summary by CodeRabbit
New Features
Refactor
Chores