Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report🔎 Preview ·
|
| Package | Where | Signature | Usage | |
|---|---|---|---|---|
| + | core |
. · Types · interface FieldStatusProps |
message: string |
import type {FieldStatusProps} from '@astryxdesign/core'; const x: FieldStatusProps = {message: …} |
| + | core |
. · Types · interface FieldStatusProps |
ref?: React.Ref<HTMLDivElement> |
import type {FieldStatusProps} from '@astryxdesign/core'; const x: FieldStatusProps = {ref: …} |
| + | core |
. · Types · interface FieldStatusProps |
type: InputStatusType |
import type {FieldStatusProps} from '@astryxdesign/core'; const x: FieldStatusProps = {type: …} |
| + | core |
. · Types · interface FieldStatusProps |
variant?: FieldStatusVariant = 'attached' |
import type {FieldStatusProps} from '@astryxdesign/core'; const x: FieldStatusProps = {variant: …} |
| + | core |
. · Types · interface FieldStatusProps |
xstyle?: StyleXStyles |
import type {FieldStatusProps} from '@astryxdesign/core'; const x: FieldStatusProps = {xstyle: …} |
| + | core |
. · Types · interface FieldStatusProps |
inherits AriaAttributes, DOMAttributes, HTMLAttributes |
import type {FieldStatusProps} from '@astryxdesign/core' |
| + | core |
. · Types · interface SearchContext |
interface SearchContext<T extends SearchableItem = SearchableItem> |
import type {SearchContext} from '@astryxdesign/core' |
| + | core |
./Typeahead · Also exported from an earlier entry point · SearchContext |
SearchContext (.) |
import {SearchContext} from '@astryxdesign/core/Typeahead' |
| ~ | core |
. · Components · component ComplexSelector |
prop statusVariant?: FieldStatusVariant = 'attached' for input selectors; 'detached' for secondary and ghost selectors → prop statusVariant?: FieldStatusPlacement = 'attached' for input selectors; 'detached' for secondary and ghost selectors |
<ComplexSelector statusVariant="attached" /> |
| ~ | core |
. · Components · component DateInput |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<DateInput statusVariant="attached" /> |
| ~ | core |
. · Components · component DateRangeInput |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<DateRangeInput statusVariant="attached" /> |
| ~ | core |
. · Components · component Field |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<Field statusVariant="attached" /> |
| ~ | core |
. · Components · component FieldStatus |
component FieldStatus(props: FieldStatusProps) → component FieldStatus @deprecated |
<FieldStatus /> |
| ~ | core |
. · Components · component FieldStatus |
prop variant?: FieldStatusVariant = 'attached' → prop variant: FieldStatusVariant = 'attached' |
<FieldStatus variant="attached" /> |
| ~ | core |
. · Components · component FileInput |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<FileInput statusVariant="attached" /> |
| ~ | core |
. · Components · component MultiSelector |
prop statusVariant?: FieldStatusVariant = 'attached' for input selectors; 'detached' for secondary and ghost selectors → prop statusVariant?: FieldStatusPlacement = 'attached' for input selectors; 'detached' for secondary and ghost selectors |
<MultiSelector statusVariant="attached" /> |
| ~ | core |
. · Components · component NumberInput |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<NumberInput statusVariant="attached" /> |
| ~ | core |
. · Components · component PowerSearch |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<PowerSearch statusVariant="attached" /> |
| ~ | core |
. · Components · component Selector |
prop statusVariant?: FieldStatusVariant = 'attached' for input selectors; 'detached' for secondary/ghost selectors → prop statusVariant?: FieldStatusPlacement = 'attached' for input selectors; 'detached' for secondary/ghost selectors |
<Selector statusVariant="attached" /> |
| ~ | core |
. · Components · component TextArea |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<TextArea statusVariant="attached" /> |
| ~ | core |
. · Components · component TextInput |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<TextInput statusVariant="attached" /> |
| ~ | core |
. · Components · component TimeInput |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<TimeInput statusVariant="attached" /> |
| ~ | core |
. · Components · component Tokenizer |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<Tokenizer statusVariant="attached" /> |
| ~ | core |
. · Components · component Typeahead |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusPlacement = 'attached' |
<Typeahead statusVariant="attached" /> |
| ~ | core |
. · Types · interface FieldStatusProps |
interface FieldStatusProps extends BaseProps<HTMLDivElement> (props of FieldStatus) → interface FieldStatusProps extends BaseProps<HTMLDivElement> |
import type {FieldStatusProps} from '@astryxdesign/core' |
| ~ | core |
. · Types · interface FieldStatusVariantMap |
tooltip: true → tooltip: true @deprecated |
import type {FieldStatusVariantMap} from '@astryxdesign/core'; const x: FieldStatusVariantMap = {tooltip: …} |
| ~ | core |
. · Types · interface SearchSource |
search: (query: string) => Promise<T[]> | T[] → search: (query: string, context?: SearchContext<T>) => Promise<T[]> | T[] |
import type {SearchSource} from '@astryxdesign/core'; const x: SearchSource = {search: …} |
| ~ | core |
. · Types · interface UseInputStatusIconOptions |
statusVariant?: FieldStatusVariant → statusVariant?: FieldStatusPlacement |
import type {UseInputStatusIconOptions} from '@astryxdesign/core'; const x: UseInputStatusIconOptions = {statusVariant: …} |
| ~ | richtext |
. · Components · component RichTextEditor |
prop statusVariant?: FieldStatusVariant = 'attached' → prop statusVariant?: FieldStatusVariant | 'tooltip' = 'attached' |
<RichTextEditor statusVariant="tooltip" /> |
Not run on this PR: add ci:run-visual, ci:run-a11y or ci:run-rtl to run them.
Generated by PR Enrichment workflow | View full report
d7da379 to
f4e2a69
Compare
There was a problem hiding this comment.
This adds the Swift ListItem, together with #8321's Item, and approval is on hold for a spec decision. The new draft records component:swift/Item and component:swift/ListItem state requirements that no current spec does, so an engineering owner needs to approve them on this head by commenting /approve-spec f4e2a69c5c94fb85f97935b301413cd1e7d56448 (the author can do this if they are an engineering owner) or say which requirement should change. Independently of that decision, these need fixing:
- A decimal number wider than
space.s4, such as "10.", widens only its own row's column (ListItem.swift:118), so the labels leave the list's text edge, while ListItem FR2 and core use a fixed 16 pt column. ListItemTests.swiftneeds the AST-093 FR5 checks for ListItem: an accessibility assertion for the number and the hidden dot or ring, a largest-text render, and a two-theme render.- The doc examples use a
Listand anAvatarthat the package does not have yet, and aListItemnested in a row's slot still takes the list's marker and inset, against FR4.
[Automated review]
| .font(astryx.font(size: "text-body-size", 14, weight: "text-body-weight")) | ||
| .foregroundStyle(astryx.tokens.color.textPrimary) | ||
| .fixedSize() | ||
| .frame(minWidth: astryx.tokens.space.s4, alignment: .leading) |
There was a problem hiding this comment.
A number wider than space.s4 (16 pt), such as "10." at the 14 pt body size, widens only this row's column, so from row 10 on the label, description and start content start further right than in rows 1-9. ListItem FR2 says "a marker in a column space.s4 wide", and core keeps the number box at width: --spacing-4 so its text edge never moves. Framing the number at exactly space.s4, as the dot and ring already are, would match both; a two-digit case in aDecimalListNumbersItsRows would pin it.
| /// draws as a plain `Item`. | ||
| /// | ||
| /// ```swift | ||
| /// List(listStyle: .decimal) { |
There was a problem hiding this comment.
The package has no List (or Avatar) view at this head, and SwiftUI's List has no listStyle: initializer, so this Quick Help example does not compile yet. Consider dropping it until List lands, or landing this with the List pull request.
| HStack(alignment: rowAlignment, spacing: astryx.tokens.space.s2) { | ||
| markerSlot | ||
| // A list's treatment is for this row only, not for rows nested in its slots. | ||
| slot(startContent.environment(\.astryxItemListTreatment, nil)) |
There was a problem hiding this comment.
This clears the treatment for nested Items, but a ListItem placed in a slot rebuilds its treatment from astryxListContext, which still flows in, so it takes the outer list's marker, density, corners and inset. ListItem FR4 says the treatment applies "not to rows nested in its start or end content". Clearing astryxListContext and astryxListNumbers for slot content, or narrowing FR4 to plain Items, would settle it.
| import Testing | ||
| @testable import Astryx | ||
|
|
||
| // Swift ListItem spec (component:swift/ListItem): an Item that takes its |
There was a problem hiding this comment.
AST-093 FR5 asks each native component for accessibility assertions, a largest Dynamic Type check, and a two-theme check. These tests have none for what ListItem draws itself: the number read as part of the row with the dot and ring hidden, the markers at .accessibility5, and the marker colour under a second theme.
There was a problem hiding this comment.
This adds the Swift ListItem, together with #8321's Item, and approval is on hold for a spec decision: the draft records component:swift/Item and component:swift/ListItem state requirements no current spec does, so an engineering owner needs to approve them on this head by commenting /approve-spec 8a88a9ab8eb073a9d34463b9732784c89ba4c3b7 (the author may do so if they are an engineering owner) or say which requirement should change. Independently of that decision, these need fixing:
- At the largest text sizes a numbered marker outgrows its fixed 16 pt column and draws over the start of its label (
ListItem.swift:141); growing the column and gap with the text scale keeps core's geometry at the default size, and a decimal case at.accessibility5would pin it. ListItem.swiftandListItem.spec.mdsit inSources/Astryx/List/, while AST-093 FR1 places each native component's spec in its own folder named as the core component,ListItem/.- The largest-text captures show the earlier per-row number column, and
Item's Quick Help example uses anAvatarview the package does not have yet.
[Automated review]
| .font(astryx.font(size: "text-body-size", 14, weight: "text-body-weight")) | ||
| .foregroundStyle(astryx.tokens.color.textPrimary) | ||
| .fixedSize() | ||
| .frame(width: astryx.tokens.space.s4, alignment: .leading) |
There was a problem hiding this comment.
At the largest text sizes the number's font grows 3.1 times (14 pt to 43.4 pt) while this frame stays 16 pt and the row adds only 8 pt before the label, so from "2." on the number draws over the start of the label; the largest-text capture in the description measures "2." at about 29 pt of ink. Growing the column and gap with the text scale, for example .frame(width: s4 * textScale + s2 * (textScale - 1), alignment: .leading), keeps core's 16 pt at the default size, and a decimal case with "10." in aMarkerStaysOnTheFirstLineAtTheLargestTextSize would pin it.
| path: "Sources/Astryx", | ||
| // Each component's spec lives beside its view (AST-093 FR1). | ||
| exclude: ["Icon/Icon.spec.md"], | ||
| exclude: ["Icon/Icon.spec.md", "Item/Item.spec.md", "List/ListItem.spec.md"], |
There was a problem hiding this comment.
AST-093 FR1 puts a native component's spec in its own folder named as the core component, Sources/Astryx/ListItem/ListItem.spec.md; here ListItem's view and record share List/ with the coming List. Moving both files, this exclude and the README line to ListItem/, or amending FR1, would settle it.
| /// | ||
| /// ```swift | ||
| /// Item("Ada Lovelace", description: "Mathematician") { | ||
| /// Avatar("Ada Lovelace") |
There was a problem hiding this comment.
There is no Avatar view in Sources/Astryx yet, so this Quick Help example does not compile; an Icon, as in ListItem's example, would.
375794c to
1e6ac1a
Compare
There was a problem hiding this comment.
This adds the Swift ListItem, together with #8321's Item, and approval is on hold for a spec decision: the new draft records component:swift/Item and component:swift/ListItem state requirements no current spec does, so an engineering owner needs to approve them on this head by commenting /approve-spec 1e6ac1abc4df99049b0f3a93bb7ebb5c1bd9f6b2 (the author may do so if they are an engineering owner) or say which requirement should change. The decision covers Item FR5's relative-href stop and static-row height, the closure-taking props initializers (Item FR6, ListItem FR5), Item OQ1's web-only swipe props, ListItem FR2's number column grown with Dynamic Type, and ListItem FR4's nested-row scope, which core's nested ListItem does not share. Independently of that decision, ListItem.swift and ListItem.spec.md sit in Sources/Astryx/List/, while AST-093 FR1 puts a native component's view and spec in a folder named as the core component, ListItem/; moving both files with the Package.swift exclude and the README line, or amending FR1, would settle it. The largest-text captures now match the grown number column, and Item's doc example uses only shipped views.
[Automated review]
1e6ac1a to
1c513b2
Compare
There was a problem hiding this comment.
This adds the Swift ListItem, together with #8321's Item, and approval is on hold for a spec decision: the new draft records component:swift/Item and component:swift/ListItem state requirements no current spec does, so an engineering owner needs to approve them on this head by commenting /approve-spec 1c513b21ff228f638f19d2b2237af5d43fbe4cf4 (the author may do so if they are an engineering owner) or say which requirement should change. The decision covers Item FR5's relative-href stop and static-row height, the closure-taking props initializers (Item FR6, ListItem FR5), Item OQ1's web-only swipe props, ListItem FR2's marker column grown with Dynamic Type, and ListItem FR4's nested-row scope, which core's nested ListItem does not share. Independently of that decision, two fixes are needed:
AstryxNodeViewdrawsItemandListItemnodes from their props alone (AstryxNodeView.swift:22-23), so a logic core'smarker,startContentandendContentnodes are silently dropped, against AST-082 FR19; passing them throughAstryxNodeList, as theCardcase does, and sending a slot node inanItemNodeDrawsAsAnItemwould settle it.ListItem.swiftandListItem.spec.mdsit inSources/Astryx/List/, while AST-093 FR1 puts a native component's view and spec in a folder named as the core component,ListItem/; moving both files with thePackage.swiftexclude and the README line, or amending FR1, would settle it.
[Automated review]
| case .item(let item): Item(item.viewProps(send: send)) | ||
| case .listItem(let row): ListItem(row.viewProps(send: send)) |
There was a problem hiding this comment.
These build each row from viewProps alone, so the marker, startContent and endContent nodes that ItemNode.Props and ListItemNode.Props decode never draw: a logic core's Icon in a row's start slot silently disappears, where main draws the unsupported placeholder that names Item. AST-082 FR19 says a content slot carries child nodes and a host draws every node it can read. Passing each slot through AstryxNodeList, as the .card case does with children, and sending a slot node in anItemNodeDrawsAsAnItem would settle it.
| path: "Sources/Astryx", | ||
| // Each component's spec lives beside its view (AST-093 FR1). | ||
| exclude: ["Icon/Icon.spec.md"], | ||
| exclude: ["Icon/Icon.spec.md", "Item/Item.spec.md", "List/ListItem.spec.md"], |
There was a problem hiding this comment.
AST-093 FR1 puts a native component's spec in its own folder named as the core component, Sources/Astryx/ListItem/ListItem.spec.md; here ListItem's view and record share List/ with the coming List. Moving both files, this exclude and the README line to ListItem/, or amending FR1, would settle it.
1c513b2 to
ec4f301
Compare
There was a problem hiding this comment.
This adds the Swift ListItem together with #8321's Item, and approval is on hold for a spec decision: the new draft records component:swift/Item and component:swift/ListItem state requirements no current spec does, so an engineering owner needs to approve them on this head by commenting /approve-spec ec4f301e7ba10a7640e483b4a7064cab7ec456fd, which the author may do if they are an engineering owner, or say which requirement should change. The decision covers Item FR5's relative-href stop and static-row height, the closure-taking props initializers (Item FR6, ListItem FR5), Item OQ1's web-only swipe props, ListItem FR2's marker column grown with Dynamic Type, and ListItem FR4's nested-row scope, which core's nested ListItem does not share. Independently of that decision, two fixes are needed:
AstryxNodeViewbuildsItemandListItemfrom their props alone (AstryxNodeView.swift:23-24), so a logic core'smarker,startContentandendContentnodes are silently dropped, against AST-082 FR19; passing each slot throughAstryxNodeList, as theCardcase does, and sending a slot node inanItemNodeDrawsAsAnItemwould settle it.ListItem.swiftandListItem.spec.mdsit inSources/Astryx/List/, while AST-093 FR1 puts a native component's view and spec in a folder named as the core component,ListItem/; moving both files with thePackage.swiftexclude and the README line, or amending FR1, would settle it.
[Automated review]
ec4f301 to
0765de5
Compare
There was a problem hiding this comment.
This adds the Swift ListItem together with #8321's Item, and approval is on hold for a spec decision: the new draft records component:swift/Item and component:swift/ListItem state requirements no current spec does, so an engineering owner needs to approve them on this head by commenting /approve-spec 0765de58bb902461c3b9b18b5d8be25b7c98a044, which the author may do if they are an engineering owner, or say which requirement should change. The decision covers Item FR5's relative-href stop and static-row height, the closure-taking props initializers (Item FR6, ListItem FR5), Item OQ1's web-only swipe props, ListItem FR2's marker column grown with Dynamic Type, ListItem FR4's nested-row scope, which core's nested ListItem does not share, and ListItem PR1, which says only rows in a list with markers measure their words while Item measures every row. Independently of that decision, two fixes are needed:
AstryxNodeViewbuildsItemandListItemfrom their props alone (AstryxNodeView.swift:25-26), so a logic core'smarker,startContentandendContentnodes are silently dropped, against AST-082 FR19; passing each slot throughAstryxNodeList, as theAvatarcase does, and sending a slot node inanItemNodeDrawsAsAnItemwould settle it.ListItem.swiftandListItem.spec.mdsit inSources/Astryx/List/, while AST-093 FR1 puts a native component's view and spec in a folder named as the core component,ListItem/; moving both files with thePackage.swiftexclude and the README line, or amending FR1, would settle it.
[Automated review]
| path: "Sources/Astryx", | ||
| // Each component's spec lives beside its view (AST-093 FR1). | ||
| exclude: ["Avatar/Avatar.spec.md", "Avatar/AvatarStatusDot.spec.md", "Icon/Icon.spec.md"], | ||
| exclude: ["Avatar/Avatar.spec.md", "Avatar/AvatarStatusDot.spec.md", "Icon/Icon.spec.md", "Item/Item.spec.md", "List/ListItem.spec.md"], |
There was a problem hiding this comment.
AST-093 FR1 places a native component's view and record in a folder named as the core component, Sources/Astryx/ListItem/ListItem.spec.md; this exclude, the README line and both ListItem files sit in List/. Moving them to ListItem/, or an engineering owner amending FR1, would settle it.
0765de5 to
a29c405
Compare
There was a problem hiding this comment.
This adds the Swift ListItem together with #8321's Item, and approval is on hold for a spec decision: the new draft records component:swift/Item and component:swift/ListItem state requirements no current spec does, so an engineering owner needs to approve them on this head by commenting /approve-spec a29c4059bbf5723816404a6ca37a6b927b94fcdb, which the author may do if they are an engineering owner, or say which requirement should change. The decision covers Item FR5's relative-href stop and static-row height, the closure-taking props initializers (Item FR6, ListItem FR5), Item OQ1's web-only swipe props, ListItem FR2's marker column grown with Dynamic Type, ListItem FR4's nested-row scope, which core's nested ListItem does not share, and ListItem PR1, which says only rows in a list with markers measure their words while Item measures every row. Independently of that decision, two fixes are needed:
AstryxNodeViewbuildsItemandListItemfrom their props alone (AstryxNodeView.swift:26-27), so a logic core'smarker,startContentandendContentnodes are silently dropped, against AST-082 FR19; passing each slot throughAstryxNodeList, as theAvatarcase does, and sending a slot node inanItemNodeDrawsAsAnItemwould settle it.ListItem.swiftandListItem.spec.mdsit inSources/Astryx/List/, while AST-093 FR1 puts a native component's view and spec in a folder named as the core component,ListItem/; moving both files with thePackage.swiftexclude and the README line, or amending FR1, would settle it.
[Automated review]
|
/approve-spec a29c405 |
a29c405 to
31c6227
Compare
There was a problem hiding this comment.
Approval is on hold for a spec decision: the draft records component:swift/Item and component:swift/ListItem state Swift-only requirements no current spec does, and the earlier /approve-spec names the previous head, so an engineering owner needs to approve them on this head by commenting /approve-spec 31c6227cb93762d4fc92fd4ab6c13896e5d93f66 (the author may do so as an engineering owner) or say which requirement should change. Independently of that decision, these need fixing:
AstryxNodeViewbuildsItemandListItemfrom their props alone, so a logic core'smarker,startContentandendContentnodes are silently dropped (AST-082 FR19); passing each slot throughAstryxNodeList, as theAvatarcase does, with a slot node inanItemNodeDrawsAsAnItem, would settle it.ListItem.swiftbelongs inSources/Astryx/ListItem/under AST-093 FR1, or FR1 needs amending.Itemre-implementsastryxBaseand Avatar'sisUnusableHrefandlinkURL; calling shared helpers keeps those paths from drifting.- ListItem PR1 says only rows in a list with markers measure their words, but
Itemmeasures every row; narrow the measurement or the wording.
[Automated review]
| │ │ ├── Icon/ # Icon | ||
| │ │ ├── Avatar/ # Avatar | ||
| │ │ ├── Item/ # Item | ||
| │ │ ├── List/ # ListItem |
There was a problem hiding this comment.
AST-093 FR1 puts a component's views in Sources/Astryx/<Name>/, named as the core component, so ListItem.swift belongs in Sources/Astryx/ListItem/; core's List/ folder name applies only to the record, which already sits at docs/specs/components/List/ListItem.swift.spec.md. Moving the file and this line, or amending FR1, would settle it.
| if props.base.isHidden != true { | ||
| control | ||
| .modifier(ItemIdentifier(id: props.base.id)) |
There was a problem hiding this comment.
astryxBase(_:) in Base/BaseProps.swift already removes the view for isHidden and maps id to the accessibility identifier, and Button, Card, Heading and Text call it. control.astryxBase(props.base) here would let the isHidden check and ItemIdentifier go, so a change to base props reaches Item too.
| nonisolated static func isUnusableHref(_ href: String?) -> Bool { | ||
| guard let href, !href.isEmpty else { return false } | ||
| guard let url = URL(string: href) else { return true } | ||
| return url.scheme == nil | ||
| } |
There was a problem hiding this comment.
This repeats Avatar.isUnusableHref (Avatar.swift:111), and linkURL below repeats Avatar's; one helper in Base/ that both call keeps the rule for which URLs an app can open in one place.
| // own disabled state). | ||
| words | ||
| .frame(maxWidth: .infinity, alignment: .leading) | ||
| .onGeometryChange(for: CGFloat.self) { $0.size.height } action: { wordsHeight = $0 } |
There was a problem hiding this comment.
Every row measures its words and writes wordsHeight here, though only the center marker in a marked list reads it; ListItem PR1 says only rows in a list with markers measure. Measuring only when listTreatment?.markerOnFirstLine == true, or rewording PR1, would line them up.
31c6227 to
6db4bc1
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the two draft records, component:swift/Item and component:swift/ListItem: their Swift-only requirements need approval at this exact head, recorded by commenting /approve-spec 6db4bc18131ae56a8e1b923766a40c7ae94f19bb (the author may do so as an engineering owner), or a note on which requirement to change. The Item wire slots and the shared destination rule are fixed; these still need changes after that decision:
AstryxNodeViewbuildsListItemfrom its props alone, so a logic core'sstartContentandendContentnodes never draw; pass them throughAstryxNodeListas theItemcase does.ListItem.swiftbelongs inSources/Astryx/ListItem/, the folder named after the core component, or the native guidelines need amending.Itemrepeats the shared base-props modifier and Avatar's href check, and that check reads the rawhrefwhile the link reads the cleaned one, so" https://example.com"stops a debug build although the row opens it.- ListItem's PR1 says only marked rows measure their words, but every
Itemrow does.
[Automated review]
| } marker: { | ||
| AstryxNodeList(nodes: item.props.marker ?? [], send: send) | ||
| } | ||
| case .listItem(let row): ListItem(row.viewProps(send: send)) |
There was a problem hiding this comment.
This builds the row from viewProps alone, so the startContent and endContent nodes ListItemNode.Props decodes never draw: a logic core's Icon in a row's start slot disappears with nothing to say so (AST-082 FR19). Passing both through AstryxNodeList, as the .item case above does, with a test like anItemNodeDrawsItsContentSlots, would fix it.
| /// build with its value; a release build draws the row without a link. | ||
| private func checkInDevelopment() { | ||
| #if DEBUG | ||
| if Item.isUnusableHref(props.href) { |
There was a problem hiding this comment.
This checks the raw href, while linkURL checks the trimmed destination from astryxSafeDestination, so " https://example.com" fires this assertion ("it draws without a link") although the row opens it. Checking the cleaned value, in one helper Avatar can share, would line the two up.
6db4bc1 to
b94da5d
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the two draft records, component:swift/Item and component:swift/ListItem: their Swift-only requirements need approval at this exact head, recorded by commenting /approve-spec b94da5d66297be0de0c786248b384566a1967c97 (the author may do so as an engineering owner), or a note on which requirement to change. Item's href check now matches its link; these still need changes after that decision:
AstryxNodeViewbuildsListItemfrom its props alone, so a logic core'sstartContentandendContentnodes never draw; pass them throughAstryxNodeListas theItemcase does, with a test.Itemrepeats the shared base-props modifier (itsisHiddencheck and a private identifier modifier); callingastryxBase(props.base)keeps base props on one path.ListItem.swiftbelongs inSources/Astryx/ListItem/, the folder named after the core component, or the native guidelines need amending.- ListItem's PR1 says only marked rows measure their words, but every
Itemrow does.
[Automated review]
b94da5d to
9a0c5a7
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the draft component:swift/ListItem record: its Swift-only requirements (the marker geometry, the nested-row scope, the closure initializers, the read-aloud markers, the performance notes and the native swipe exclusion) need approval at this exact head, recorded by commenting /approve-spec 9a0c5a74f2070680891d233c029e63b702162c4a (the author may do so as an engineering owner), or a note on which requirement to change. The ListItem wire slots now draw and the Swift tests pass; these still need changes after that decision:
- Move
ListItem.swifttoSources/Astryx/ListItem/and update the README line, since AST-093 FR1 puts a component's views in a folder named for it, or amend FR1. - Measure the words at
Item.swift:151only for a centre-aligned row in a marked list, or reword the record's PR1, which says only those rows measure. - Say in the description that the list look and the
Astryx.List(listStyle:)example arrive with the stacked Swift List.
[Automated review]
…arker, dividers, and edge
9a0c5a7 to
bfe451c
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the draft component:swift/ListItem record: its Swift-only requirements (the marker column and number geometry, the nested-row scope, the closure initializers, the number read aloud, PR1–PR2, and keeping swipe actions web-only on native) are stated by no current spec. An engineering owner records approval by commenting /approve-spec bfe451cd99163af21b3180a0e6b5fd02265114e6 on this pull request (the author may do so as an engineering owner), or says which requirement to change. The earlier folder, PR1 and description points are fixed, and nothing else needs to change.
[Automated review]
User need
Bulleted steps, numbered instructions, settings lists, and divided lists are everywhere on iOS. With only
Item, each app has to set the same density, marker, corners, and inset on every row by hand, and a bullet next to a wrapped row slides to the middle instead of staying by the first line. People get lists whose rows don't match each other or the web.Problem and solution fit
ListItemview. Core's list row: anItemthat takes its density, marker, dividers, and edge from theListaround it, so every row of a list matches without the app repeating them.Item. Outside a list it draws exactly asItem.Current specification
spec:AST-082FR3, FR4, FR7, FR19.spec:AST-093FR1–FR5.component:swift/Item, which draws the row; its DEC-1 keeps swipe actions web-only for now.component:swift/ListItematdocs/specs/components/List/ListItem.swift.spec.md(draft,platform_of: component:ListItem), beside core's ListItem record per AST-093 FR1.Public delta and caller burden
ListItem(Astryx) and generatedListItemProps,ListItemAlign(AstryxNative).Astryx.List(listStyle:)example above arrive with the SwiftList, the next pull request in this stack; until it lands, aListItemdraws as anItem.onClickcomes last.swipeActionsandswipeBehavior, web-only for now (component:swift/ItemDEC-1),target,rel.Itemgains internal list hooks (marker on the first line, inline inset, square corners) that onlyListItemsets, and shares its density inset rule withListItem; anItemalone is unchanged.Design and alternatives
Evidence
Capture set: iPhone 16 simulator, iOS 18.5, Xcode 16.4, built from the List pull request's head (which adds the
Listthese rows sit in, on top of this one), before the rebase onto the new spec layout and the number alignment; the number alignment, shared column, and wire slots are covered by the tests below. Each capture shows bullets on the first line for each alignment, circle and numbered markers, divided rows (selected with square corners, disabled), padded and flush edges on a filled container, compact and spacious density, and a row alone, in light and dark at the default and largest text sizes.check:knowledgepasses with the record at its new path.Implementation choices under current rules
ListItem.swiftsits in its own folder,Sources/Astryx/ListItem/, as AST-093 FR1 names a component's views; its record sits indocs/specs/components/List/, beside core's ListItem record, since core keeps ListItem inList/.Open-work search
Searched current records and open pull requests before creating
component:swift/ListItem, by canonical id (component:swift/ListItem,component:ListItem), paths (packages/native/apple/Sources/Astryx/List/,docs/specs/components/List/,packages/native/src/components/ListItem.ts), symbols (ListItem,ListItemProps,ListContext), and terms (native,swift,list). No other record or open pull request owns this platform component;component:swift/Item(#8321) draws its row.Scope
Writing rubric results
Testing
vitest run packages/native/src/contract: the ListItem contract (row slots and press; swipes,target,rel, andinteractiveRefon the web) passes; the committed Swift binding matches the contracts.swiftc -typecheckofAstryxNative,Astryx, and the tests passes. An immediate-mode render checks the row alone againstItem, list density, flush inset, square corners between dividers, the number column, numbering from 3, periods lining up for "1." and "10.", the shared column for a wide number, a wire row's content slots, and the marker's center on the label's first line forcenter,start, andfirstLine.Every changed file is under
packages/native/, plus the new record indocs/specs/components/List/.