Conversation
改变（button/large/primary/shadow/square/default）背景颜色（仅测试）
There was a problem hiding this comment.
Code Review
This pull request introduces a new community login page UI and updates an asset path in a demo file. The review feedback identifies a path inconsistency in the demo, Windows-incompatible filenames containing colons, a missing fill property for a stroke element, undeclared font families, and redundant color variables that should be removed for better maintainability.
| height: 900px; | ||
| position: relative; | ||
| background: #dbe6ef url("../../../images/quick-config-bg.png") center/cover no-repeat; | ||
| background: #dbe6ef url("../images/quick-config-bg.png") center/cover no-repeat; |
There was a problem hiding this comment.
The relative path for the background image has been changed to ../images/, but other assets in this file (e.g., sprite-review.png on line 74) still use ../../../images/. This inconsistency suggests the path might be broken. Additionally, if this asset is intended to be shared with community-login.pen, the pathing should be verified as they currently point to different directory levels.
| "fill": { | ||
| "type": "image", | ||
| "enabled": true, | ||
| "url": "../../images/Login:background:pc.png", |
There was a problem hiding this comment.
The filename Login:background:pc.png contains colons, which are reserved characters on Windows file systems. This will cause issues for users on Windows when cloning the repository or running builds. Consider renaming the file to use a safe separator like a hyphen or underscore.
"url": "../../images/Login-background-pc.png",
| "stroke": { | ||
| "align": "inside", | ||
| "thickness": 1 | ||
| }, |
There was a problem hiding this comment.
The stroke object for the logo name container is missing a fill property. Without a color definition, the stroke will not be rendered. If a border is intended, please add a fill color (e.g., "fill": "$r:grey500"); otherwise, remove the stroke property if it's not needed.
| "id": "cgp06", | ||
| "fill": "$r:grey1000", | ||
| "content": "登录", | ||
| "fontFamily": "Inter", |
There was a problem hiding this comment.
The font family Inter is used here (and HarmonyOS Sans is used on line 155), but neither is declared in the fonts array at the end of the file. To ensure consistent rendering across different devices, all custom fonts should be explicitly listed in the fonts section with their source URLs.
| "variables": { | ||
| "grey1000": { | ||
| "type": "color", | ||
| "value": [ | ||
| { | ||
| "value": "#24292f", | ||
| "theme": { | ||
| "Accent": "Default" | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "grey100": { | ||
| "type": "color", | ||
| "value": [ | ||
| { | ||
| "value": "#ffffff", | ||
| "theme": { | ||
| "Accent": "Default" | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "turquoise500": { | ||
| "type": "color", | ||
| "value": [ | ||
| { | ||
| "value": "#0bc0cf", | ||
| "theme": { | ||
| "Accent": "Default" | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "grey800": { | ||
| "type": "color", | ||
| "value": [ | ||
| { | ||
| "value": "#6e7781", | ||
| "theme": { | ||
| "Accent": "Default" | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "blue400": { | ||
| "type": "color", | ||
| "value": [ | ||
| { | ||
| "value": "#4db2fd", | ||
| "theme": { | ||
| "Accent": "Default" | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "code-black": { | ||
| "type": "color", | ||
| "value": [ | ||
| { | ||
| "value": "#000000", | ||
| "theme": { | ||
| "Accent": "Default" | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "code-red": { | ||
| "type": "color", | ||
| "value": [ | ||
| { | ||
| "value": "#db1c2b", | ||
| "theme": { | ||
| "Accent": "Default" | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "green600": { | ||
| "type": "color", | ||
| "value": [ | ||
| { | ||
| "value": "#3ca80c", | ||
| "theme": { | ||
| "Accent": "Default" | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "grey400": { | ||
| "type": "color", | ||
| "value": [ | ||
| { | ||
| "value": "#eaeff3", | ||
| "theme": { | ||
| "Accent": "Default" | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "grey500": { | ||
| "type": "color", | ||
| "value": [ | ||
| { | ||
| "value": "#d9dfe5", | ||
| "theme": { | ||
| "Accent": "Default" | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "red500": { | ||
| "type": "color", | ||
| "value": [ | ||
| { | ||
| "value": "#ef4149", | ||
| "theme": { | ||
| "Accent": "Default" | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "yellow500": { | ||
| "type": "color", | ||
| "value": [ | ||
| { | ||
| "value": "#FAA135", | ||
| "theme": { | ||
| "Accent": "Default" | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| }, |
There was a problem hiding this comment.
This variables block defines numerous colors that seem to be redundant. The components in this file reference variables using the $r: prefix (e.g., "$r:grey100"), which refers to the library imported on line 415. If these variables are already defined in the imported library, this local block should be removed to improve maintainability and reduce file size.
There was a problem hiding this comment.
Review Summary
This PR adds the community login page design (PC + app layouts), new background images, and updates the icon font files. The design implementation looks complete. A few issues worth addressing:
Portability issue with image filenames
Login:background:app.png and Login:background:pc.png use colons in their names. Colons are illegal path characters on Windows (NTFS) and can cause unexpected behavior on macOS (HFS+). These files should be renamed using a safe separator such as - or _ (e.g., Login-background-app.png), with the references in both community-login.pen and the component library updated accordingly.
Duplicate Login/background/pc component in the library
builder-component.lib.pen now contains two top-level frames both named Login/background/pc (different IDs uQ2UJ and wJo2Q) with near-identical structure — same dimensions (1440×1016), same fill, same nested path geometry with blur effects (~1,200 lines each). If one is a stale draft, the unused copy should be removed to avoid inflating the already-large library file.
Mislabeled text nodes
Three text nodes across the login components are named "Github Sign In Text" but their content is "使用微信登录" (WeChat login) and "立即登录" (Log in now). These appear to be copy-paste artifacts from a GitHub button variant. The names should match the actual content/purpose of each button.
Inconsistent component naming conventions
The login-related components in this PR use three distinct styles within the same feature namespace:
Login/...(PascalCase prefix) — page-level frameslogin/...(lowercase) — library sub-componentslogin input box-...(flat lowercase with dashes) — new input box components
A consistent convention across the feature would improve navigability.
Minor: community-login.pen is missing a trailing newline. All other .pen files in the repo end with one.
| "layout": "vertical", | ||
| "gap": 10, | ||
| "padding": [ | ||
| 208, |
There was a problem hiding this comment.
Colons in image filenames (Login:background:pc.png, Login:background:app.png) are illegal on Windows NTFS and can cause issues on macOS HFS+. Consider renaming to use - or _ as the separator (e.g., Login-background-pc.png) and updating all references.
| "url": "../../images/xbuilder-icons-02.ttf" | ||
| } | ||
| ] | ||
| } No newline at end of file |
There was a problem hiding this comment.
File is missing a trailing newline. All other .pen files in this repository end with one.
| height: 900px; | ||
| position: relative; | ||
| background: #dbe6ef url("../../../images/quick-config-bg.png") center/cover no-repeat; | ||
| background: #dbe6ef url("../images/quick-config-bg.png") center/cover no-repeat; |
There was a problem hiding this comment.
Good fix. Note that line 74 in this same file still references ../../../images/sprite-review.png with a three-level-up path — worth verifying that path is also correct for the file's current location.
Summary