refactor: unify platform icons across homepage, APIStatus, and PlatformSvg#1082
Merged
Conversation
✅ Deploy Preview for lynx-doc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (8)
📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughCentralizes platform icon rendering by moving PlatformSvg to a lookup-table-based icon URL mapper, then replaces per-platform icon components and CSS with a makeIcon factory and a PlatformIconWrapper using a generic ChangesPlatform Icon System Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying lynx-website-next with
|
| Latest commit: |
baa075a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6b8ebd3e.lynx-website-next.pages.dev |
| Branch Preview URL: | https://feat-unify-platform-icons.lynx-website-next.pages.dev |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors platform icon rendering so homepage feature cards, API status surfaces, and the platform navigation components all route through the shared <PlatformSvg /> implementation backed by the compat-table icon set, reducing duplication and keeping icons consistent when new platforms are added.
Changes:
- Centralized platform icon asset mapping in
PlatformIcon.tsx(compat-table icons +mapPlatformNameToIconName). - Replaced homepage per-platform SVGR icon components with thin
<PlatformSvg />wrappers and a shared.platform-iconstyle. - Replaced API status inline JSX SVGs /
createMaskIconwith amakeIcon(platformName)factory that delegates to<PlatformSvg />.
Reviewed changes
Copilot reviewed 4 out of 11 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/components/platform-navigation/PlatformIcon.tsx | Centralizes icon URL mapping and reuses compat-table platform→icon-name mapping. |
| src/components/home-comps/features/index.module.less | Replaces per-icon fill/size overrides with a single .platform-icon rule. |
| src/components/home-comps/features/icon.tsx | Swaps SVGR-per-icon components for <PlatformSvg /> wrappers. |
| src/components/api-status/constants.tsx | Removes inline SVG icon components in favor of <PlatformSvg />-based factory icons. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
48
to
56
| export const PlatformSvg = ({ | ||
| platformName, | ||
| className, | ||
| key, | ||
| }: { | ||
| // Accept extra string ids ('clay', 'macos', 'macos-arm64', 'macos-intel', | ||
| // 'windows') alongside the canonical PlatformName so PlatformTabs and | ||
| // ChoiceTabs can use the homepage-aligned icon set without TS friction. | ||
| platformName: PlatformName | string; | ||
| className?: string; | ||
| key?: string; | ||
| }) => { |
Comment on lines
+70
to
+85
| const toPlatformName = (platform: string): PlatformName => { | ||
| switch (platform) { | ||
| case 'ios': | ||
| case 'ios-simulator': | ||
| case 'macos': | ||
| case 'macos-arm64': | ||
| case 'macos-intel': | ||
| return 'ios'; | ||
| case 'android': | ||
| return 'android'; | ||
| case 'web': | ||
| return 'web_lynx'; | ||
| default: | ||
| return 'web_lynx'; | ||
| } | ||
| }; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/platform-navigation/PlatformIcon.tsx (1)
70-106:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftFix macOS icon rendering in
PlatformIcon(toPlatformNameincorrectly mapsmacos*→ios)
src/components/platform-navigation/PlatformIcon.tsxcurrently normalizesmacos/macos-arm64/macos-intelto'ios', soPlatformTabs(which passesplatforms={[option.id]}whereoption.idcan bemacos*) ends up showing the Apple icon.PlatformSvg/toIconUrlalready has explicit handling to resolvemacos*to the macOS SVG, so this normalization layer breaks consistency.Update
toPlatformNamesomacos*resolves to a value thattoIconUrlcan render as macOS (instead of'ios'). If you consider passingplatformthrough directly, also handleios-simulator(since the current fallback mapping would otherwise render the wrong icon).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/platform-navigation/PlatformIcon.tsx` around lines 70 - 106, toPlatformName currently maps macos, macos-arm64, macos-intel to 'ios', causing PlatformIcon -> PlatformSvg to render the wrong Apple iOS icon; update toPlatformName so macos* values map to a macOS-identifying value that toIconUrl/PlatformSvg recognize (e.g., return 'macos' for 'macos', 'macos-arm64', 'macos-intel'), and keep ios-simulator handled explicitly (either map 'ios-simulator' to 'ios-simulator' or to 'ios' depending on how toIconUrl expects it) so PlatformIcon/PlatformSvg render the correct macOS and iOS simulator icons.
🧹 Nitpick comments (1)
src/components/platform-navigation/PlatformIcon.tsx (1)
29-46: 💤 Low value
toIconUrl's explicitclay/web/windowsbranches are redundant.
mapPlatformNameToIconNamereturns its argument unchanged in thedefaultcase, soclay,web, andwindowsalready resolve to the correctICON_NAME_TO_URLentries via the fallback path. Only themacos/macos-arm64/macos-intelbranch is genuinely needed (none map to amacoskey in the table). The duplicated branches add a maintenance hazard if the table and the inline checks drift.♻️ Suggested simplification
const toIconUrl = (platformName: PlatformName | string): string => { - // Extra string ids used by tabs/badges that aren't in BCD.PlatformName. - if (platformName === 'clay') return ClayIcon; - if ( - platformName === 'macos' || - platformName === 'macos-arm64' || - platformName === 'macos-intel' - ) { - return MacosIcon; - } - if (platformName === 'web') return WebIcon; - if (platformName === 'windows') return WindowsIcon; - + // macOS variant ids aren't in BCD.PlatformName and don't map to a table key. + if ( + platformName === 'macos' || + platformName === 'macos-arm64' || + platformName === 'macos-intel' + ) { + return MacosIcon; + } // BCD-known platform names route through compat-table's mapping so we stay // consistent with the APITable headers (clay_macos → macos-text etc.). const iconName = mapPlatformNameToIconName(platformName as PlatformName); return ICON_NAME_TO_URL[iconName] ?? ClayIcon; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/platform-navigation/PlatformIcon.tsx` around lines 29 - 46, The toIconUrl function has redundant explicit branches for 'clay', 'web', and 'windows' because mapPlatformNameToIconName returns its input by default and ICON_NAME_TO_URL already covers those names; remove the explicit checks for 'clay', 'web', and 'windows' and let the iconName = mapPlatformNameToIconName(platformName as PlatformName) → ICON_NAME_TO_URL[iconName] ?? ClayIcon path handle them, while preserving the special-case branch that consolidates 'macos'/'macos-arm64'/'macos-intel' to return MacosIcon; keep the final fallback to ClayIcon intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/home-comps/features/index.module.less`:
- Around line 38-41: The Stylelint rule wants a blank line before the `color`
declaration in the `.platform-icon` block; update the `.platform-icon` rule in
index.module.less (the block containing `--icon-size` and `color`) by inserting
a single empty line between the `--icon-size: 18px;` custom property and the
`color: var(--home-blog-btn-color);` declaration so the
`declaration-empty-line-before` rule is satisfied.
---
Outside diff comments:
In `@src/components/platform-navigation/PlatformIcon.tsx`:
- Around line 70-106: toPlatformName currently maps macos, macos-arm64,
macos-intel to 'ios', causing PlatformIcon -> PlatformSvg to render the wrong
Apple iOS icon; update toPlatformName so macos* values map to a
macOS-identifying value that toIconUrl/PlatformSvg recognize (e.g., return
'macos' for 'macos', 'macos-arm64', 'macos-intel'), and keep ios-simulator
handled explicitly (either map 'ios-simulator' to 'ios-simulator' or to 'ios'
depending on how toIconUrl expects it) so PlatformIcon/PlatformSvg render the
correct macOS and iOS simulator icons.
---
Nitpick comments:
In `@src/components/platform-navigation/PlatformIcon.tsx`:
- Around line 29-46: The toIconUrl function has redundant explicit branches for
'clay', 'web', and 'windows' because mapPlatformNameToIconName returns its input
by default and ICON_NAME_TO_URL already covers those names; remove the explicit
checks for 'clay', 'web', and 'windows' and let the iconName =
mapPlatformNameToIconName(platformName as PlatformName) →
ICON_NAME_TO_URL[iconName] ?? ClayIcon path handle them, while preserving the
special-case branch that consolidates 'macos'/'macos-arm64'/'macos-intel' to
return MacosIcon; keep the final fallback to ClayIcon intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aed8d0e7-b4c5-4742-b9f4-4e6d11d6afa2
📒 Files selected for processing (4)
src/components/api-status/constants.tsxsrc/components/home-comps/features/icon.tsxsrc/components/home-comps/features/index.module.lesssrc/components/platform-navigation/PlatformIcon.tsx
…rmSvg Platform icons used to live in three places with three different rendering strategies: SVGR `?react` imports with `fill` overrides on the homepage, inline JSX SVGs with `currentColor` in api-status, and mask-image with currentColor in PlatformIcon. APITable already used mask-image from the compat-table icon set. Consolidate around the compat-table set as the canonical icon source and route every consumer through `<PlatformSvg />`: - `PlatformIcon.tsx`: drop the `@assets/home/*` imports, route through one `ICON_NAME_TO_URL` table backed by `compat-table/assets/icons/`. The mapping reuses compat-table's `mapPlatformNameToIconName` so the homepage and APITable stay in lockstep when new platforms are added. - `home-comps/features/icon.tsx`: replace per-platform SVGR imports with thin `<PlatformSvg />` wrappers and a shared `.platform-icon` size class. - `api-status/constants.tsx`: replace inline JSX SVGs and the `createMaskIcon` helper with a single `makeIcon(platformName)` factory that delegates to `<PlatformSvg />`. Visible change: the homepage Android icon switches from the half-dome face to the full Bugdroid body that APITable / APISummary already used, so the two surfaces match. Change-Id: I63a57690e19a7bedee6a6a8c0838a873668b73bd
All consumers route through PlatformSvg backed by the compat-table icon set after the previous commit, so the home/ duplicates have no references left. Removes home-icon-android.svg, home-icon-apple.svg, home-icon-apple-dark.svg, home-icon-web.svg, plus the harmony/windows/clay copies that lived in two places. Change-Id: I477373262facabc57ce574b3a0e6c309bb0f1b00
Replace the full Bugdroid body with the half-dome face that the homepage
used to ship, retained as a single fill-rule="evenodd" path so the eyes
become cutouts in the silhouette. Because every consumer renders the
icon through mask-image + currentColor, the result is fully monochrome
and inherits whatever color the parent sets, so callers can recolor it
programmatically per surface (e.g. style={{ color }} or a scoped CSS
rule) when needed.
Change-Id: I666a542e9aa8005e804127a1851291e9206665c6
Drop the uniform dark-grey wash on the homepage feature card and tint each platform icon with a hue that reads as native to the platform: zinc for Apple (iOS / macOS), emerald for Android, rose for HarmonyOS, orange for Web (HTML5), sky for Windows. All sit at Tailwind ~600 in light mode and ~400 in dark mode so saturation and tonal weight stay matched even though the hues span the wheel. Change-Id: Id6ca10297a8e6b38092ac4b5faff55d70f3b40f8
Huxpro
force-pushed
the
feat/unify-platform-icons
branch
from
ae72f12 to
fb6b699
Compare
Change-Id: I0273c96570f5ac0df26d29521fe26e7a4544e31e
This was referenced Jun 4, 2026
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Platform icons used to live in three places with three rendering strategies:
@assets/home/home-icon-*.svg?react+ per-iconfilloverridescreateMaskIconfor macOS / Windows@assets/home/*(mostly)mask-image+currentColorcompat-table/assets/icons/*mask-imagevia_icon.scssThis PR routes every surface through a single
<PlatformSvg />that reads from the compat-table icon set. New platforms now mean "add one SVG and one entry inICON_NAME_TO_URL" instead of touching three files.What changed
PlatformIcon.tsx: dropped the@assets/home/*imports; oneICON_NAME_TO_URLtable backed bycompat-table/assets/icons/. The mapping reuses compat-table'smapPlatformNameToIconNameso homepage and APITable stay in lockstep when platforms are added.home-comps/features/icon.tsx: per-platform SVGR imports replaced with thin<PlatformSvg />wrappers and a shared.platform-iconsize class.api-status/constants.tsx: inline JSX SVGs and thecreateMaskIconhelper replaced with onemakeIcon(platformName)factory that delegates to<PlatformSvg />.Visible change
The homepage Android icon switches from the half-dome face to the full Bugdroid body that APITable / APISummary already showed, so the two surfaces now match. iOS / HarmonyOS / Web / macOS / Windows render the same glyph on every surface.
Test plan
/— feature card platform row shows iOS, Android (Bugdroid), HarmonyOS, Web (HTML5), Desktop (macOS + Windows)<APITable />(e.g./guide/devtool) — column headers show the same glyphs/guide/start/integrate-with-existing-apps.html— platform tabs match/api-statusor any APISummary surface — platform badges matchcolorfor the maskConsolidate platform icon rendering into single canonical source
@assets/home/* SVG usage and per-platform CSS overrides; add .platform-icon sizing rule in home-comps/features/index.module.less