Conversation
|
cc @3gf8jv4dv |
|
I will take a look later. |
3gf8jv4dv
left a comment
There was a problem hiding this comment.
👍👍👍
可以先把另外两份 i18n 同步下,然后我审一下。
Co-authored-by: 3gf8jv4dv <3gf8jv4dv@gmail.com>
| lastlyOpenedURL = url; | ||
|
|
||
| try { | ||
| Thread.sleep(3000); |
There was a problem hiding this comment.
感觉强制等待三秒会影响到了解登陆流程的用户的使用体验,可能需要更好的逻辑来改善用户体验。
There was a problem hiding this comment.
这个实际上问题不大,登录微软账户也不是什么需要频繁进行的操作
There was a problem hiding this comment.
这个实际上问题不大,登录微软账户也不是什么需要频繁进行的操作
我使用的时候对这个硬控三秒感到很烦躁。至少对我个人来说,这个非常影响使用体验。
There was a problem hiding this comment.
这个实际上问题不大,登录微软账户也不是什么需要频繁进行的操作
我使用的时候对这个硬控三秒感到很烦躁。至少对我个人来说,这个非常影响使用体验。
实际上其他启动器也是类似的逻辑。如果你依然认为有必要更改,可以直接缩短此处的时间。
There was a problem hiding this comment.
可以把等待时间改短一点。我觉得 1s 或 2s 应该够了。
There was a problem hiding this comment.
Pull request overview
This pull request optimizes the Microsoft login page UI/UX with several improvements to make the authentication process clearer and more user-friendly. The changes simplify text descriptions, highlight error messages and authentication codes, add a 3-second delay before opening the browser to ensure users see the code, and reorganize hyperlinks to show them only when needed.
Key Changes
- Simplified and reorganized i18n strings for Microsoft login flow across all language files (Chinese Simplified, Chinese Traditional, and English)
- Refactored
CreateAccountPane.javato display the authentication code more prominently with better error message handling - Added a 3-second delay in
OAuthServer.javabefore opening the browser to give users time to see the code - Updated hyperlink handling to use
setExternalLink()method for cleaner code
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| HMCL/src/main/resources/assets/lang/I18N_zh_CN.properties | Updated Simplified Chinese strings for Microsoft login, simplified instructions, added HTML links in error messages, and added new code display label |
| HMCL/src/main/resources/assets/lang/I18N_zh.properties | Updated Traditional Chinese strings for Microsoft login with simplified instructions and HTML links (contains bugs in URL and direction text) |
| HMCL/src/main/resources/assets/lang/I18N.properties | Updated English strings for Microsoft login with clearer, more concise instructions and embedded hyperlinks |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/account/OAuthAccountLoginDialog.java | Refactored to use setExternalLink() method for hyperlinks instead of manual event handlers |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/account/CreateAccountPane.java | Completely refactored Microsoft login UI with prominent code display, better error handling, and conditional visibility for different UI elements |
| HMCL/src/main/java/org/jackhuang/hmcl/setting/Accounts.java | Updated error message key to use unified add_family message |
| HMCL/src/main/java/org/jackhuang/hmcl/game/OAuthServer.java | Added 3-second delay before opening browser to allow users to see the authentication code |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| runInFX(() -> deviceCode.set(value)) | ||
| )); | ||
|
|
||
| HBox linkBox = new HBox(); |
There was a problem hiding this comment.
[nitpick] The HBox linkBox is created without spacing between hyperlinks. The previous code used FlowPane with setHgap(8) to provide spacing. Consider setting spacing for the HBox: HBox linkBox = new HBox(8); to maintain visual consistency.
| HBox linkBox = new HBox(); | |
| HBox linkBox = new HBox(8); |
| Label hint = new Label(i18n("account.methods.microsoft.code")); | ||
| Label code = new Label(); | ||
| code.setMouseTransparent(true); | ||
| code.setStyle("-fx-font-size: 24"); |
There was a problem hiding this comment.
[nitpick] The font size is set using an inline style string ("-fx-font-size: 24"). This could cause inconsistency with the application's theme. Consider defining a CSS class for the code display or using the theme's font sizes for consistency.
| code.setStyle("-fx-font-size: 24"); | |
| code.getStyleClass().add("microsoft-device-code"); |
| try { | ||
| Thread.sleep(3000); | ||
| } catch (InterruptedException ignored) { | ||
| } |
There was a problem hiding this comment.
Using Thread.sleep() blocks the calling thread for 3 seconds. If this method is called on the UI thread or a thread from a limited thread pool, it could cause unresponsiveness or performance issues. Consider using a scheduled executor or asynchronous delay (e.g., CompletableFuture.delayedExecutor() or JavaFX PauseTransition) instead of blocking the thread.
| if (Accounts.OAUTH_CALLBACK.getClientId().isEmpty()) { | ||
| HintPane hintPane = new HintPane(MessageDialogPane.MessageType.WARNING); | ||
| hintPane.setSegment(i18n("account.methods.microsoft.snapshot")); | ||
| vbox.getChildren().add(hintPane); |
There was a problem hiding this comment.
After the early return on line 295, the detailsPane is never added to detailsContainer. The detailsContainer.getChildren().add(detailsPane) call on line 373 is unreachable for this code path. Consider adding detailsContainer.getChildren().add(detailsPane); before the return statement on line 295.
| vbox.getChildren().add(hintPane); | |
| vbox.getChildren().add(hintPane); | |
| detailsContainer.getChildren().add(detailsPane); |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Glavo
left a comment
There was a problem hiding this comment.
请更新所有语言文件。如果你不会其中的一些语言,那么请删除这些文件中过时的译文,让它们回退到英语,而不是保留这些过时译文。
|
优化后的效果很不错。 |
TODO:i18n