Repository navigation
feat(native): Swift AvatarStatusDot shows status by shape and color, named with the avatar - #8328
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: No accessibility violations detected. Visual Regression55 of 502 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 35 more. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
f35f6e8 to
4681976
Compare
4681976 to
b830ce5
Compare
b830ce5 to
d330a66
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about the two new draft Swift records, component:swift/Avatar and component:swift/AvatarStatusDot: an engineering owner records it by commenting /approve-spec d330a66cdaf3b0e6c2aff5c2669f27568001bf71 on this pull request (the author may do so, being an engineering owner), and says for each of three departures from current spec whether the current claim will be amended or this pull request should conform: fixed-size initials versus spec:AST-082 FR11, the missing tooltip versus spec:AST-082 FR3, and AvatarStatusDot.spec.md living in Avatar/ versus spec:AST-093 FR1. Separately from that decision, these need fixing:
- Three of the six linked captures (Dark, Light largest text, Dark largest text) are the same image and it shows the light appearance, so please relink the dark and largest-text captures.
AvatarStatusDotTests.swiftchecks only the default light theme and never asserts the standalone dot's label and image trait, both of whichspec:AST-093FR5 asks for.warnIfUnnamedinAvatar.swiftcounts the status label as a name, so a link or button avatar with only a labelled dot can skip the warning that core and Avatar AR4 require.Avatar.spec.mdline 73 still lists status dots as a non-goal with no Swift view.
[Automated review]
|
|
||
| private func warnIfUnnamed() { | ||
| #if DEBUG | ||
| if Avatar.needsNameWarning(tap: tap, isNamed: accessibilityLabel != nil) { |
There was a problem hiding this comment.
accessibilityLabel includes the status dot's label, so a link or button avatar with no alt or name but a labelled dot can skip this warning once the label has arrived. Core counts only alt or name here, and AR4 says the same; consider isNamed: (Avatar.meaningful(props.alt) ?? Avatar.meaningful(props.name)) != nil.
|
|
||
| - The meaning of the props, the initials rule, and the size scale — owned by | ||
| `component:Avatar`. | ||
| - Status dots and avatar groups, which have no Swift view. |
There was a problem hiding this comment.
Status dots now have a Swift view, and FR7 and AR5 of this record own the slot; this non-goal could read "Avatar groups, which have no Swift view."
|
|
||
| @MainActor | ||
| @Test func eachVariantPaintsItsTokensAndShape() { | ||
| let tokens = Theme.default.light.color |
There was a problem hiding this comment.
This reads one theme only. spec:AST-093 FR5 asks for a render from two different themes and for assertions of the label and traits, here the standalone dot's label and image trait.
77a6f38 to
9c8add6
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about the two new draft Swift records, component:swift/Avatar and component:swift/AvatarStatusDot: an engineering owner records it by commenting /approve-spec 9c8add60d60a308ae621d9e37c7327efcf7fdb48 on this pull request (the author may do so, being an engineering owner), and says for each departure from current spec whether the current claim will be amended or this pull request should conform: the 1.6× cap on Dynamic Type growth versus spec:AST-082 FR11, the touch behaviour of tooltip and its new string | boolean contract kind versus spec:AST-082 FR3 and FR4, and AvatarStatusDot.spec.md living in Avatar/ versus spec:AST-093 FR1. Separately from that decision, two things need fixing:
- The default tooltip goes through
.help, which also sets the VoiceOver hint, so every avatar with anameis read with its name twice (Avatar.swift:66); core keeps the default name tooltip out of the description, so give the default name no hint and keep the hint for custom words. AvatarStatusDotTests.swiftnever renders the dot at the largest Dynamic Type size, whichspec:AST-093FR5 asks for, although anlgavatar's dot moves to the 32 pt tier there.
[Automated review]
| public var body: some View { | ||
| if props.base.isHidden != true { | ||
| control | ||
| .modifier(AvatarTooltipModifier(text: tooltipText)) |
There was a problem hiding this comment.
.help also sets the accessibility hint (Apple: "Adding help to a view configures the view's accessibility hint and its help tag"), and tooltipText is the name by default, so VoiceOver reads a named avatar's name and then the same name again as its hint. Core keeps the default name tooltip visual-only and describes only custom words (packages/core/src/Avatar/Avatar.tsx:638-643). Consider leaving the default name out of the hint and keeping it for a custom string, with a test of which case sets a hint.
| } | ||
|
|
||
| @MainActor | ||
| @Test func theDotSitsOnTheAvatarsCornerWithoutChangingItsSize() { |
There was a problem hiding this comment.
spec:AST-093 FR5 asks for a check at the largest Dynamic Type size, and no test here renders the dot at any size but the default. At .accessibility5 an lg avatar draws 77 pt, so its dot moves to the 32 pt tier and statusOffset changes; a variant of this test with typeSize: .accessibility5 asserting the 77 pt side and 32 pt dot would cover it.
9c8add6 to
f8428af
Compare
f8428af to
61f5fef
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about the two new draft Swift records, component:swift/Avatar and component:swift/AvatarStatusDot: an engineering owner records it by commenting /approve-spec 61f5feffd2c6374b4cae87ffaaca4b882229afe5 on this pull request (the author may do so, being an engineering owner), and says for each departure from current spec whether the current claim will be amended or this pull request should conform: the 1.6× cap on Dynamic Type growth versus spec:AST-082 FR11, the pointer-only tooltip and its new string | boolean contract kind versus spec:AST-082 FR3 and FR4, and AvatarStatusDot.spec.md living in Avatar/ versus spec:AST-093 FR1. Separately from that decision, four things need fixing:
- The pointer tooltip waits 500 ms and calls that core's delay (
Avatar.swift:433-434, Avatar FR8), but core's Avatar tooltip opens after 200 ms. Avatar.spec.mddoes not list the theme values the tooltip bubble reads (color.textPrimary,color.backgroundSurface,space.s1,space.s2,radius.container, and the body text size and weight), whichspec:AST-093FR4 asks for.- No test covers a hidden
AvatarStatusDot, althoughspec:AST-093FR5 asks for parameter-mapping tests. - The description says the largest-text captures match the default ones, but they show the avatars at 1.6× and the
lgdot at 32 pt.
[Automated review]
| @State private var pending: Task<Void, Never>? | ||
|
|
||
| /// Core's tooltip delay before showing on hover. | ||
| static let delay: Duration = .milliseconds(500) |
There was a problem hiding this comment.
Core's Avatar tooltip opens after 200 ms: Avatar.tsx calls useTooltip({placement: 'above', isEnabled: showTooltip}) with no delay, and useTooltip defaults to delay = 200. Consider 200 ms here and in Avatar FR8, or keep 500 ms as a stated platform choice rather than calling it core's.
| pages: Images, Buttons, and Accessibility in the Apple Human Interface | ||
| Guidelines. | ||
|
|
||
| Theme values read: `color.neutral`, `color.textSecondary`, `radius.element`, |
There was a problem hiding this comment.
spec:AST-093 FR4 asks each spec to list the theme values the component reads. AvatarTooltipBubble also reads color.textPrimary, color.backgroundSurface, space.s1, space.s2, radius.container, and the body text size and weight; they could join this list.
| } | ||
|
|
||
| @MainActor | ||
| @Test func aDotAloneIsAnImageNamedByItsLabel() { |
There was a problem hiding this comment.
No test hides a dot. A test like aHiddenAvatarDrawsNothing for AvatarStatusDot(AvatarStatusDotProps(base: AstryxBaseProps(isHidden: true))) would cover spec:AST-093 FR5's parameter mapping and guard against a hidden dot still adding its label to the avatar's name.
61f5fef to
20fed2e
Compare
20fed2e to
a11f1b6
Compare
df6b25e to
c0dd705
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about the two new draft records, component:swift/Avatar and component:swift/AvatarStatusDot: an engineering owner records it by commenting /approve-spec c0dd70535f3114d3ea14d72f205ccf6ed3bc73ff on this pull request (the author may do so as an engineering owner) and says, for each departure from current spec, whether the current claim will be amended or this pull request should conform. Three record fixes remain whatever that decision is.
- Departures to settle: the 1.6x Dynamic Type cap versus
spec:AST-082FR11; the newstring | booleancontract kind versus FR4; Avatar's tooltip, which a scroll view can cut off because the shared bubble draws in the avatar's own layer, versus FR3's rule that a platform never offers a prop it honors only in part; andAvatarStatusDot.spec.mdinAvatar/versusspec:AST-093FR1. - Avatar FR7 says four o'clock where core and
circleEdgeInsetuse the 45° point; use core's wording. - AvatarStatusDot FR2 makes the base
color.successandcolor.errora requirement where core requires the surface status roles and records the base tokens as a gap; align the row with core. - AvatarStatusDot's verification row says its tests fail on a name without the status label, but no test reads the label the dot reports to the avatar; mark that path not measured, as the Avatar record does, or add the test.
[Automated review]
| | FR5 | With an `href` that is an absolute URL (it has a scheme), the avatar MUST be a SwiftUI `Link`; with `onClick` too, `onClick` runs as it opens and the URL then goes to the app's own URL handler, or the system's. An `href` that is not an absolute URL, such as a web path, MUST stop a development build with a message naming it; a release build draws the avatar without a link, as a button with `onClick` or static. With only `onClick`, a SwiftUI `Button`. Either draws exactly as the static avatar, and the avatar has no disabled state. | Swift component API A4 | verify | | ||
| | FR6 | Both initializers MUST build the same `AvatarProps`; `name` is the unlabeled first parameter, a single trailing closure is the status, and `onClick` comes last. Base props come through `AvatarProps`: with `isHidden` the view draws nothing; `id` becomes the accessibility identifier. | Swift component API A2–A4, A7; `spec:AST-082` FR7 | verify | | ||
| | FR7 | The `status` slot MUST draw its content, one `AvatarStatusDot`, outside the avatar's clip: on a circle with the dot's center on the edge at four o'clock, on a rounded or square avatar overhanging the bottom trailing corner by a quarter of the dot, mirrored in right-to-left layouts. It MUST NOT change the avatar's size or layout, and gives the dot the avatar's side for its size tier. | core `Avatar` status position; Swift slots S4 | verify | | ||
| | FR8 | `tooltip` MUST resolve, as core does, to the name for `nil` or `true`, the given words for a string, and nothing for `false` or blank words, and MUST show through the Astryx tooltip (`architecture:native-swift-library-tooltip`), with the focus and touch the Tooltip by input table below states: a static avatar is a tab stop and opens on a tap; a link or button keeps its focus and its tap. Given words are the VoiceOver hint. On the wire it is the web value, a string or a Bool. | core `Avatar` `tooltip`; core `Tooltip`; `spec:AST-082` FR3 | verify | |
There was a problem hiding this comment.
FR8 shows tooltip through the shared Astryx tooltip, whose draft record says the bubble "draws in the view's own layer, so a scroll view or another clipping container around the view cuts it off" (packages/native/specs/swift/library/tooltip.spec.md INV1). Avatar is the first view to offer tooltip through it, and people lists scroll, while spec:AST-082 FR3 says a platform never offers a prop it honors only in part. This is one of the departures waiting on an engineering owner's ruling: amend FR3 on main, or draw the bubble above clipping containers.
c0dd705 to
7907c24
Compare
7907c24 to
891203c
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about the new draft rows in component:swift/AvatarStatusDot and component:swift/Avatar (FR6, FR7 and AR6): an engineering owner records it by commenting /approve-spec with the full SHA of the head they approve, /approve-spec 891203c1d6aa354d33fb3832e24c996820877478 for this one, and the author may do so as an engineering owner. The decision also needs to settle whether spec:AST-093 FR1 will be amended to allow the dot's record and view in Avatar/, or they should move to AvatarStatusDot/. One fix is needed either way:
- Right to left, the status dot lands inside the avatar's leading edge: SwiftUI already mirrors the
.offsetinside the.bottomTrailingoverlay, so negating it atAvatar.swift:201moves the dot inward (5 pt inside a square's edge and 6 pt inside a circle's in the pull request's own capture); dropping the negation and adding a right-to-left render test fixes it.
[Automated review]
| // Outside the clip, so the dot is never cut off. | ||
| status | ||
| .environment(\.astryxAvatarSide, side) | ||
| .offset(x: layoutDirection == .rightToLeft ? -statusOffset : statusOffset, y: statusOffset) |
There was a problem hiding this comment.
Right to left, SwiftUI already mirrors this horizontal offset inside the .bottomTrailing overlay, so the negation moves the dot inward: in the capture attached to this pull request the dot sits 5 pt inside a square avatar's leading edge and its centre 6 pt inside a circle's 45° point, where core Avatar FR4 and this record's FR7 mirror the left-to-right placement. .offset(x: statusOffset, y: statusOffset) keeps the outward push in both directions, and a right-to-left render test would pin it.
891203c to
1a2902f
Compare
1a2902f to
006afd5
Compare
There was a problem hiding this comment.
The Swift status dot follows core's tiers, marks, placement, and composed name, and its tests pass on macOS and the iOS Simulator. Approval waits on an engineering owner deciding the draft Swift requirements this head adds (the new component:swift/AvatarStatusDot record and Avatar FR6, FR7, and AR6, including the trailing-closure change) by commenting /approve-spec 006afd55abdc041a6e7a14a6d7ed9c1aa11a3184 on this pull request; the author may do so as an engineering owner, and a later commit needs the command again. The owner should also say whether AvatarStatusDot.spec.md may stay in Avatar/ under AST-093 FR1 or must move to AvatarStatusDot/. Two fixes are needed either way:
- The right-to-left rows in the attached captures put the dot inside the leading edge, while FR7 and
rightToLeftTheDotMirrorsOutsideTheLeadingCornerput it outside; please capture again from this head or mark right-to-left placement as not measured. eachVariantPaintsItsTokensAndShapereads only the neutral dot's centre, so a missing ring still passes; assert a pixel on the ring or narrow the FR2 verification row.
[Automated review]
| #expect(close(success.at(16, 16), rgba(tokens.success))) | ||
| #expect(close(error.at(16, 16), rgba(tokens.backgroundSurface))) | ||
| #expect(close(error.at(16, 9), rgba(tokens.error))) | ||
| #expect(close(neutral.at(16, 16), rgba(tokens.backgroundSurface))) |
There was a problem hiding this comment.
This reads the neutral dot's centre, which is the surface plate with or without the ring, and no other test draws .neutral, so removing the ring keeps every test green although the FR2 verification row says a missing glyph fails. A pixel on the ring would pin it, for example at the large tier, where the ring spans 10 to 12 pt from the centre:
#expect(close(neutral.at(16, 5), rgba(tokens.textSecondary)))
| | FR5 | With an `href` that is an absolute URL (it has a scheme), the avatar MUST be a SwiftUI `Link`; with `onClick` too, `onClick` runs as it opens and the URL then goes to the app's own URL handler, or the system's. An `href` that is not an absolute URL, such as a web path, MUST stop a development build with a message naming it; a release build draws the avatar without a link, as a button with `onClick` or static. With only `onClick`, a SwiftUI `Button`. Either draws exactly as the static avatar, and the avatar has no disabled state. | Swift component API A4 | verify | | ||
| | FR6 | Both initializers MUST build the same `AvatarProps`; `name` is the unlabeled first parameter and `onClick` the last, so it can be a trailing closure. Base props come through `AvatarProps`: with `isHidden` the view draws nothing; `id` becomes the accessibility identifier. | Swift component API A2–A4, A7; `spec:AST-082` FR7 | verify | | ||
| | FR6 | Both initializers MUST build the same `AvatarProps`; `name` is the unlabeled first parameter, a single trailing closure is the status, and `onClick` comes last. Base props come through `AvatarProps`: with `isHidden` the view draws nothing; `id` becomes the accessibility identifier. | Swift component API A2–A4, A7; `spec:AST-082` FR7 | verify | | ||
| | FR7 | The `status` slot MUST draw its content, one `AvatarStatusDot`, outside the avatar's clip: on a circle centered on the circle's edge at the bottom trailing 45° point, on a rounded or square avatar overhanging the bottom trailing corner by a quarter of the dot, mirrored in right-to-left layouts. It MUST NOT change the avatar's size or layout, and gives the dot the avatar's side for its size tier. | core `Avatar` status position; Swift slots S4 | verify | |
There was a problem hiding this comment.
The captures attached to the description draw this row's right-to-left case with the dot inward: on the lg square it sits 5 pt inside the leading edge and on the circle its centre is 6 pt inside the mirrored 45° point, while .offset(x: statusOffset, y: statusOffset) and rightToLeftTheDotMirrorsOutsideTheLeadingCorner put it outside. AST-093 FR5 rests appearance on captures of the change, so the right-to-left rows need a capture from this head, or a not-measured note.
006afd5 to
40735fc
Compare
40735fc to
82b9b6f
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the draft Swift requirements this head adds (the new component:swift/AvatarStatusDot record and Avatar FR6, FR7 and AR6, including the trailing-closure change), recorded by commenting /approve-spec 82b9b6f72c9b6fc8073ee7bceddfbeaa98064e2f on this pull request; the author may do so as an engineering owner, and a later commit needs the command again. The owner should also say whether AvatarStatusDot.spec.md may stay in Avatar/ under AST-093 FR1 or must move to AvatarStatusDot/. One fix is needed either way:
- The busy dot's minus bar is drawn in
color.backgroundSurface(AvatarStatusDot.swiftline 88), while theme-tokens INV11 and core's AvatarStatusDot FR2 put it in on-error; in the Y2K light theme it is 1.52:1 against its plate, so please usecolor.onError, update FR2 to match, and test the bar in a theme where the two colors differ.
[Automated review]
|
|
||
| /// What the glyph draws in: the surface on a colored dot, secondary text on the ring. | ||
| var ink: Color { | ||
| variant == .neutral ? astryx.tokens.color.textSecondary : astryx.tokens.color.backgroundSurface |
There was a problem hiding this comment.
The error bar takes backgroundSurface here, while theme-tokens INV11 puts content on a strong status fill in --color-on-<status> and core's AvatarStatusDot FR2 paints the minus bar in --color-on-error. In the Y2K light theme this bar is white on the pale pink plate at 1.52:1 (on-error gives 6.69:1), and in the default dark theme it is 4.37:1. Returning astryx.tokens.color.onError for .error fixes both.
| | ID | Candidate invariant | Basis | Draft review state | | ||
| | --- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------- | ------------------ | | ||
| | FR1 | The dot MUST take core's tier for its avatar's side: up to 36 points a 10-point dot with a 1-point border and 1-point glyph stroke; up to 72 a 20-point dot, 2-point border, 1.5-point stroke; larger a 32-point dot, 4-point border, 2-point stroke. Outside an avatar it is small. | core `AvatarStatusDot` size tiers | verify | | ||
| | FR2 | `success` MUST fill with the success plate color and draw no glyph; `neutral` MUST fill with `color.backgroundSurface` and draw a ring in `color.textSecondary` on the inner field; `error` MUST fill with the error plate color and draw a round-capped bar in `color.backgroundSurface` across three quarters of the inner field. Every dot has a `color.backgroundSurface` border. Status fills use the surface-specific status roles, as core requires (`architecture:theme-tokens` INV11); core records that its plates still use the base tokens, so the success and error plates fill with `color.success` and `color.error`, as core's do, until core moves. | core `AvatarStatusDot` styles | verify | |
There was a problem hiding this comment.
FR2 draws the error bar in color.backgroundSurface and says core's plates "still use the base tokens"; core's current FR2 calls the plates "the strong status fills" and draws the bar in --color-on-error. This row and the theme-value list should name color.onError for the bar.
82b9b6f to
7d64e9d
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the draft Swift requirements this head adds (the new component:swift/AvatarStatusDot record and Avatar FR6, FR7 and AR6, including the trailing-closure change), recorded by commenting /approve-spec 7d64e9d4085c8a6b8b6327b152de86b8ac0eef50 on this pull request; the author may do so as an engineering owner, and a later commit needs the command again. The owner should also say whether AvatarStatusDot.spec.md may stay in Avatar/ under AST-093 FR1 or must move to AvatarStatusDot/. Two fixes are needed either way:
- The status slot places and names everything as a dot (
Avatar.swiftlines 212–218), so other content, such as a labeledIcon, lands off the position core's Avatar FR4 gives it and its label never joins the name as core's FR5 composes it; please size the offset from the content and read its label, or limit the slot toAvatarStatusDot. - AvatarStatusDot FR2 still says core records base-token plates, while core's current FR2 calls them the strong status fills; please update that sentence.
[Automated review]
| var statusOffset: CGFloat { | ||
| let dot = AvatarStatusDot.tier(forAvatarSide: side).dot | ||
| switch resolvedShape { | ||
| case .circle: return dot / 2 - side * Avatar.circleEdgeInset | ||
| case .rounded, .square: return dot / 4 | ||
| } | ||
| } |
There was a problem hiding this comment.
The offset comes from the dot tier, not from what the slot holds, so any other status content (an Icon, a wire node, any view) lands off the point core's Avatar FR4 gives it: a 24 pt icon in an md circle's slot centers at (23.7, 23.7) instead of (30.7, 30.7). Core translates its slot by 50% or 25% of its own size. Its label also never joins the name, since only AvatarStatusDot reports one, while core FR5 reads a string label on the passed element. Sizing the offset from the content (for example alignment guides reading its ViewDimensions) and reading a held component's label would match core; or the slot could accept only AvatarStatusDot.
| | ID | Candidate invariant | Basis | Draft review state | | ||
| | --- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------- | ------------------ | | ||
| | FR1 | The dot MUST take core's tier for its avatar's side: up to 36 points a 10-point dot with a 1-point border and 1-point glyph stroke; up to 72 a 20-point dot, 2-point border, 1.5-point stroke; larger a 32-point dot, 4-point border, 2-point stroke. Outside an avatar it is small. | core `AvatarStatusDot` size tiers | verify | | ||
| | FR2 | `success` MUST fill with the success plate color and draw no glyph; `neutral` MUST fill with `color.backgroundSurface` and draw a ring in `color.textSecondary` on the inner field; `error` MUST fill with the error plate color and draw a round-capped bar in `color.onError` across three quarters of the inner field. Every dot has a `color.backgroundSurface` border. Status fills use the surface-specific status roles, as core requires (`architecture:theme-tokens` INV11); core records that its plates still use the base tokens, so the success and error plates fill with `color.success` and `color.error`, as core's do, until core moves. | core `AvatarStatusDot` styles | verify | |
There was a problem hiding this comment.
"core records that its plates still use the base tokens … until core moves" no longer holds: core's current AvatarStatusDot FR2 says "The plates are the strong status fills and their ink is the matching content role", and theme-tokens INV11 makes --color-<status> the strong status fill. The code is right; this sentence could say the plates are the strong status fills color.success and color.error with their on-color ink.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the draft Swift requirements this head adds (the new component:swift/AvatarStatusDot record, Avatar FR6, FR7 and AR6 including the trailing-closure change, and the Swift Icon record's new AR1 sentence), recorded by commenting /approve-spec 1c4bdf00c288ce0f283a2370147214268b06a12e on this pull request; the author may do so as an engineering owner, and a later commit needs the command again. The owner should also say whether AvatarStatusDot.spec.md may stay in Avatar/ under AST-093 FR1 or must move to AvatarStatusDot/. Two changes are needed either way:
- A labeled
Iconplaced before anAvatarStatusDotin the status slot outranks the dot's label (AvatarStatusDot.swiftlines 152–153), so VoiceOver reads "Ada Lovelace, Verified" where core's Avatar FR5 and ORD2 give "Ada Lovelace, Online"; please let a dot's label win. - The non-dot status state has no capture and no test that renders its measured placement, which AST-093 FR5 asks for; please attach captures of an avatar holding a labeled
Iconat this head, or report that state as not measured, and add a test that fails without the measurement.
[Automated review]
| static let defaultValue: String? = nil | ||
|
|
||
| static func reduce(value: inout String?, nextValue: () -> String?) { | ||
| value = value ?? nextValue() |
There was a problem hiding this comment.
This keeps the first label in view order, and Icon now writes the same key (Icon.swift line 49), so HStack { Icon(.success, label: "Verified"); AvatarStatusDot(label: "Online") } names the avatar "Ada Lovelace, Verified". Core's Avatar FR5 and ORD2 let the dot's reported label win, and core names the same slot "Ada Lovelace, Online". Carrying the source with the value and preferring a dot's label here would match core.
| // slot without changing the avatar's size. Its label joining the name in a | ||
| // rendered tree is not measured (AR3). | ||
| @MainActor | ||
| @Test func aLabeledIconInTheStatusSlotIsPlacedFromItsSize() { |
There was a problem hiding this comment.
This checks only that the avatar keeps its size, so it still passes with the .onGeometryChange line in Avatar.swift removed, when the icon would sit where a dot of the avatar's tier does. AST-093 FR5 asks for a state-change test and captures of this state; checking the icon's position, or recording the measured placement as not measured in the FR7 row, would close the gap.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the draft Swift requirements this head adds (the new component:swift/AvatarStatusDot record, Avatar FR6, FR7 and AR6 including the trailing-closure change, and the Swift Icon record's new AR1 sentence), recorded by commenting /approve-spec b9865c967252d1daa4b15628c0bf61eeef2e51c9 on this pull request; the author may do so as an engineering owner, and a later commit needs the command again. The owner should also say whether AvatarStatusDot.spec.md may stay in Avatar/ under AST-093 FR1 or must move to AvatarStatusDot/, and whether status content should keep its own text-size growth, since at the largest text size a labeled Icon reaches 62 pt on a 77 pt avatar in the description's captures. One change is needed either way: the Swift Avatar record's AR1 and ORD2 still say an avatar without alt or name is hidden from VoiceOver, while AR6 now names it by its status label, so please align them as core's Avatar AR1 and ORD2 read.
[Automated review]
| `color.textSecondary` on `color.neutral`, as core's Avatar does. In the | ||
| default and neutral themes' dark appearance that pair is below 4.5:1, in | ||
| core as well; the fix belongs to the theme tokens. | ||
| - **AR6 — Status in the name.** The status label joins the avatar's name as |
There was a problem hiding this comment.
This AR6 names an avatar by its status label alone, and Avatar.swift line 288 does so, but AR1 (line 144) still says an avatar without alt or name "is hidden from VoiceOver" and ORD2 (line 134) still reads "alt → name → none". Please add "and no status label" to AR1, and the status composition with the dot-then-Icon label order to ORD2, as core's Avatar AR1 and ORD2 read.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the draft Swift requirements this head adds (the new component:swift/AvatarStatusDot record; Avatar FR6, FR7 with its new Accessibility 1 cap on status content, and AR6; and the Swift Icon record's AR1 sentence), recorded by commenting /approve-spec 60235fe3a37942c722af01b84368b8f54efe6227 on this pull request; the author may do so as an engineering owner, and a later commit needs the command again. The owner should also say whether spec:AST-082 FR11 will be amended for that cap, which stops text in the status slot from growing past Accessibility 1, and whether AvatarStatusDot.spec.md may stay in Avatar/ under spec:AST-093 FR1 or must move to AvatarStatusDot/. One change is needed either way: the description's largest-text captures were built before the cap and show a 62 pt status Icon this head no longer draws, so please recapture them from this head, or report that state as not measured, and mention the cap in the description.
[Automated review]
| .environment(\.astryxAvatarSide, side) | ||
| // The owner sizes what it holds (slots S4): status content never | ||
| // grows with text faster than the avatar does. | ||
| .dynamicTypeSize(...Avatar<EmptyView>.statusTypeSizeCap) |
There was a problem hiding this comment.
This cap is new at this head, so the description's largest-text captures, built before it, show a status Icon at 62 pt on a 77 pt avatar where this line now holds it near 32 pt. spec:AST-093 FR5 rests text-size claims on captures attached to the change: please recapture the light and dark largest-text "end" captures from this head, or report the capped state as not measured, and mention the cap in the description.
| | FR4 | Initials and the person symbol MUST draw in `color.textSecondary` on `color.neutral`; initials use the theme's body family at medium weight and 40% of the side. | core `Avatar` fallback styles; `spec:AST-093` FR4 | verify | | ||
| | FR5 | With an `href` that is an absolute URL (it has a scheme), the avatar MUST be a SwiftUI `Link`; with `onClick` too, `onClick` runs as it opens and the URL then goes to the app's own URL handler, or the system's. An `href` that is not an absolute URL, such as a web path, MUST stop a development build with a message naming it; a release build draws the avatar without a link, as a button with `onClick` or static. With only `onClick`, a SwiftUI `Button`. Either draws exactly as the static avatar, and the avatar has no disabled state. | Swift component API A4 | verify | | ||
| | FR6 | Both initializers MUST build the same `AvatarProps`; `name` is the unlabeled first parameter, a single trailing closure is the status, and `onClick` comes last. Base props come through `AvatarProps`: with `isHidden` the view draws nothing; `id` becomes the accessibility identifier. | Swift component API A2–A4, A7; `spec:AST-082` FR7 | verify | | ||
| | FR7 | The `status` slot MUST draw its content, usually one `AvatarStatusDot`, outside the avatar's clip, placed from the content's own size, in the same layout pass, as core places any status: on a circle centered on the circle's edge at the bottom trailing 45° point, on a rounded or square avatar overhanging the bottom trailing corner by a quarter of its size, mirrored in right-to-left layouts. It MUST NOT change the avatar's size or layout, gives a dot the avatar's side for its size tier, and lets other content grow with Dynamic Type no faster than the avatar, up to the text size at its 1.6 times growth, since the owner sizes what it holds (Swift slots S4). | core `Avatar` status position; Swift slots S4 | verify | |
There was a problem hiding this comment.
The new last clause holds every status view at Accessibility 1, so text placed in the slot stops following text sizes above it, while spec:AST-082 FR11 says "text follows the platform's text-size setting". This is one of the rulings the open owner question asks for: amend FR11 on main, or let text in the slot keep growing.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the draft Swift requirements this head adds (the new component:swift/AvatarStatusDot record; Avatar FR6, FR7 and AR6; and the Swift Icon record's AR1 sentence), recorded by commenting /approve-spec 850812ff80fd3fb89ac9b8f52f970d8140f3122c on this pull request; the author may do so as an engineering owner, and a later commit needs the command again. The owner should also say whether spec:AST-082 FR11 allows FR7's Accessibility 1 cap on text in the status slot, and whether AvatarStatusDot.spec.md may stay in Avatar/ under spec:AST-093 FR1 or must move to AvatarStatusDot/. One fix is needed either way: AstryxStrings.avatarNameWithStatus substitutes {status} into the name itself, so an avatar named "Fan of {status}" with an "Online" dot is read as "Fan of Online, Online"; replacing {status} before {name}, or filling both in one pass, keeps the name as core's message does.
[Automated review]
| .replacingOccurrences(of: "{name}", with: name) | ||
| .replacingOccurrences(of: "{status}", with: status) |
There was a problem hiding this comment.
Two passes substitute into the name too: with name = Fan of {status} and an "Online" dot, the second pass turns the name's own {status} into "Online", giving "Fan of Online, Online", where core's ICU message (core Avatar FR5) keeps the name as given. Replacing {status} first and {name} last (the status is the app's own label), or filling both placeholders in one pass, avoids it; a case in theStatusJoinsTheAvatarsName would pin it.
…named with the avatar
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the draft Swift requirements this head adds (the new component:swift/AvatarStatusDot record; Avatar FR6, FR7 and AR6; and the Swift Icon record's AR1 sentence), recorded by commenting /approve-spec a440083d66c668951f71a9ffaf451cae8415688f on this pull request; the author may do so as an engineering owner, and a later commit needs the command again. The owner should also say whether spec:AST-082 FR11 allows FR7's Accessibility 1 cap on text in the status slot, and whether AvatarStatusDot.spec.md may stay in Avatar/ under spec:AST-093 FR1 or must move to AvatarStatusDot/. The name-with-status fill now fills each placeholder once, so a name containing {status} is read as written, and no other change is needed.
[Automated review]
|
Posted by Cindy's agent on her behalf. Decision (Cindy, 2026-10-26): approved. The Swift AvatarStatusDot record and the Avatar and Icon record changes on this head are accepted as written. That includes the status-content growth cap under slots S4, which keeps AST-082 FR11 unamended, and the spec file staying beside its view in What this PR needs: nothing more. It merges at this head once CI is green. |
|
/approve-spec a440083 |
Builds on #8306 (Swift Avatar) and carries its commits until #8306 lands; the last commit is this change. The base is
mainso CI runs.User need
People lists and chat headers show who is online, away, or busy on each person's avatar. Swift
Avatarhad nostatusslot, so an iOS app could not show presence at all, or drew its own dot that relied on color alone and that VoiceOver never read.Problem and solution fit
AvatarStatusDotview. Core's three variants, each with its own shape so status never rests on color alone:successa filled dot,neutrala ring,errora minus bar. Core's size tiers (10, 20, 32 pt) follow the avatar's size.Avatargains itsstatusslot. The dot sits outside the avatar's clip, on a circle's edge at four o'clock or overhanging a rounded or square corner by a quarter, mirrored in right-to-left layouts, and never changes the avatar's size.Current specification
spec:AST-082FR4 (typed content slot), FR7;spec:AST-093FR1–FR5.component:swift/AvatarStatusDotatpackages/native/apple/Sources/Astryx/Avatar/AvatarStatusDot.spec.md(draft,platform_of: component:AvatarStatusDot, beside the view as core keeps it inAvatar/).component:swift/Avatar(from feat(native): Swift Avatar with core's picture, initials, and fallback order #8306): FR7 (status placement), AR4 (status in the name), FR6 (a single trailing closure is now the status); its OQ2 is resolved and removed.Public delta and caller burden
AvatarStatusDotand generatedAvatarStatusDotProps,AvatarStatusDotVariant.Avataris generic over its status content, and a single trailing closure is the status. A button avatar now passesonClick:by label or as a trailingonClick:closure. This matches Item, where a single trailing closure is content and the action comes last, and it stopsAvatar("Ada") { AvatarStatusDot() }from silently becoming a button.iconinside the dot (core's is free-form content; it needs the shared icon-name slot).Design and alternatives
Evidence
Capture set: iPhone 16 simulator, iOS 18.5, Xcode 16.4, built from
60235fe3(the head since changes only the record), with the initials drawn, the right-to-left dot outside the leading corner, the busy bar in on-error, and a labeled Icon in the status slot placed from its own size (circle, square, right to left). At the largest text size the status Icon grows only as far as the avatar does, 1.6 times: status content still grows with Dynamic Type (AST-082 FR11) up to its owner's growth, since the owner sizes what it holds (Swift slots S4). Each capture shows the three variants, the three tiers on sm, md, lg, and xl avatars, the three shapes, right-to-left placement, and two themes.At the largest text size the avatars grow 1.6 times with #8306's text-size growth, and an lg avatar's dot moves up to the 32 pt tier.
Not measured here: #8306's tooltip on hover, focus, and tap, which
simctlcaptures cannot drive; an app-hosted UI test opens it with a tap above the avatar.Not measured: the composed name ("Ada Lovelace, Online") in a rendered accessibility tree. No app-hosted test reads it yet;
AvatarStatusDotTests.swifttests the name the avatar composes from the dot's label.An AvatarStatusDot wire node draws as AvatarStatusDot in
AstryxNodeView, and an Avatar node draws itsstatuschildren as its dot (AST-082 FR19). The component API record lists both among the viewsAstryxNodeViewdraws. The status name joins the avatar's as AR6; tests check the composed name and a render of the dot. The name in a rendered accessibility tree is not measured (AR3).check:knowledgedoes not scan native component specs yet; both records pass the checker in #8288, which does.Implementation choices under current rules
AvatarStatusDot.spec.mdsits beside its view inSources/Astryx/Avatar/.spec:AST-093FR1 names the folder after core's, and core keeps the dot inAvatar/.spec:AST-082FR3, FR4, and FR11 as they stand.Open-work search
Searched current records and open pull requests before creating
component:swift/AvatarStatusDot, by canonical id (component:swift/AvatarStatusDot,component:AvatarStatusDot), paths (packages/native/apple/Sources/Astryx/AvatarStatusDot/,packages/native/src/components/AvatarStatusDot.ts), symbols (AvatarStatusDot,AvatarStatusDotProps,statusslot,@astryx.avatar.nameWithStatus), and terms (native,swift,avatarstatusdot). No other record or open pull request owns this platform component; the Swift views moved onto core names in #8299 and the Icon slot in #8302 are its neighbors.Scope
Writing rubric results
Testing
vitest run packages/native: the Avatar status slot and the AvatarStatusDot contract pass; the regenerated string catalogs pass their drift check.swiftc -typecheckofAstryxNative,Astryx, and the tests passes. An immediate-mode render checks each variant's fill and glyph at the large tier, that a dot leaves a 48 pt avatar 48 pt, and the placement offsets.Every changed file is under
packages/native/.