优化鼠标光标 & 添加按钮等悬停效果#5757
Conversation
|
感觉改之后很多地方没有足够的空间显示阴影，看着很奇怪。 |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the hover animation logic by moving it from RipplerContainer to JFXRippler, introducing a hoverOverlay rectangle and implementing mouse event handlers for background interpolation. The review feedback recommends using Animation or Timeline instead of Transition to prevent visual jumps during rapid mouse movements and suggests simplifying the interpolateBackground method for better efficiency.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3d2410e06
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
/gemini reivew |
|
您好， |
很多控件都是方形的涟漪，很奇怪。 |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request migrates hover animation logic from RipplerContainer to JFXRippler, introducing a hoverOverlay rectangle and mouse event handlers to manage hover states. Feedback focuses on improving the component's reusability by removing hardcoded theme dependencies, correcting the animation interpolators for a more standard UI feel, and optimizing performance in the resetRippler method to prevent excessive object allocation.
| Color onSurface = Themes.getColorScheme().getOnSurface(); | ||
| hoverOverlay.setFill(Color.color(onSurface.getRed(), onSurface.getGreen(), onSurface.getBlue(), 0.04)); |
There was a problem hiding this comment.
The JFXRippler class now has a direct dependency on org.jackhuang.hmcl.theme.Themes. As a general-purpose UI component, it should ideally remain agnostic of the application's specific theme logic. Hardcoding this dependency makes the component less reusable and breaks the separation of concerns. Consider using a CSS-stylable property or a dedicated hoverFill property that can be configured from the outside.
| hoverOverlay.setY(bounds.getMinY() + diffMinY - snappedTopInset()); | ||
| hoverOverlay.setWidth(control.getLayoutBounds().getWidth()); | ||
| hoverOverlay.setHeight(control.getLayoutBounds().getHeight()); | ||
| hoverOverlay.setClip(getMask()); |
There was a problem hiding this comment.
resetRippler is called frequently during layout changes. Calling getMask() here on every pulse is inefficient because it creates a new Node object each time. This can lead to excessive object allocation and GC pressure during UI transitions (e.g., when the control is resizing). Consider caching the mask node or only updating it when the mask type or significant bounds change.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors hover effects and animations across several UI components, primarily removing explicit hand cursor settings from multiple Java classes and CSS styles, and cleaning up hover transition logic in RipplerContainer. Instead, hover animations are centralized in JFXRippler using a new hoverOverlay rectangle. Feedback on these changes suggests adding a null check for hoverOverlay in the mouse event handler to prevent a potential NullPointerException, and optimizing resetRippler by avoiding setting a clip mask when the mask type is a rectangle to reduce GC pressure.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| mouseEventHandler = event -> { | ||
| if (coverAnimation != null) { | ||
| coverAnimation.stop(); | ||
| } | ||
|
|
||
| boolean isEntered = event.getEventType() == MouseEvent.MOUSE_ENTERED; | ||
| coverAnimation = new Timeline(new KeyFrame(Motion.SHORT4, | ||
| new KeyValue(hoverOverlay.opacityProperty(), isEntered ? 1 : 0, isEntered ? Motion.EASE_IN : Motion.EASE_OUT))); | ||
| coverAnimation.play(); | ||
| }; |
There was a problem hiding this comment.
在启用动画的分支中，如果 hoverOverlay 为 null，调用 hoverOverlay.opacityProperty() 将会抛出 NullPointerException。虽然 hoverOverlay 在 createRippleUI() 中进行了初始化，但为了保持与其他方法（如 interpolateBackground 和 resetRippler）中对 hoverOverlay 进行空检查的一致性，建议在此处也添加空检查。
| mouseEventHandler = event -> { | |
| if (coverAnimation != null) { | |
| coverAnimation.stop(); | |
| } | |
| boolean isEntered = event.getEventType() == MouseEvent.MOUSE_ENTERED; | |
| coverAnimation = new Timeline(new KeyFrame(Motion.SHORT4, | |
| new KeyValue(hoverOverlay.opacityProperty(), isEntered ? 1 : 0, isEntered ? Motion.EASE_IN : Motion.EASE_OUT))); | |
| coverAnimation.play(); | |
| }; | |
| mouseEventHandler = event -> { | |
| if (hoverOverlay == null) { | |
| return; | |
| } | |
| if (coverAnimation != null) { | |
| coverAnimation.stop(); | |
| } | |
| boolean isEntered = event.getEventType() == MouseEvent.MOUSE_ENTERED; | |
| coverAnimation = new Timeline(new KeyFrame(Motion.SHORT4, | |
| new KeyValue(hoverOverlay.opacityProperty(), isEntered ? 1 : 0, isEntered ? Motion.EASE_IN : Motion.EASE_OUT))); | |
| coverAnimation.play(); | |
| }; |
| if (hoverOverlay != null && control != null) { | ||
| Bounds bounds = control.getBoundsInParent(); | ||
| double diffMinX = Math.abs(control.getBoundsInLocal().getMinX() - control.getLayoutBounds().getMinX()); | ||
| double diffMinY = Math.abs(control.getBoundsInLocal().getMinY() - control.getLayoutBounds().getMinY()); | ||
| hoverOverlay.setX(bounds.getMinX() + diffMinX - snappedLeftInset()); | ||
| hoverOverlay.setY(bounds.getMinY() + diffMinY - snappedTopInset()); | ||
| hoverOverlay.setWidth(control.getLayoutBounds().getWidth()); | ||
| hoverOverlay.setHeight(control.getLayoutBounds().getHeight()); | ||
| hoverOverlay.setClip(getMask()); | ||
| } |
There was a problem hiding this comment.
当 getMaskType() == RipplerMask.RECT 时，hoverOverlay 本身已经是一个大小完全相同的矩形，因此不需要再设置裁剪遮罩（Clip）。在频繁触发的 resetRippler() 中，每次都创建并设置新的 Rectangle 遮罩会带来不必要的性能开销和 GC 压力。建议在遮罩类型为 RECT 时将 Clip 设为 null 以进行优化。
if (hoverOverlay != null && control != null) {
Bounds bounds = control.getBoundsInParent();
double diffMinX = Math.abs(control.getBoundsInLocal().getMinX() - control.getLayoutBounds().getMinX());
double diffMinY = Math.abs(control.getBoundsInLocal().getMinY() - control.getLayoutBounds().getMinY());
hoverOverlay.setX(bounds.getMinX() + diffMinX - snappedLeftInset());
hoverOverlay.setY(bounds.getMinY() + diffMinY - snappedTopInset());
hoverOverlay.setWidth(control.getLayoutBounds().getWidth());
hoverOverlay.setHeight(control.getLayoutBounds().getHeight());
if (getMaskType() == RipplerMask.RECT) {
hoverOverlay.setClip(null);
} else {
hoverOverlay.setClip(getMask());
}
}
其实你这样不断的在 pr 下触发 ai 审查很 spam，我的 inbox 快被这些信息塞爆了 |
|
您好， |
是这样的 |
我已认识到我的错误 现在开始我不会在没有请求审查的时候进行 /gemini review |
虽然上面这条评论以及这条评论本身或许也有些 spam 的意味，但我不想纠结于此。我更想指出的是，我理解你是抱着为社区做贡献的初衷，但以下行为确实已经构成了困扰：
这些操作，包括 PR 审查、进度追踪等，本应由项目维护人员负责，而不是由我们这样的 Outsider 越俎代庖。你的行为会向至少 98 人的 inbox 和邮箱发送通知，实实在在地造成了骚扰。 这条评论也不是旨在贬低或者什么，你也有在社区中发表了有价值的意见，例如： |
总得回复吧?
我先不说 (就在本PR)
这些是没有什么明确性的拒绝及在公测群内反应
这个我有频繁@他人吗? 就@了一次诶
我和这位贡献者是有聊天方式的 所以请不要见怪不怪
我无法反驳
但是我指出一些我个人的观点: 1.基本上 我的一些动作是在公测群里有反馈的若有什么不合理行为我不会继续下去(就像我之前说想把每个停更太久的PR ping下author) 2.反复触发审查并不是我的个人行为 包括主要维护者(Glavo) (如果有什么说错的地方请见谅(因为我不怎么会表达 但是最后也谢谢你指出我的错误)) |
其他的 ChouChiu 说的比较透了。关于你说的这一点，我个人的看法是，对自己的 PR 召唤 AI review 无可厚非 (#6235、#6238 就是良好的例子)；对于其他人的 PR，除非你是维护者，或者你和 PR 作者认识/经过沟通，否则不应该主动召唤 AI review。这本质上还是礼节问题。 此外，也不是所有 PR 都一定要 AI 来 review。一些比较简单、规模小、扫两眼就能确认的 PR 就没有用牛刀的必要。如果反复召唤 AI 导致一个小 PR 堆积了几十甚至上百的评论，那也算是 spam。 Edit 1: 润色 |
thanks 指正 but
这是在公测群里有聊过的 但是 除了这个 #6061 但又说回来 #1771 情况不差不多吗?
|
5 participants
#6107