Skip to content

Refactor: InstallersPageSkin versionNamePane#6349

Open
KSSJW wants to merge 7 commits into
HMCL-dev:mainfrom
KSSJW-Contribution:installers-page-skin-version-name-pane-refactor
Open

Refactor: InstallersPageSkin versionNamePane#6349
KSSJW wants to merge 7 commits into
HMCL-dev:mainfrom
KSSJW-Contribution:installers-page-skin-version-name-pane-refactor

Conversation

@KSSJW

@KSSJW KSSJW commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

重构InstallersPageSkin.versionNamePane

  • 修复了control.txtName被截断的问题。
  • 更改了输入框长度。
  • 为输入框添加了清空重置的按钮。

实况

更改前 更改后
1 2

@KSSJW

KSSJW commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces "clear" and "reset" buttons to the version name text field pane in the installer pages. The review feedback highlights several areas for improvement: the reset button should only be added conditionally when the page is an instance of InstallersPage to prevent displaying a non-functional button on other subclasses; the clear button should be disabled when the text field is empty; the visibility of setTxtNameWithLoaders() should remain private to preserve encapsulation; and an unused import in AbstractInstallersPage.java should be removed.

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.

Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/download/AbstractInstallersPage.java Outdated
Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/download/InstallersPage.java Outdated
Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/download/AbstractInstallersPage.java Outdated
@KSSJW

KSSJW commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request enhances the version name pane in the installer pages by adding a clear button to clear the version name text field and a reset button to restore the default name. The reviewer suggested refactoring the implementation to avoid using instanceof in the abstract base class's skin, which introduces tight coupling. Instead, they recommended defining polymorphic methods canResetDefaultName() and resetDefaultName() in AbstractInstallersPage and overriding them in InstallersPage to adhere to the Open-Closed Principle.

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.

Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/download/AbstractInstallersPage.java Outdated
Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/download/InstallersPage.java Outdated
@KSSJW

KSSJW commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces UI enhancements to the installers page, adding "clear" and "reset" buttons to the version name input pane. This is supported by new abstract methods in AbstractInstallersPage implemented by its subclasses. The review feedback suggests correcting a spelling mistake by renaming showExpendPane to showExtendedPane, and maintaining proper encapsulation by keeping the overridden methods as protected instead of promoting them to public.

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.

Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/download/AbstractInstallersPage.java Outdated
Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/download/AbstractInstallersPage.java Outdated
@KSSJW

KSSJW commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces clear and reset buttons to the installer version name input pane in AbstractInstallersPage when showExtendPane() is enabled, implementing these behaviors for InstallersPage (enabled) and AdditionalInstallersPage (disabled). The review feedback suggests binding the disable properties of the clear and reset buttons to the disabled state of the text input field to prevent interactions when the input is disabled.

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.

@KSSJW KSSJW left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed

@KSSJW KSSJW marked this pull request as ready for review July 9, 2026 09:56
@Glavo

Glavo commented Jul 9, 2026

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2c7f6d1b0d

ℹ️ 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".

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.

2 participants