Skip to content

Fix #5307 #5037 : 列表点击后无法 esc 返回#5311

Closed
CiiLu wants to merge 1 commit into
HMCL-dev:mainfrom
CiiLu:focus
Closed

Fix #5307 #5037 : 列表点击后无法 esc 返回#5311
CiiLu wants to merge 1 commit into
HMCL-dev:mainfrom
CiiLu:focus

Conversation

@CiiLu

@CiiLu CiiLu commented Jan 24, 2026

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes issues where, after clicking into certain list views, pressing Esc no longer triggers the global “back” navigation handler.

Changes:

  • Disables focus traversal on the resourcepack list JFXListView.
  • Disables focus traversal on all JFXListView instances created by ToolbarListPageSkin.

Reviewed changes

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

File Description
HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ResourcepackListPage.java Prevents the resourcepack list from becoming focus-traversable to avoid consuming Esc/back.
HMCL/src/main/java/org/jackhuang/hmcl/ui/ToolbarListPageSkin.java Applies the same focus-traversable change to all list pages using the shared skin.

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

center.getStyleClass().add("large-spinner-pane");
center.loadingProperty().bind(control.loadingProperty());

listView.setFocusTraversable(false);

Copilot AI Jan 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting the ListView to non-focus-traversable is a very broad fix for the ESC/back issue and can regress keyboard interaction (arrow-key navigation, type-to-select, accessibility focus). In other list pages (e.g., versions/ModListPageSkin.java and versions/DatapackListPageSkin.java) the pattern is to bypass ListViewBehavior only for ESC via FXUtils.ignoreEvent(listView, KeyEvent.KEY_PRESSED, e -> e.getCode() == KeyCode.ESCAPE) so ESC can reach the global handler without disabling focus entirely. Consider switching to that approach here instead of setFocusTraversable(false).

Copilot uses AI. Check for mistakes.
Comment on lines 64 to 66
this.listView.setPadding(Insets.EMPTY);
this.listView.setFocusTraversable(false);
this.listView.setCellFactory(listView -> createListCell((JFXListView<E>) listView));

Copilot AI Jan 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

listView.setFocusTraversable(false) changes focus behavior for every page using ToolbarListPageSkin (Java management, installer list, world list, etc.) and can break keyboard navigation/selection in these lists. The underlying ESC/back problem is typically handled more narrowly by letting ESC bypass the ListView dispatcher (see FXUtils.ignoreEvent(...) usage in versions/ModListPageSkin.java / versions/DatapackListPageSkin.java). Consider replacing this with an ESC-specific FXUtils.ignoreEvent(listView, KeyEvent.KEY_PRESSED, e -> e.getCode() == KeyCode.ESCAPE) so ESC reaches DecoratorController’s global handler while preserving normal focus/keyboard behavior.

Copilot uses AI. Check for mistakes.
@Glavo

Glavo commented Jan 27, 2026

Copy link
Copy Markdown
Member

这样改是不是会进一步破坏用键盘控制时的体验?

@CiiLu CiiLu closed this Jan 30, 2026
@CiiLu CiiLu deleted the focus branch February 3, 2026 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants