Repository navigation
feat(native): Swift Icon draws shared icon names as SF Symbols - #8302
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
PR Analysis Report
|
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval is on hold for an engineering-owner decision: the draft component:swift/Icon record's FR2 (the Dynamic Type multiplier), FR4 (the color map), FR5 (the two initializers) and PR1 (symbols only), and import Astryx re-exporting AstryxNative, are not yet covered by any current spec. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or says which parts to change or drop. Separately, spec:AST-093 still needs two things:
- Attach the simulator or device captures of every state (each size and color in light and dark, inside an owner's slot, labeled, and at the largest text size); the record and the description say they exist, but none is linked here.
- Make
pnpm check:knowledgevalidatepackages/native/apple/Sources/Astryx/Icon/Icon.spec.md; its discovery skips native records today, so the record's frontmatter, id andplatform_ofgo unchecked.
[Automated review]
| | Behavior | Draws the SF Symbol the theme or the platform maps the name to, in a square box at core's icon size grown with Dynamic Type, in core's icon color token or the surrounding foreground. A name nothing maps draws nothing. | | ||
| | End-user impact | A person sees the platform's own symbol for each shared icon, sized and colored as on the web, and VoiceOver reads only icons that carry a label. | | ||
| | Builder impact | An app passes a shared name (`.search`) or a name it maps in its theme (`"acme:rocket"`). An SVG component, which the web also accepts, has no Swift form. | | ||
| | Compatibility/readiness | Additive: a new view and a new generated `IconName` type. Draft; built and tested on macOS and the iOS Simulator; device captures are attached to the change. | |
There was a problem hiding this comment.
This row says device captures are attached to the change, but no capture or link is on the pull request. spec:AST-093 FR5 needs them attached (each size and color in light and dark, inside an owner's slot, labeled, and at the largest text size), or each uncaptured claim reported as not measured.
b764c72 to
a2084db
Compare
|
/approve-spec a2084db |
There was a problem hiding this comment.
This is ready for an engineering owner's approval once the queued checks finish: the /approve-spec on this commit settles the draft component:swift/Icon requirements that were on hold, the simulator captures are attached, and the Swift tests pass on macOS and the iOS Simulator. None of the following blocks approval:
- OQ1 asks whether a theme can map a name to an image the app ships, which
spec:AST-082FR17 already allows; asking how Swift carries such an asset would match it. - The record's
schema_version: 5andtemplate_version: 8come from #8288, so landing after it keeps the record's schema onmain.
[Automated review]
|
|
||
| ## Open questions | ||
|
|
||
| - **OQ1 — Can a theme map a name to an image the app ships?** (`human-api`) |
There was a problem hiding this comment.
Optional: spec:AST-082 FR17 already lets native theme configuration name an image the app ships ({assetName: 'pixel.close'}), so whether a theme may map a name to an app image is decided. What stays open is how Swift carries one, since Theme(icons:) holds SF Symbol names only; asking that would keep OQ1 from reading as an open question about FR17 itself.
| nonisolated static func symbol(for name: IconName, themeIcons: [String: String]) -> String? { | ||
| themeIcons[name.rawValue] ?? astryxDefaultIcons[name.rawValue] |
There was a problem hiding this comment.
Optional: this repeats AstryxStyle.icon(_:) (Theme.swift:107-111), which Banner and Button already use and which stops a debug build on an unknown name. Drawing from astryx.icon(props.icon.rawValue) keeps one lookup, so asset icons or any later resolution change land once.
| | Behavior | Draws the SF Symbol the theme or the platform maps the name to, in a square box at core's icon size grown with Dynamic Type, in core's icon color token or the surrounding foreground. A name nothing maps draws nothing. | | ||
| | End-user impact | A person sees the platform's own symbol for each shared icon, sized and colored as on the web, and VoiceOver reads only icons that carry a label. | | ||
| | Builder impact | An app passes a shared name (`.search`) or a name it maps in its theme (`"acme:rocket"`). An SVG component, which the web also accepts, has no Swift form. | | ||
| | Compatibility/readiness | Additive: a new view and a new generated `IconName` type. Draft; built and tested on macOS and the iOS Simulator; device captures are attached to the change. | |
There was a problem hiding this comment.
Nit: the attached captures come from the iPhone simulator, which spec:AST-093 FR5 accepts; 'simulator captures' would match them.
User need
An iOS screen built with Astryx needs icons: in list rows, in place menus, in settings rows, beside a status. Today there is no Swift
Icon, so an app either draws its ownImage(systemName:)with guessed sizes and colors, or has no way to use Astryx's shared icon names at all. The person using the app gets icons that don't match the theme's colors, don't grow with their text size, and may be read aloud by VoiceOver when they are only decoration.Problem and solution fit
Iconview. Draws a shared icon name as the platform's own SF Symbol, in a square box at core's icon size (12, 16, 20, 24 pt) grown with Dynamic Type, in core's icon color token. Decorative unless it has alabel, as core'sIconis.iconprop isIconName | ComponentType<SVGProps>; the generator gains aniconslot kind (AST-082 FR4, FR17), soIconProps.iconis a generatedIconNameinstead of being dropped as web-only.IconNameis string-backed with every built-in name as a static member (.search,.close), so a theme-registered"acme:rocket"is valid too. The same slot kind is what Button's and Badge's icon props need next.Current specification
spec:AST-082FR4 (typed slots), FR7 (base props), FR17 (shared icon names, platform symbols).spec:AST-093FR1–FR5.component:swift/Iconatpackages/native/apple/Sources/Astryx/Icon/Icon.spec.md(draft,platform_of: component:Icon).Public delta and caller burden
Icon(Astryx), and generatedIconProps,IconColor,IconSize,IconName(AstryxNative, re-exported byAstryx).Theme(icons:)already maps a name to an SF Symbol; it now also registers a<library>:<name>name.Design and alternatives
Image(systemName:)(AST-093 FR2), no custom drawing.resizable().scaledToFit()), so a size means the same box as on the web.sizewins (component size cascade). Avatar and Item are the first owners.Evidence
Capture set: iPhone 16 simulator, iOS 18.5, Xcode 16.4, built from this PR's head (
cdf32a5d) by a scratch capture app. Each capture shows every size, every color,inheritunder a pink and a blue foreground, all 29 built-in names, labeled status icons beside text, and the accent in three themes.Not measured: an Icon inside an owner's icon slot. No Swift component gives its icon slot a size yet (Avatar and Item do not, as in core), so that state has no capture;
IconTests.swiftchecks the precedence by rendering with the owner's size set.check:knowledgedoes not scan native component specs yet; this record passes the checker in #8288, which does.Scope
Writing rubric results
Testing
vitest run packages/native: contract tests, including the new Icon contract andIconNamegeneration, pass; the committed Swift binding matches the contracts.swiftc -typecheckofAstryxNative,Astryx, the tests, and the templates passes; an immediate-mode render checks 20/24/12 pt boxes, the owner's size, growth at AX5, a hidden icon, aninheriticon taking the surrounding foreground, and that all 29 built-in symbols exist.Every changed file is under
packages/native/.