Repository navigation
feat(native): Swift Button takes core's props; every view reads typed tokens - #8310
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewNo new or modified components detected. Bundle Size SummaryNo component packages changed. Accessibility AuditStatus: The accessibility audit did not run on this pull request. It runs after merges to Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested: moving Swift Button and TextInput onto their generated props follows the native guidelines, but two TextInput behaviors need fixing first.
- A read-only
type: .passwordfield draws its value as plain, selectable text that VoiceOver reads; keep it masked, as core's read-only password input is. - The clear, status-tip and label-info buttons are hit only on their 12-point glyph; give each at least a 44 × 44 point hit area, as
Buttonalready does.
The Swift tests have not run on this head because CI only runs for pull requests into main, so approval also needs a green test-native-swift run once the branch targets main.
[Automated review]
| if isReadOnly { | ||
| // Read-only: the value at full strength, selectable, never editable. | ||
| SwiftUI.Text(props.value) | ||
| .textSelection(.enabled) | ||
| .frame(maxWidth: .infinity, alignment: .leading) | ||
| .accessibilityValue(SwiftUI.Text(props.value)) |
There was a problem hiding this comment.
A read-only type: .password field reaches this branch before type is read, so the password is drawn in clear text, can be selected and copied, and VoiceOver speaks it through accessibilityValue. Core keeps type="password" on its read-only input (AST-082 FR3: a prop is never honored only in part). Draw a masked value here for .password, without text selection or the raw accessibility value.
| if showsClear { | ||
| SwiftUI.Button { | ||
| props.onChange?("") | ||
| isFocused = true | ||
| } label: { | ||
| icon("close") | ||
| } | ||
| .buttonStyle(.plain) |
There was a problem hiding this comment.
This .plain button's hit area is the 12-point glyph from icon(_:); the 44-point row does not enlarge it. AST-082 FR11 asks for the platform's touch minimum: add .frame(minWidth: 44, minHeight: 44) and .contentShape(Rectangle()) here and in tipButton, as Button does.
a7ed31c to
1781ce0
Compare
eefd3c1 to
d391bea
Compare
There was a problem hiding this comment.
Changes requested: moving Swift Button and TextInput onto their generated props follows the native guidelines, but two TextInput behaviors need fixing first.
- A read-only
type: .passwordfield draws its value as plain, selectable text that VoiceOver reads; keep it masked, as core's read-only password input is. - The clear, status-tip and label-info buttons are hit only on their 12-point glyph; give each at least a 44 × 44 point hit area, as
Buttonalready does.
Approval also needs a green test-native-swift run, which CI starts only once the branch targets main, and a TextInput case in the two-theme test.
[Automated review]
| | A2 | A compile test builds every view from its generated `<Name>Props` with every prop set, plus each content slot's generated node type, and through the labeled initializer with a builder for each content slot, on iOS and on macOS. A source check finds no parameter that a view accepts and ignores; Switch's `size` exists on macOS only. | | ||
| | A3 | A test builds a view through each initializer with the same arguments and finds every `<Name>Props` field the same in both, comparing callbacks by invoking them. | | ||
| | A4 | `AnyView` does not appear in the components module. An enabled Button with `href` is a `Link` in the view tree and reads as a link to VoiceOver; a disabled Button with `href` reads as a dimmed button; an Avatar with `href` draws the same as without it. A disabled Item with `href` reads as a dimmed link and does not open. | | ||
| | A4 | `AnyView` does not appear in the components module. An enabled Button with `href` is a `Link` in the view tree and reads as a link to VoiceOver; a disabled Button with `href` reads as a dimmed button; an Avatar with `href` draws the same as without it. A disabled Item with `href` reads as a dimmed link and does not open. A loading Button with `href` stays a `Link`, keeps focus, reads as busy, and ignores a tap until loading ends. | |
There was a problem hiding this comment.
This check drops A4's own exception: as written, an interruptible loading Button with href fails it, although A4 (and family:buttons FR5) let that button accept the press. Suggest "…reads as busy, and ignores a tap until loading ends unless the Button is interruptible."
| - `TextInput` takes a `Binding` where core has `value` and `onChange`. | ||
| the same across web and native. Every view with a core contract takes its | ||
| generated `<Name>Props` (AST-093 FR5) and has a labeled initializer that | ||
| builds the same props; `Banner` has no core contract yet, so its parameters |
There was a problem hiding this comment.
Core does ship Banner (packages/core/src/Banner/Banner.tsx, Banner.spec.md); what it lacks is a native contract, since packages/native/src/components/ has no Banner.ts. Suggest "Banner has no native contract yet, so its parameters are hand-written."
1781ce0 to
8e5470b
Compare
d391bea to
c0aa70c
Compare
8e5470b to
a32df9f
Compare
c0aa70c to
414356e
Compare
There was a problem hiding this comment.
Changes requested: moving Swift Button and TextInput onto their generated props follows the native guidelines, but two TextInput behaviors and one spec check still need fixing.
- A read-only
type: .passwordfield draws its value as plain, selectable text that VoiceOver reads; keep it masked, as core's read-only password input is. - The clear, status-tip and label-info buttons are hit only on their 12-point glyph; give each at least a 44 × 44 point hit area, as
Buttonalready does. - The new A4 check row in
component-api.mdsays a loading link ignores a tap until loading ends; add "unless the Button is interruptible", as A4's own text says.
Approval also needs a green test-native-swift run, which CI starts once the branch targets main, and a TextInput case in the two-theme test.
[Automated review]
a32df9f to
9962508
Compare
414356e to
acd0d8f
Compare
9962508 to
e211ae1
Compare
acd0d8f to
0abb1aa
Compare
There was a problem hiding this comment.
Changes requested: moving Swift Button and TextInput onto their generated props follows the native guidelines, but two TextInput behaviors and one spec check row need fixing, and approval also needs a green Swift test run (CI's test-native-swift starts only for pull requests into main), captures, and TextInput and Banner two-theme checks.
- A read-only
type: .passwordfield draws its value as plain, selectable text that VoiceOver reads; keep it masked, as core's read-only password input does. - The clear and tip buttons hit-test only their 12-point glyph; give them a 44-point frame and content shape, as
Buttonhas. - The new A4 check row in
component-api.mdshould keep “unless the Button is interruptible”, as the A4 text above it does.
[Automated review]
e211ae1 to
4e533ce
Compare
0abb1aa to
5bf043c
Compare
There was a problem hiding this comment.
Changes requested: moving Swift Button and TextInput onto their generated props follows the native guidelines, but two TextInput behaviors, one spec check row and one README sentence need fixing. The TextInput fixes belong in #8308 and reach this branch on rebase; approval also needs a green test-native-swift run, which CI starts only once the branch targets main, plus the TextInput and Banner two-theme checks and a Banner capture that the native guidelines ask for.
- A read-only
type: .passwordfield draws its value as plain, selectable text that VoiceOver reads; keep it masked, as core's read-only password input does. - The clear and tip buttons take taps only on their 12-point glyph; give them a 44-point frame and content shape, as
Buttonhas. - The new A4 check row in
component-api.mdshould keep "unless the Button is interruptible", as the A4 text above it does. - The README should say Banner has no generated native contract yet, since core's Banner has a current spec.
[Automated review]
…me's status roles
… view structure and its focus
|
/approve-spec 54df851 |
…ent current state
…nvironment count Banner and Icon; Banner two-theme test
…ponent API points to it
…on; the Swift Button record names its guideline pages and theme values
…he package excludes Button.spec.md
…and its focus; theme environment counts eight components
…fusing it; the link focus gap is recorded
… one structure and moves focus between them
|
/approve-spec 18f9ea9 |
There was a problem hiding this comment.
Approval recommended: the Swift Button now takes core's generated ButtonProps, matches core's Button contract and the draft component:swift/Button record an engineering owner approved at this head, and its tests pass on macOS and the iOS Simulator. This branch carries the TextInput commits of #8308, so it should land after #8308; rebasing onto main then creates a new head that needs a fresh /approve-spec with its full SHA from an engineering owner (the author may give it as one). A relative href such as /settings becomes a link that opens nothing, which no spec defines yet, so a follow-up spec line for native relative destinations would help.
[Automated review]
What changes for people
An iOS developer gets core's whole Button:
width: .fillfor a full-width button;href, which makes the button a real link the system opens and VoiceOver reads as a link;tooltipas a pointer tip and a VoiceOver hint;elevationdrawn with the theme's shadow;isInterruptible, so a loading button can still be pressed to interrupt it.With this, every Swift view that has a core contract takes its generated props, and every view reads the theme through typed tokens. The Login template's Sign in button fills its card through
width: .fill, as on the web.What this does
Buttontakes the generatedButtonProps(AST-093 FR5; Swift component API A2, A3, A4): a labeled initializer with the press last as a trailing closure (onClick), and the props initializer.hrefon an enabled button draws a SwiftUILinkstyled with the Astryx button style; a disabled one stays a disabled button.openURL, soonClickruns on every activation: tap, VoiceOver, or keyboard.tooltipis SwiftUI's.help: a hover tip with a pointer and a hint VoiceOver reads; a tap belongs to the button, as with core'stouchTrigger: auto.elevationdraws the theme's shadow;widthiscontentorfill;sizeandvariantcome from typed tokens.AstryxNodeViewalso draws Button and TextInput nodes.astryx.color("…"),size,space,fontSize,font(size:…)) are no longer public.component:swift/Button(packages/native/apple/Sources/Astryx/Button/Button.spec.md, draft), per Cindy's decision that the loading rule belongs on the Swift Button spec. FR4 holds the loading-link rule: a loading Button withhrefstays aLink, keeps focus, shows its spinner, reads as busy, and does not open its destination until loading ends, unless the Button is interruptible, in which case each activation opens it. It never swaps to a button while busy (core Button FR9 and FR11). FR1–FR3 record the initializers, the loading presentation, and enabled or disabledhref; FR5 says a script URL (javascript:,vbscript:) or an HTML document in adata:URL never opens, and the Button stays an enabled button. FR6: every disabled Button keeps its keyboard focus stop and shows its tooltip on focus, draws dimmed, reads as a dimmed button, and never runsonClick(core Button FR10, buttons family FR3); the focus stop is not measured until an app-hosted test can observe it. Each has a verification row. The record names the Apple guideline pages it follows (Buttons, Progress indicators, Accessibility) and the theme values Button reads. The shared component API record keeps no loading rule, only a pointer to Button FR4, and states no general rule. Theme environment's Current state describes theAstryxproduct.Landing order: after #8307 (Card) and #8308 (TextInput); #8305 (Button
width) is onmain. This branch carries #8308's commit so it builds; it is rebased ontomainas each lands, leaving only its own change (8 files).tooltipshows through the shared Astryx tooltip on hover and keyboard focus, its words the VoiceOver hint; no view uses.help.Compatibility: breaking for canary callers.
action:,fillsWidth:,icon:, and the hand-writtenAstryxButtonVariantandAstryxButtonSizegive way to the generatedButtonProps(onClick:,width:,ButtonVariant,ButtonSize); the record states it.Writing rubric results
component:swift/Buttonand the component API pointer describe the system as it is.Captures (AST-093 FR5)
Capture set: iPhone 16 simulator, iOS 18.5, Xcode 16.4, built from this PR's head (
6cad853). Each capture shows each variant and size, loading and disabled, full width,hrefas a link, loading and disabled links, each elevation, and two themes.Accessibility tree: not measured in these captures; the Swift tests check names, traits and values.
Banner (statuses; two themes):
Testing
hrefmakes an enabled button a link and a disabled one a button;javascript:,vbscript:, anddata:text/htmldestinations never open and leave an enabled button, whiledata:imageandmailto:still open;filltakes the whole width, in pixels;swiftc -typecheckof both modules, the tests, and the templates passes; the Swift tests run in CI on the iOS Simulator.Every changed file is under
packages/native/.