更新 JFXListView by Glavo · Pull Request #5569 · HMCL-dev/HMCL

Glavo · 2026-02-18T21:43:42Z

No description provided.

Copilot

Pull request overview

This PR updates the JFoenix-based JFXListView implementation used in HMCL by introducing JFXListView/JFXListCell controls and adjusting the existing skin to use the project’s FXUtils listener helper.

Changes:

Refactor JFXListViewSkin depth handling to use FXUtils.onChangeAndOperate.
Add new JFXListView control with depth, expanded/vertical-gap CSS properties, and optional tooltips.
Add new JFXListCell implementing ripple + expand/collapse gap animations.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File	Description
`HMCL/src/main/java/com/jfoenix/skins/JFXListViewSkin.java`	Simplifies depth listener wiring via `FXUtils.onChangeAndOperate`.
`HMCL/src/main/java/com/jfoenix/controls/JFXListView.java`	New ListView control with depth + styleable expanded/vertical gap + mouse event propagation helper.
`HMCL/src/main/java/com/jfoenix/controls/JFXListCell.java`	New ListCell implementing ripple effect, gap animations, and optional tooltips.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-18T21:52:09Z

HMCL/src/main/java/com/jfoenix/controls/JFXListCell.java

+    protected void updateItem(T item, boolean empty) {
+        super.updateItem(item, empty);
+        if (empty) {
+            setText(null);
+            setGraphic(null);
+            // remove empty (Trailing cells)
+            setMouseTransparent(true);
+            setStyle("-fx-background-color:TRANSPARENT;");
+        } else {
+            setMouseTransparent(false);
+            setStyle(null);
+            if (item instanceof Node newNode) {
+                setText(null);
+                Node currentNode = getGraphic();
+                if (currentNode == null || !currentNode.equals(newNode)) {
+                    cellContent = newNode;
+                    cellRippler.rippler.cacheRippleClip(false);
+                    // build the Cell node
+                    // RIPPLER ITEM : in case if the list item has its own rippler bind the list rippler and item rippler properties
+                    if (newNode instanceof JFXRippler newRippler) {
+                        // build cell container from exisiting rippler
+                        cellRippler.ripplerFillProperty().bind(newRippler.ripplerFillProperty());
+                        cellRippler.maskTypeProperty().bind(newRippler.maskTypeProperty());
+                        cellRippler.positionProperty().bind(newRippler.positionProperty());
+                        cellContent = newRippler.getControl();
+                    }
+                    ((Region) cellContent).setMaxHeight(cellContent.prefHeight(-1));
+                    setGraphic(cellContent);
+                }
+            } else {
+                setText(item == null ? "null" : item.toString());
+                setGraphic(null);
+            }
+            // show cell tooltip if it's toggled in JFXListView
+            if (getListView() instanceof JFXListView<?> listView && listView.isShowTooltip()) {
+                if (item instanceof Label label) {
+                    setTooltip(new Tooltip(label.getText()));
+                } else if (getText() != null) {
+                    setTooltip(new Tooltip(getText()));
+                }
+            }


Tooltips are only set when showTooltip is enabled, but they are never cleared when empty becomes true or when showTooltip is false. Because list cells are reused, stale tooltips can persist on unrelated rows. Consider calling setTooltip(null) in the empty branch and in the non-tooltip branch.

Copilot · 2026-02-18T21:52:10Z

HMCL/src/main/java/com/jfoenix/controls/JFXListView.java

+
+    private void updateVerticalGap() {
+        if (isExpanded()) {
+            currentVerticalGapProperty().set(verticalGap.get());


updateVerticalGap() calls verticalGap.get() directly, but verticalGap may still be null if expandedProperty() is created/invalidated before verticalGapProperty() is ever accessed (e.g., when -jfx-expanded is set via CSS). This can throw an NPE. Consider reading via verticalGapProperty()/getVerticalGap() (or guarding null) to ensure the value is available.

Suggested change

currentVerticalGapProperty().set(verticalGap.get());

currentVerticalGapProperty().set(getVerticalGap());

Copilot · 2026-02-18T21:52:10Z

HMCL/src/main/java/com/jfoenix/controls/JFXListView.java

+    public void propagateMouseEventsToParent() {
+        this.addEventHandler(MouseEvent.ANY, e -> {
+            e.consume();
+            this.getParent().fireEvent(e);


propagateMouseEventsToParent() calls getParent().fireEvent(e) without checking for a null parent. If this is invoked before the control is attached to a scene graph, it will throw an NPE. Consider guarding getParent() != null and/or deferring registration until the control is added to a parent.

Suggested change

this.getParent().fireEvent(e);

if (this.getParent() != null) {

this.getParent().fireEvent(e);

}

Copilot · 2026-02-18T21:52:10Z

HMCL/src/main/java/com/jfoenix/controls/JFXListCell.java

+///
+/// NOTE: passive nodes (Labels and Shapes) will be set to mouse transparent in order to
+/// show the ripple effect upon clicking , to change this behavior you can override the
+/// method {[#makeChildrenTransparent()]


The documentation comment has a malformed reference: method {[#makeChildrenTransparent()] is missing closing }/], which makes the comment confusing and inconsistent with other docs in this package. Please fix the bracket/brace pairing.

Suggested change

/// method {[#makeChildrenTransparent()]

/// method [#makeChildrenTransparent()]

Copilot · 2026-02-18T21:52:10Z

HMCL/src/main/java/com/jfoenix/controls/JFXListCell.java

+        listViewProperty().addListener((listObj, oldList, newList) -> {
+            if (newList instanceof JFXListView<?> listView) {
+                listView.currentVerticalGapProperty().addListener((o, oldVal, newVal) -> {
+                    cellRippler.rippler.setClip(null);


A listener is added to listView.currentVerticalGapProperty() when the cell is attached to a JFXListView, but it is never removed when the cell is detached or when listViewProperty() changes. Because the listener lambda captures the cell, this can keep cells strongly reachable from the list view and leak memory over time. Consider removing the listener from oldList, or registering it via a WeakChangeListener (e.g., FXUtils.onWeakChange).

Copilot · 2026-02-18T21:52:10Z

HMCL/src/main/java/com/jfoenix/controls/JFXListCell.java

+                        getListView().requestLayout();
+                    } else {
+                        // fake expand state
+                        double gap = clip.getY() * 2;


The currentVerticalGapProperty() listener uses clip.getY() to compute gap, but clip is only initialized during layoutChildren(). If the gap property changes before the first layout pass, this will throw an NPE. Consider initializing clip earlier, or computing the gap from newVal/getGap() (with null-safe handling) rather than dereferencing clip.

Suggested change

double gap = clip.getY() * 2;

double previousGap = oldVal == null ? 0.0 : oldVal.doubleValue();

double gap = previousGap * 2;

Copilot · 2026-02-18T21:52:11Z

HMCL/src/main/java/com/jfoenix/controls/JFXListCell.java

+                    if (newNode instanceof JFXRippler newRippler) {
+                        // build cell container from exisiting rippler
+                        cellRippler.ripplerFillProperty().bind(newRippler.ripplerFillProperty());
+                        cellRippler.maskTypeProperty().bind(newRippler.maskTypeProperty());
+                        cellRippler.positionProperty().bind(newRippler.positionProperty());
+                        cellContent = newRippler.getControl();


In updateItem, when newNode is a JFXRippler you bind cellRippler properties to it, but the cell is reused and these properties are never unbound. Rebinding on a later item can throw, and non-rippler items will still be affected by the old bindings. Consider unbinding before binding (and resetting to defaults) each time the graphic changes, and unbinding when newNode is not a JFXRippler.

Copilot · 2026-02-18T21:52:11Z

HMCL/src/main/java/com/jfoenix/controls/JFXListCell.java

+                        cellRippler.positionProperty().bind(newRippler.positionProperty());
+                        cellContent = newRippler.getControl();
+                    }
+                    ((Region) cellContent).setMaxHeight(cellContent.prefHeight(-1));


cellContent is cast to Region unconditionally. If a list item provides a Node that isn't a Region (or a JFXRippler whose getControl() isn't a Region), this will throw ClassCastException. Consider checking instanceof Region before setting max height, or handling non-Region nodes gracefully.

Suggested change

((Region) cellContent).setMaxHeight(cellContent.prefHeight(-1));

if (cellContent instanceof Region region) {

region.setMaxHeight(region.prefHeight(-1));

}

Glavo added 15 commits February 19, 2026 05:13

init

0701563

cleanup

9d02df2

cleanup

388b7c9

cleanup

2a264be

cleanup

0d6f2e1

cleanup

99dc7c6

cleanup

1929e4b

update

8f058c5

update

57ec65e

update

02d178d

update

06393f5

update

e4ca815

update

c0d90f6

update

7d7faba

更新 JFXListView

da80edb

Glavo requested a review from Copilot February 18, 2026 21:47

Copilot started reviewing on behalf of Glavo February 18, 2026 21:48 View session

update

cd30fa1

Copilot AI reviewed Feb 18, 2026

View reviewed changes

Merge remote-tracking branch 'upstream/main' into list-view

e46d5db

Glavo merged commit b6fcbb8 into HMCL-dev:main Feb 19, 2026
2 checks passed

Glavo deleted the list-view branch February 19, 2026 12:46

Glavo added a commit to Glavo/HMCL that referenced this pull request Mar 29, 2026

更新 JFXListView (HMCL-dev#5569)

93ecf57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

更新 JFXListView#5569

更新 JFXListView#5569
Glavo merged 17 commits intoHMCL-dev:mainfrom
Glavo:list-view

Glavo commented Feb 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	currentVerticalGapProperty().set(verticalGap.get());
	currentVerticalGapProperty().set(getVerticalGap());

	/// method {[#makeChildrenTransparent()]
	/// method [#makeChildrenTransparent()]

	double gap = clip.getY() * 2;
	double previousGap = oldVal == null ? 0.0 : oldVal.doubleValue();
	double gap = previousGap * 2;

-                    ((Region) cellContent).setMaxHeight(cellContent.prefHeight(-1));
+                    if (cellContent instanceof Region region) {
+                        region.setMaxHeight(region.prefHeight(-1));
+                    }

Conversation

Glavo commented Feb 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants