Prototype of replacing jBox tooltips with native HTML/CSS #4530
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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
|
|
|
|
|
|
Preview URL: https://2f61cfe0.betaflight-configurator.pages.dev |
Done.
The docs suggest this could be straightforward (https://floating-vue.starpad.dev/guide/). I'll have a look, but it may not be till later this month. |
|
I spent a bit of time on this and forgot to link to it, but here is the VERY simplest of code that uses floating-vue to implement tool tips in Betaflight: DavidAnson@3b3cfe9 No styling, formatting, etc., just the first thing I got working. |
|
^^ Okay, now there's a LITTLE bit of styling. |
|
i think all of these font sizes were inadvertently reduced -- not matching surrounding sizes. |
|
@nerdCopter, as part of THIS draft commit? Most of the CSS changes here should be scoped to tooltip UI, but there ARE 3 overflow:hidden that were removed for the initial CSS-only proposal. Those changes should not be necessary with the floating-vue proposal, so I will remove them as I work on that more. However, I would not expect any of the CSS changes to affect font sizes outside tooltips. (Though it's possible someone used a tooltip CSS class where they shouldn't have?) |
|
@DavidAnson , my sincere apologies, such is the case in master as well. |
|
@nerdCopter, no problem, I'm just glad I don't need to track that down! :) |
|
Superseded by #4582 |
@haslinghuis asked me to look at removing this project’s use of jBox. Part 1/2 of that can be found in #4484; this is Part 2/2. However, whereas replacing model dialogs could be done ~without compromise, removing tooltips may not be as clean.
First, the good news:
hover, tooltips open immediately (but I do not see this as a problem).Next, some problems with the prototype that can be fixed:
<br>with\nin about 200 places in the localized strings file, which is easy but not something I wanted to do proactively.cf_tip_left). This would be a simple, one-time manual change. If we wanted to run some code, maybe each tab could be scanned after initialization and updated at runtime. I didn't want to spend time on this prematurely.Finally, the bad news:
I don't know how big a deal loss of formatting is and I don't see a clean way around it other than moving to a dedicated library for tooltips. But at that point, it's not clear to me why the project wouldn't just stay with jBox which is already implemented and working. There could be size or performance benefits to a different library, but I'm not familiar enough with the landscape to recommend one. I didn't explore this option, but assume something could be found with similar requirements as jBox.
So that's the state of things as I understand them. I'd ask interested parties to please play around with this prototype to get sense of what the compromises are. Then let me know what you think! :)
Note: The OSD tab has its own call to initialize jBox/Tooltip. It looks similar, so I did not spend time on that tab assuming there won't be any new issues there.