feat(pages): redesign hero with two-column layout and three install channels#408
feat(pages): redesign hero with two-column layout and three install channels#408hezheng-max wants to merge 1 commit into
Conversation
|
🔍 OpenCodeReview found 5 issue(s) in this PR.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| 'hero.terminal': '终端', | ||
| 'hero.copied': '已复制', | ||
| 'hero.copyFailed': '复制失败', | ||
| 'hero.installNpm': 'Npm 下载', |
There was a problem hiding this comment.
"Npm" should be "npm" (all lowercase). The official npm branding always uses lowercase "npm". This is also inconsistent with the command shown below it (npm i -g ...) which correctly uses lowercase.
Suggestion:
| 'hero.installNpm': 'Npm 下载', | |
| 'hero.installNpm': 'npm 下载', |
| 'hero.terminal': 'Terminal', | ||
| 'hero.copied': 'Copied!', | ||
| 'hero.copyFailed': 'Copy failed', | ||
| 'hero.installNpm': 'Npm Download', |
There was a problem hiding this comment.
The conventional spelling is "npm" (all lowercase), not "Npm". This is consistent across the official npm documentation and the command itself (npm i -g ...). The same issue exists in the ja.ts and zh.ts translations.
Suggestion:
| 'hero.installNpm': 'Npm Download', | |
| 'hero.installNpm': 'npm Download', |
| {ch.key === 'unix' ? (() => { | ||
| const parts = t(ch.labelKey).split('/'); | ||
| const macLabel = (parts[0] ?? '').trim(); | ||
| const linuxLabel = (parts[1] ?? '').trim(); |
There was a problem hiding this comment.
Fragile i18n parsing logic: Splitting the translated string on '/' to separate macOS and Linux labels is brittle. If any future translation uses a different separator, omits the slash, or contains additional slashes (e.g., in descriptive text), the UI will render incorrectly. Consider using separate i18n keys instead (e.g., hero.installMac and hero.installLinux) to avoid relying on string parsing of translated content.
| { key: 'unix', labelKey: 'hero.installUnix', cmd: 'curl -fsSL https://raw.githubusercontent.com/alibaba/open-code-review/main/install.sh | sh', icons: [appleIcon, linuxIcon] }, | ||
| { key: 'windows', labelKey: 'hero.installWindows', cmd: 'irm https://raw.githubusercontent.com/alibaba/open-code-review/main/install.ps1 | iex', icons: [windowsIcon] }, |
There was a problem hiding this comment.
Hardcoded URLs: These installation script URLs (raw.githubusercontent.com/...) are business-critical resource paths embedded directly in component code. If the repository path changes or environment-specific URLs are needed, this requires a code change. Consider extracting these into a configuration file or constants module for easier maintenance.
| width: '100vw', | ||
| marginLeft: 'calc(-50vw + 50%)', | ||
| height: isMobile ? 850 : isTablet ? 830 : 990, | ||
| height: isMobile ? 1000 : isTablet ? 900 : 860, |
There was a problem hiding this comment.
Nested ternary expression: This nested ternary (isMobile ? ... : isTablet ? ... : ...) violates the code quality rule prohibiting nested ternaries. The project already has a pattern for avoiding this — see useResponsiveStyle.ts which uses if/else-if/else blocks. Consider creating a similar helper or using if/else logic to determine the height value.
…hannels - Change hero from centered stack to desktop two-column layout (text left, terminal right) - Replace top install badge with three install channels: npm, macOS/Linux, Windows PowerShell - Add brand icons (npm/apple/linux/windows) with #ffffff 90% opacity - Interleave apple/linux icons within the macOS / Linux label - Add i18n keys hero.installNpm/installUnix/installWindows for en/zh/ja - Adjust hero title to three lines (en) and content paddingTop
9efd2b4 to
9a680ec
Compare
1 participant
Overview
Redesign the landing page Hero section into a desktop two-column layout and upgrade the single top install badge into three install channels.
Changes
npm i -g @alibaba-group/open-code-reviewcurl -fsSL .../install.sh | shirm .../install.ps1 | iexhero.installNpm/hero.installUnix/hero.installWindowsfor en / zh / jaRobustness
?? ''fallback + graceful degradation) to avoid a crash if a translation lacks '/'