feat(pages): add MacOS, Linux, and Windows install channels - #904
Conversation
…ection Add platform-specific install options (curl|sh for MacOS/Linux, irm|iex for Windows) to the 'More' dropdown in the homepage hero. - Add apple.svg, linux.svg, windows.svg icons - Add three new secondary install channels with OS-specific commands - Reorder MacPorts to the bottom of the dropdown - Add i18n keys for all four locales - Fix dropdown clipping by removing overflow:hidden on hero section - Fix dropdown alignment (left:0 instead of right:0)
|
🔍 OpenCodeReview found 3 issue(s) in this PR.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| { key: 'macos', labelKey: 'hero.installMacOS', cmd: 'curl -fsSL https://open-codereview.ai/install.sh | sh', icons: [appleIcon], primary: false }, | ||
| { key: 'linux', labelKey: 'hero.installLinux', cmd: 'curl -fsSL https://open-codereview.ai/install.sh | sh', icons: [linuxIcon], primary: false }, | ||
| { key: 'windows', labelKey: 'hero.installWindows', cmd: 'irm https://open-codereview.ai/install.ps1 | iex', icons: [windowsIcon], primary: false }, |
There was a problem hiding this comment.
Hardcoded URLs: The installation URLs (https://open-codereview.ai/install.sh and https://open-codereview.ai/install.ps1) are hardcoded business-related strings that should be extracted to a constants or configuration file. This violates the coding standard that prohibits hardcoded URL paths.
Additionally, these URLs appear in multiple places throughout the codebase (documentation files, README files), creating maintenance burden. If the domain or path changes, you would need to update many files.
Recommendation: Create a constants file (e.g., src/constants/install.ts) to centralize these URLs:
export const INSTALL_URLS = {
SHELL_SCRIPT: 'https://open-codereview.ai/install.sh',
POWERSHELL_SCRIPT: 'https://open-codereview.ai/install.ps1',
};Then reference them in the install channels:
{ key: 'macos', ..., cmd: `curl -fsSL ${INSTALL_URLS.SHELL_SCRIPT} | sh`, ... }
{ key: 'windows', ..., cmd: `irm ${INSTALL_URLS.POWERSHELL_SCRIPT} | iex`, ... }| 'hero.installNpm': 'npm', | ||
| 'hero.installBrew': 'Homebrew', | ||
| 'hero.installMacPorts': 'MacPorts', | ||
| 'hero.installMacOS': 'MacOS', |
There was a problem hiding this comment.
Minor branding nit: Apple's official styling is "macOS" (lowercase 'm', uppercase 'OS'). Consider using "macOS" for consistency with Apple's branding convention.
Suggestion:
| 'hero.installMacOS': 'MacOS', | |
| 'hero.installMacOS': 'macOS', |
| 'hero.installNpm': 'npm', | ||
| 'hero.installBrew': 'Homebrew', | ||
| 'hero.installMacPorts': 'MacPorts', | ||
| 'hero.installMacOS': 'MacOS', |
There was a problem hiding this comment.
Apple's official branding for the operating system is "macOS" (lowercase 'm', uppercase 'OS'). Using "MacOS" is inconsistent with Apple's standard naming convention. This applies across all four language files (en, ja, ru, zh).
Suggestion:
| 'hero.installMacOS': 'MacOS', | |
| 'hero.installMacOS': 'macOS', |
2 participants
Summary
curl -fsSL https://open-codereview.ai/install.sh | sh(Apple icon)curl -fsSL https://open-codereview.ai/install.sh | sh(Tux icon)irm https://open-codereview.ai/install.ps1 | iex(Windows icon)overflow: hiddenTest plan
npm run typecheckpassesnpm run buildsucceeds