Conversation
There was a problem hiding this comment.
Pull request overview
This pull request updates the Microsoft account login dialog to improve the user experience by adding QR code login support and refactoring the authentication flow with a cleaner state-based architecture.
Changes:
- Added QR code generation capability using the nayuki QR code library for device-based authentication
- Refactored login flow using a sealed interface pattern with distinct states (Init, StartAuthorizationCodeLogin, WaitForOpenBrowser, StartDeviceCodeLogin, WaitForScanQrCode, LoginFailed)
- Simplified UI layout by consolidating authentication methods into a single streamlined interface with dynamic state transitions
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle/libs.versions.toml | Added nayuki QR code generator library dependency (version 1.8.0) |
| HMCL/build.gradle.kts | Imported nayuki-qrcodegen library for QR code generation |
| HMCLCore/src/main/java/org/jackhuang/hmcl/util/StringUtils.java | Added XML attribute escaping utility function |
| HMCL/src/main/java/org/jackhuang/hmcl/util/QrCodeUtils.java | New utility class for converting QR codes to SVG path format |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/construct/JFXHyperlink.java | Added constructor accepting text and external link |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/construct/HintPane.java | Added overload for setSegment with custom hyperlink action |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/account/MicrosoftAccountLoginPane.java | Major refactor with state-based architecture and QR code support |
| HMCL/src/main/java/org/jackhuang/hmcl/ui/account/CreateAccountPane.java | Updated to use simplified MicrosoftAccountLoginPane |
| HMCL/src/main/resources/assets/lang/*.properties | Updated localization strings for new login flow |
| HMCL/src/main/resources/assets/css/root.css | Updated CSS for new dialog layout and code box styling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public static String escapeXmlAttribute(String str) { | ||
| return str | ||
| .replace("&", "&") | ||
| .replace("\"", """); |
There was a problem hiding this comment.
The XML escaping is incomplete. Besides "&" and """, other XML special characters like "<", ">", and "'" should also be escaped. Additionally, "&" must be replaced first, before other entities, otherwise you'll double-escape characters. For example, if the input contains "<", it becomes "<", then the "&" becomes "<". The correct order should escape "&" first, then other characters.
| .replace("\"", """); | |
| .replace("\"", """) | |
| .replace("<", "<") | |
| .replace(">", ">") | |
| .replace("'", "'"); |
HMCL/src/main/java/org/jackhuang/hmcl/ui/account/MicrosoftAccountLoginPane.java
Show resolved
Hide resolved
HMCL/src/main/java/org/jackhuang/hmcl/ui/account/MicrosoftAccountLoginPane.java
Outdated
Show resolved
Hide resolved
| public static String escapeXmlAttribute(String str) { | ||
| return str | ||
| .replace("&", "&") | ||
| .replace("\"", """); | ||
| } |
There was a problem hiding this comment.
The new escapeXmlAttribute method lacks test coverage. Given that HMCLCore has a comprehensive test suite (as evidenced by StringUtilsTest.java), a test should be added to verify correct XML escaping behavior, especially for edge cases like strings containing "&", "<", ">", """, and "'" characters.
|
账户列表 > 微软账户 我担心如果遇到长文本，可能窗口会溢出， 
|
|
没有账户时启动游戏： 使用一些字宽较长的字体，可能会有一部分文本被遮挡。 图片中使用 Verdana。 
|
2 participants
No description provided.