Skip to content

设备代码流在完成授权后隐藏二维码#6087

Open
CiiLu wants to merge 5 commits into
HMCL-dev:mainfrom
CiiLu:mmsssssss
Open

设备代码流在完成授权后隐藏二维码#6087
CiiLu wants to merge 5 commits into
HMCL-dev:mainfrom
CiiLu:mmsssssss

Conversation

@CiiLu
Copy link
Copy Markdown
Contributor

@CiiLu CiiLu commented May 13, 2026

No description provided.

Comment thread HMCL/src/main/resources/assets/lang/I18N.properties
Comment thread HMCL/src/main/resources/assets/lang/I18N_zh_CN.properties Outdated
Comment thread HMCL/src/main/resources/assets/lang/I18N_zh.properties Outdated
@Glavo
Copy link
Copy Markdown
Member

Glavo commented May 13, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

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 Microsoft device login flow by introducing a new 'DeviceLoginCompleted' state and event. This ensures that users are notified once the authorization step is finished while the launcher completes the remaining login tasks. The changes involve updates to the OAuth callback interface, the addition of a specific event class, and UI logic to display a completion hint. Review feedback correctly identified a grammatical error in the English localization and character inconsistencies in the Traditional Chinese translation, both of which included specific code suggestions for correction.

Comment thread HMCL/src/main/resources/assets/lang/I18N.properties
Comment thread HMCL/src/main/resources/assets/lang/I18N_zh.properties Outdated
Co-authored-by: 3gf8jv4dv <3gf8jv4dv@gmail.com>
Comment thread HMCL/src/main/resources/assets/lang/I18N_zh.properties Outdated
@Glavo
Copy link
Copy Markdown
Member

Glavo commented May 15, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

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 adds a completion event to the OAuth device login flow, enabling the UI to display a message once authorization is finished. It includes updates to the OAuth callback interface, event management, and localized strings. Reviewer feedback highlights the need for explicit error handling in the polling loop to avoid null tokens and suggests a grammatical fix for the English translation.

Comment thread HMCLCore/src/main/java/org/jackhuang/hmcl/auth/OAuth.java
Comment thread HMCL/src/main/resources/assets/lang/I18N.properties
@Glavo
Copy link
Copy Markdown
Member

Glavo commented May 18, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

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 implements a completion event for Microsoft device code login, updating the UI to show a progress indicator and localized message once authorization is finished. Feedback was provided regarding the need for robust error handling in the OAuth polling loop to prevent incorrect success signals on failure, as well as a suggestion to correct the English grammar in the new localization string.

continue;
}

options.callback.loginCompletedDeviceCode();
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.

high

The polling loop should handle other potential error responses from the token endpoint (e.g., access_denied if the user cancels the authorization). Currently, if an unhandled error occurs, the code incorrectly proceeds to call loginCompletedDeviceCode() and returns a Result with null tokens, which may cause issues later. It is safer to check for errors and throw an appropriate exception before signaling completion.

            if (tokenResponse.error != null) {
                handleErrorResponse(tokenResponse);
                throw new RemoteAuthenticationException(tokenResponse.error, tokenResponse.errorDescription, "");
            }

            options.callback.loginCompletedDeviceCode();

account.methods.microsoft.hint=Click the "Log in" button to start adding your Microsoft account.
account.methods.microsoft.methods.device=Log In with QR Code
account.methods.microsoft.methods.device.hint=Scan QR code or visit <a href="%s">%s</a> to complete login, enter <b>%s</b> in the opened page.
account.methods.microsoft.methods.device.hint.completed=Microsoft account authorization is now completed. There are some extra works for us, just wait for a while.
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.

medium

The English phrasing "There are some extra works for us" is grammatically incorrect (work is uncountable here) and sounds unpolished. A more standard phrasing would improve the user experience.

account.methods.microsoft.methods.device.hint.completed=Microsoft account authorization is completed. The remaining steps will be performed automatically, please wait.

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