Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses #4909 by moving HMCL’s tooltip behavior/styling away from JavaFX’s built-in Tooltip (which can change font metrics when the OS theme flips in “follow system” mode) to a custom tooltip implementation with explicit CSS styling.
Changes:
- Introduces
JFXTooltip(Popup-based) and updatesFXUtils.install*Tooltiphelpers to use it. - Migrates multiple UI pages/cells from
javafx.scene.control.TooltiptoJFXTooltip(including uninstall logic updates). - Updates
root.csstooltip styling to target.jfx-tooltipand tweaks one sidebar tooltip to use the “slow” delay.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| HMCL/src/main/resources/assets/css/root.css | Replaces .tooltip styling with .jfx-tooltip and adds explicit tooltip sizing/visuals. |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/WorldListPage.java | Switches cell tooltip from JavaFX Tooltip to JFXTooltip. |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/SchematicsPage.java | Migrates tooltip usage to JFXTooltip and updates uninstall behavior. |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java | Uses JFXTooltip for warning tooltips and updates uninstall calls. |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/main/RootPage.java | Changes one tooltip install call from fast to slow delay. |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/main/MainPage.java | Migrates launch-button tooltip logic to JFXTooltip. |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/main/JavaManagementPage.java | Migrates remove-button tooltip to JFXTooltip and cleans imports. |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/construct/JFXTooltip.java | Adds the new tooltip implementation (Popup + fade transitions + delays). |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/account/AccountListPage.java | Migrates item tooltip to JFXTooltip. |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/account/AccountListItemSkin.java | Migrates subtitle tooltip to JFXTooltip. |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/account/AccountAdvancedListItem.java | Migrates item tooltip field to JFXTooltip. |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/FXUtils.java | Reworks tooltip helper APIs to install/uninstall JFXTooltip instead of JavaFX Tooltip. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .jfx-tooltip { | ||
| -fx-text-fill: -monet-inverse-on-surface; | ||
| -fx-background-color: -monet-inverse-surface; | ||
| -fx-background-insets: 0; | ||
| -fx-effect: dropshadow( three-pass-box , rgba(0,0,0,0.5) , 10, 0.0 , 0 , 3 ); | ||
| -fx-font-size: 12px; |
There was a problem hiding this comment.
root.css no longer styles the standard JavaFX .tooltip selector (it was replaced by .jfx-tooltip). However, the codebase still creates/installs JavaFX Tooltip in the embedded JFoenix classes (e.g. com/jfoenix/controls/JFXListCell, com/jfoenix/skins/JFXColorPalette, com/jfoenix/skins/JFXTabPaneSkin). Those tooltips will fall back to the default JavaFX styling and may regress appearance/behavior compared to before. Consider keeping the old .tooltip rules (or applying the same rules to both .tooltip and .jfx-tooltip).
| public void install(Node targetNode) { | ||
| if (attachedNode != null) { | ||
| uninstall(); | ||
| } | ||
| this.attachedNode = targetNode; | ||
|
|
There was a problem hiding this comment.
install(Node) always calls uninstall() whenever attachedNode != null, even if the tooltip is being (re)installed onto the same node. In list cell updateItem paths (e.g. where FXUtils.installSlowTooltip(left, tooltip) is called repeatedly), this causes unnecessary handler churn and transition resets. Consider making install idempotent for the same targetNode (early-return if attachedNode == targetNode) and/or avoid re-registering handlers unless the target node actually changes.
resolves #4909