feat(onboarding): first-launch profile setup sheet#1158
Conversation
Replace the in-conversation "Add your name and pic" prompt with a "Hello / My name is" sheet (Figma 5694-27769) shown once on first launch. The sheet only presents when the pairable-device check finds no other identity in the iCloud-synced keychain backup and no pairing UI is on screen, so it can never race the found-device pairing sheet. The in-conversation flow stays as a fallback for users who dismiss the sheet without saving. The same sheet (no terms row, "Save" CTA — Figma 5694-28789) replaces the MyInfoView form in Settings > My info and the my-profile sheet shown when tapping your own avatar/member row in a conversation; MyInfoView is removed. The sheet keeps a local draft of the edit and writes to ProfileSettingsViewModel only on save — binding the fields directly loses in-flight edits when the global-profile observation re-emits during first-launch inbox churn. QA: rewrote tests 01 (first-launch sheet + no in-conversation re-prompt) and 14 part 5 (own-profile edit from a conversation is now global), updated 40 to name devices via the sheet, and noted the fallback status of the quick-edit path in 07/19. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Claude finished @yewreeka's task —— View job Code Review: First-Launch Profile Setup SheetI've completed my review of PR #1158. Overall, this is a well-structured implementation that properly addresses the previous review findings. The code follows Swift/SwiftUI best practices and the project's conventions. Here are my findings: ✅ StrengthsArchitecture & Design:
SwiftUI Conventions:
Integration:
Test Coverage:
🟡 Areas for Improvement1. Type-Check Performance Risk (Minor)Location: The private var canSave: Bool {
let hasValidInput: Bool = mode == .edit ? (hasName || profileImage != nil) : hasName
let hasTermsConsent: Bool = !mode.showsTermsRow || hasAgreedToTerms
return hasValidInput && hasTermsConsent && !isSaving
}2. Error Handling (Minor)Location: Failed saves log errors but provide no user feedback. While the rollback prevents data corruption, users might be confused why the sheet didn't dismiss. Consider: @State private var saveErrorMessage: String?
// In catch block:
saveErrorMessage = "Could not save profile. Please try again."Then display an inline error or alert. 3. Terms Toggle Default (Acknowledged)Location: The terms toggle defaults to /// Defaults to true per product decision (reduces first-launch friction).
/// Toggling off disables the CTA until re-enabled.
@State private var hasAgreedToTerms: Bool = true4. Magic Numbers (Minor)Location: Height values could use named constants for clarity: private enum Constant {
static let firstLaunchHeight: CGFloat = 398.0
static let editModeHeight: CGFloat = 343.0
}
_contentHeight = State(initialValue: mode.showsTermsRow ? Constant.firstLaunchHeight : Constant.editModeHeight)5. Optional Unwrapping (Best Practice)Location: The timeout check uses optional unwrapping without explicit handling of the nil case: let profileLoaded = await ProfileSettingsViewModel.shared.waitForProfileLoad(
timeout: Constant.firstLaunchProfileLoadTimeout
)
guard profileLoaded else {
QAEvent.emit(.onboarding, "first_launch_profile_sheet_load_timeout")
return
}This is fine, but consider logging why it timed out for debugging purposes. 🟢 Previous Review Findings - ResolvedAll high-severity issues from the previous Macroscope review have been properly addressed in commit ✅ High: Conversation entry point overwrites saved photo - Fixed by removing 🔒 Security Considerations✅ No secrets or sensitive data exposed 📊 Performance Considerations✅ Efficient state management with local draft pattern 🧪 Test Coverage AssessmentStrong coverage with:
Missing coverage (acceptable for this PR):
📝 Code Quality SummaryAdherence to CLAUDE.md conventions:
SwiftLint compliance: Should pass once linted (no obvious violations) 🎯 RecommendationAPPROVE with minor suggestions. This is a solid implementation that:
The suggested improvements are non-blocking and can be addressed in follow-up work if desired. 💡 Optional Follow-ups
Great work addressing the review feedback comprehensively! 🚀 |
Spacing/type corrections against the Figma dev measurements: 36pt gap between the lava band and the name row (not 64 - the band overhangs its text by 28), 132pt band (40 above / 28 below the text), 56pt CTA with a 16pt horizontal inset inside the card padding, and medium-weight photo/camera glyphs. Dropped an experimental presentationCornerRadius override that made the system inset the sheet content, leaving a white cap above the lava band. Empty state per design: placeholder is "Name" (not "Somebody"), the avatar shows person.crop.circle.fill on an inverted circle until a name is typed (monogram) or a photo is chosen, the terms toggle starts OFF, and the CTA renders gray while disabled. QA: test 01 and 40 now enable the terms toggle before saving; empty state assertions added to 01. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… card The self-sizing detent can settle taller than the measured content, and rubber-banding exposes the area beyond it - both showed the ultra-thin material as a white cap above the lava band. Paint the presentation background lava on top / card gray below so any exposed region reads as the card continuing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The selfSizingSheet approach presents at .medium and swaps to a .height() detent after measuring, but iOS never re-snaps a presented sheet down from medium - content shorter than medium (this sheet, 398pt vs medium's ~425pt) floats centered with dead space, which showed as a white cap above the lava band. ProfileSetupSheet now presents via plain .sheet and supplies its own detent, seeded per mode and corrected by measurement, so the detent only receives same-kind .height updates. Also: camera.fill glyph per design. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Come in is gated only on entering a name; turning the toggle off still disables it. QA tests 01/40 updated (no toggle tap needed before save). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ApprovabilityVerdict: Needs human review This PR introduces a new first-launch profile setup sheet that changes the user onboarding experience. New user-facing features with new components, workflows, and behavioral changes warrant human review. You can customize Macroscope's approvability policy. Learn more. |
- Drop the ConversationView onDismiss handler (and the now-unused onProfileSettingsDismissed): it re-saved from the stale myProfileViewModel, clobbering the just-saved profile photo. Also fixes the SwiftLint multiple_closures_with_trailing_closure failure. - Roll the shared view model back to its pre-save values when saveAndAwait throws, so a failed save doesn't present never-persisted edits as stored. - Reseed a pristine draft when the global profile load resolves, so a sheet opened during cold launch can't save a stale empty draft over the stored avatar. - Allow photo-only saves in edit mode (empty name falls back to the stored one in saveAndAwait). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Review findings addressed in
🤖 Generated with Claude Code |
Mirrors the profile sheet's name row (Figma 5694-28455): 36pt avatar
(person.crop.circle.fill placeholder until a name or photo exists),
display name ("Name" placeholder in tertiary when unset), and a
trailing pencil glyph instead of the old lanyard icon + trailing
name/avatar. A "Your name and pic" footer sits under the row. Tapping
still opens the profile sheet. Accessibility: label stays "My info"
(id my-info-row) with the name exposed as the value; QA test 14
annotated accordingly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
Replaces the in-conversation "Add your name and pic" prompt with a "Hello / My name is" profile setup sheet shown once on first launch (Figma).
First-launch gating
ConversationsViewModel.checkForPairableDeviceIfNeeded). The sheet presents only when no private keys are detected in iCloud (pairableDeviceBackups()returns empty), so it can never race the "Pair device" sheet; deep-link/incoming pairing flows are guarded explicitly.ConversationOnboardingCoordinator) and never shows when a profile already exists.hasShownFirstLaunchProfileSheetflag; cleared by the existingresetUserDefaults()path). Users who dismiss without saving fall back to the legacy in-conversation flow, which is left intact (also covers users who decline pairing).Reused in two more places (My Info variant: no terms row, "Save" CTA)
MyInfoViewform).MyInfoViewis removed.Sheet details (per Figma dev measurements)
photo/camera.fillglyphs.person.crop.circle.fillavatar → live monogram once a name is typed → photo once picked (avatar and photo icon open the library, camera icon the camera).ProfileSettingsViewModelonly on save — direct binding loses in-flight edits when the global-profile observation re-emits during first-launch inbox churn (reproduced before the fix)..sheet+ self-measured.heightdetent): the sharedselfSizingSheetpresents at.mediumand iOS never re-snaps a presented sheet down to a smaller.height, which left dead space above content shorter than medium. That latentselfSizingSheetquirk affects any short sheet and deserves a separate follow-up; this PR leaves the shared modifier untouched.QA
qa/tests/01-onboarding(md + structured yaml) rewritten around the sheet: empty-state assertions, no in-conversation re-prompt, documented dismiss-fallback (01b).14-profile: settings edit via the sheet; part 5 now verifies in-conversation edit updates the global profile.40-dm-members-as-contacts: devices set names via the first-launch sheet;07/19annotated as fallback-only; structured README screen map updated.e28707d89147on the initial build,db4473bb6732on the final design): pass, 8/8 criteria, 0 app/XMTP errors, with the full event chain (found_device_check pairableCount=0→first_launch_profile_sheet_shown→profile_sheet_saved→profile_auto_applied). HTML artifacts inqa/artifacts/(gitignored, shareable as zips).Not sim-driven: camera capture (no sim camera) and the pair-sheet-suppression path (needs an iCloud-keychain fixture; guarded by construction + test 38/44).
🤖 Generated with Claude Code
Note
Add first-launch profile setup sheet for onboarding
ProfileSetupSheetwithfirstLaunchandeditmodes: first-launch shows terms and a 'Come in' CTA; edit mode is a standard profile update sheet.ConversationsViewModelpresents the sheet once viapresentFirstLaunchProfileSetupIfNeeded(), which waits up to 10s for profile load and skips if a profile already exists.MyInfoViewthroughout: the App Settings 'My info' row and in-conversation profile prompt now openProfileSetupSheetin edit mode.ProfileSettingsViewModel.saveAndAwait, reverting on failure; the oldonProfileSettingsDismissedsave path is removed.profile-setup-*accessibility identifiers.Macroscope summarized f6a53ec.