Repository navigation
feat: onboarding spotlight flows, navigation refactor & test coverage - #86
Conversation
…uide - Remove completed items from TODO (document upload, PDF support, image gen fix, resolved test issues) — only future work remains - Add Testing section to README with test commands and codecov badge - Add scripts/test-count.sh to dynamically count tests across all platforms (Jest, Android JUnit, iOS XCTest, Maestro E2E) - Add test:count npm script - Update CODEBASE_GUIDE directory structure to reflect refactored components (ChatInput/, ChatMessage/, checklist/, onboarding/) and screens (HomeScreen/, ChatScreen/, ModelsScreen/) into subdirectories - Add @react-native-documents/picker and viewer to platform services table Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Update CI workflow to count tests across all platforms (Jest, Android JUnit, iOS XCTest) and publish total via schneegans/dynamic-badges-action - Add CI status badge and dynamic test count badge to README Testing section - Remove hardcoded test count, replace with shields.io endpoint badge updated automatically on every push to main - Badge requires GIST_SECRET and TEST_BADGE_GIST_ID to be configured in repo settings Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move all sub-screens (Chat, ProjectDetail, ProjectEdit, ModelSettings, VoiceSettings, DeviceInfo, StorageSettings, SecuritySettings) from nested tab stacks into the RootStack. Tabs now point directly to their screen components instead of wrapping sub-stack navigators. Key changes: - Remove ChatsStack, ProjectsStack, ModelsStack, SettingsStack navigators - Merge all screen params into RootStackParamList, simplify MainTabParamList - Use CompositeNavigationProp for tab-hosted screens navigating to RootStack - Fix SettingsScreen reset onboarding: getParent()?.dispatch() (Tab→RootStack) - Add useFocusEffect to ModelsScreen to reset detail view on tab blur - Collapse (not unmount) ModelsScreen chrome during detail to keep AttachSteps mounted - Add back buttons to ChatScreen's NoModelScreen and LoadingScreen - Move SpotlightTourProvider from MainTabs to RootStack level so all screens (including Chat, ProjectEdit, ModelSettings) can use useSpotlightTour() - Update test mocks for flattened navigation structure Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add 6 new spotlight steps (indices 11-16) and reactive spotlight system: - Flow 2: step 11 highlights first model in picker sheet (animated border) - Flow 3: step 12 chains to voice input hint after ChatInput spotlight - Flow 4: steps 13-16 guide through image gen (load model, new chat, draw, settings) Key changes: - Add shownSpotlights store for one-shot reactive spotlight tracking - Conditional AttachStep mounting (one at a time) to prevent waypoint dots/lines - MaybeAttachStep wrapper avoids ChatInput remounting during step chains - Pulsating border in ModelPickerSheet (AttachStep can't work inside Modal) - triedImageGen completion now requires actual image generation, not just download - 32 new unit tests covering all flows, reactive conditions, and reset behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds 11 test suites covering all 6 onboarding flows across 3 testing layers: Unit tests: - spotlightTooltips: tooltip content matches spec for all 17 steps - handleStepPress: pending spotlight queuing, navigation, timing delays - chatScreenSpotlight: step 3→12 chaining, chainingRef guard, AttachStep mapping - reactiveSpotlightConditions: boolean guards for reactive steps 13-16 - onboardingFlows: step count, shape, render function validation Integration tests: - spotlightFlowIntegration: full lifecycle of all 6 flows, cross-flow interactions, reset behavior, step-to-flow mapping RNTL render-based integration tests: - HomeScreenSpotlight: handleStepPress renders with correct goTo calls, reactive imageLoad spotlight (step 13), 800ms/600ms timing - ChatsListScreenSpotlight: reactive imageNewChat spotlight (step 14) - ChatScreenSpotlight: pending step consumption, step 3→12 chain, reactive imageDraw (step 15) and imageSettings (step 16) - ModelSettingsScreenSpotlight: pending step 6 consumption on mount - ProjectEditScreenSpotlight: pending step 8 consumption on mount Also fixes cross-tab navigation timing (600ms → 800ms) in handleStepPress to allow sheet-close + tab-switch animation to complete before spotlight. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the new user experience by introducing a guided onboarding process with interactive spotlights and a streamlined navigation system. The refactoring simplifies the app's routing, making it more robust and easier to manage, especially for cross-screen interactions. The extensive new test suite ensures the reliability and correctness of these new features and the refactored components, providing a solid foundation for future development. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive onboarding spotlight tour system to guide new users through the application. Key changes include adding a new 'Testing' section to the README.md with CI/CD badges and test commands, and significantly refactoring the TODO.md by removing completed or deprioritized tasks related to document upload and image generation. The core of the PR involves adding extensive new unit and integration tests (__tests__/integration/onboarding/spotlightFlowIntegration.test.ts, __tests__/rntl/onboarding/*.test.tsx, __tests__/unit/onboarding/*.test.ts) to validate the six onboarding flows: downloading a model, loading a model, sending a message, trying image generation, exploring settings, and creating a project. These tests cover multi-step sequences, reactive triggers, state coordination, and tooltip content. The AppNavigator.tsx was updated to wrap the entire application with SpotlightTourProvider, and navigation logic across HomeScreen.test.tsx, ProjectDetailScreen.test.tsx, and SettingsScreen.test.tsx was simplified by flattening nested navigators and directly navigating to screens instead of using nested tab navigation. The ChatInput component now accepts an activeSpotlight prop to conditionally wrap its action button or the entire component with AttachStep for spotlighting. The ModelPickerSheet implements a pulsating border animation for onboarding. The HomeScreen now displays a pulsating icon to indicate pending onboarding steps and includes the OnboardingSheet. The ModelsScreen, ModelSettingsScreen, ProjectEditScreen, and ChatsListScreen were updated to consume pending spotlights on mount and trigger spotlights for their respective onboarding steps. The useAppStore was extended to manage onboardingChecklist flags, shownSpotlights for reactive tracking, and checklistDismissed state, with corresponding reset logic. The imageGenerationService now marks the triedImageGen checklist step as complete upon successful image generation. A new ONBOARDING_FLOWS.md document was added to detail all onboarding flows and their mechanics. A new scripts/test-count.sh script was added to count tests across platforms. Review comments highlighted the brittleness of using hardcoded setTimeout delays for triggering spotlights, suggesting InteractionManager.runAfterInteractions for more reliable timing, and noted an inefficient useEffect in SettingsScreen that causes unnecessary store updates due to an unstable dependency.
| setTimeout(() => { | ||
| step3ShownRef.current = true; | ||
| goTo(3); | ||
| }, 600); | ||
| } else if (pending !== null) { | ||
| setTimeout(() => goTo(pending), 600); | ||
| } | ||
| }, []); // eslint-disable-line react-hooks/exhaustive-deps | ||
|
|
||
| // Track whether we're in the middle of chaining to avoid premature cleanup | ||
| const chainingRef = useRef(false); | ||
|
|
||
| // When the spotlight tour stops after step 3, fire the chained step 12 | ||
| useEffect(() => { | ||
| if (current === undefined && step3ShownRef.current && pendingNextRef.current !== null) { | ||
| step3ShownRef.current = false; | ||
| chainingRef.current = true; | ||
| const next = pendingNextRef.current; | ||
| pendingNextRef.current = null; | ||
| // Switch AttachStep index — need time for new AttachStep to mount + measure layout | ||
| setChatSpotlight(next); | ||
| setTimeout(() => { | ||
| chainingRef.current = false; | ||
| goTo(next); | ||
| }, 800); | ||
| } else if (current === undefined && !chainingRef.current && !step3ShownRef.current && pendingNextRef.current === null) { | ||
| // Tour stopped and no chain pending — clear spotlight | ||
| setChatSpotlight(null); | ||
| } | ||
| }, [current, goTo]); | ||
|
|
||
| // Reactive: image model loaded, no image generated yet → spotlight ChatInput (step 15) | ||
| useEffect(() => { | ||
| if ( | ||
| chat.imageModelLoaded && | ||
| !shownSpotlights.imageDraw && | ||
| !onboardingChecklist.triedImageGen | ||
| ) { | ||
| markSpotlightShown('imageDraw'); | ||
| setChatSpotlight(IMAGE_DRAW_STEP_INDEX); | ||
| setTimeout(() => goTo(IMAGE_DRAW_STEP_INDEX), 800); | ||
| } | ||
| }, [chat.imageModelLoaded, shownSpotlights, onboardingChecklist.triedImageGen, markSpotlightShown, goTo]); | ||
|
|
||
| // Reactive: after first image generated → spotlight image mode toggle (step 16) | ||
| const generatedImages = useAppStore(s => s.generatedImages); | ||
| useEffect(() => { | ||
| if ( | ||
| generatedImages.length > 0 && | ||
| !shownSpotlights.imageSettings && | ||
| onboardingChecklist.triedImageGen | ||
| ) { | ||
| markSpotlightShown('imageSettings'); | ||
| setChatSpotlight(IMAGE_SETTINGS_STEP_INDEX); | ||
| setTimeout(() => goTo(IMAGE_SETTINGS_STEP_INDEX), 800); | ||
| } |
There was a problem hiding this comment.
The use of hardcoded setTimeout delays (600ms, 800ms) to trigger spotlights is brittle and can lead to race conditions on different devices or if animation timings change. This pattern is repeated for consuming pending spotlights and for reactive spotlights.
A more robust approach would be to use InteractionManager.runAfterInteractions to ensure animations and layout changes have completed before calling goTo(). This would make the spotlight timing more reliable.
For example:
useEffect(() => {
const pending = consumePendingSpotlight();
if (pending !== null) {
InteractionManager.runAfterInteractions(() => {
goTo(pending);
});
}
}, [goTo]);This would apply to all setTimeout(() => goTo(...), ...) calls in this file.
There was a problem hiding this comment.
Fixed in commit cd9cb79 — replaced setTimeout with InteractionManager.runAfterInteractions() in ChatScreen for both pending spotlight consumption and reactive step 16. The remaining setTimeout calls in HomeScreen's handleStepPress are intentional — they coordinate with the sheet-close + tab-switch animation timing and fire from a user-initiated callback (not a lifecycle hook), so InteractionManager wouldn't help there.
| useEffect(() => { | ||
| completeChecklistStep('exploredSettings'); | ||
| }, [completeChecklistStep]); |
There was a problem hiding this comment.
This useEffect will run on every render because the completeChecklistStep function reference from the Zustand store is not stable. While this is idempotent, it's inefficient and causes unnecessary store updates on every render of this screen.
To fix this and ensure the effect runs only once on mount, you can use an empty dependency array and disable the lint rule for that line, as the function's identity is not important for this effect's logic.
| useEffect(() => { | |
| completeChecklistStep('exploredSettings'); | |
| }, [completeChecklistStep]); | |
| useEffect(() => { | |
| completeChecklistStep('exploredSettings'); | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, []); |
There was a problem hiding this comment.
Already fixed in a previous commit — the useEffect now has an empty dependency array with an eslint-disable comment, so it only runs once on mount.
- Add Step 17: spotlight first recommended image model card for download
- Make handleStepPress('triedImageGen') state-aware (3 branches: model loaded, downloaded, or missing)
- Skip image model auto-load during onboarding so reactive spotlights can fire
- Fix Step 14 tooltip missing by using multi-index AttachStep (index={[2, 14]})
- Fix Step 15 not firing by switching from reactive useFocusEffect to setPendingSpotlight mechanism
- Fix Step 16 pointing at ChatInput instead of settings icon in header
- Disable "Try image generation" checklist step until text model is loaded
- Replace setTimeout with InteractionManager.runAfterInteractions for spotlight consumers
- Fix unstable completeChecklistStep dependency in SettingsScreen
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unused react-native-onboarding-checklist file dependency that broke CI (ENOENT on tgz path that only exists locally) - Replace setTimeout with InteractionManager.runAfterInteractions for Step 16 reactive spotlight in ChatScreen (no cleanup to avoid race condition where markSpotlightShown re-triggers the effect) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This is an impressive pull request that delivers a comprehensive onboarding spotlight system, a significant navigation refactor, and extensive test coverage. The new onboarding flows are well-designed, using a combination of immediate and reactive spotlights to guide users through the app's core features. The navigation refactor to flatten the stack is a smart architectural change that enables the global SpotlightTourProvider to work seamlessly across all screens. The test coverage for this new functionality is excellent, with thorough unit, integration, and RNTL tests that validate the complex logic. I found one minor UI regression where the image count was removed from the gallery card, which I've flagged for review. Overall, this is a high-quality contribution that greatly enhances the new user experience.
| <Text style={styles.galleryCardTitle}>Image Gallery</Text> | ||
| <Text style={styles.galleryCardMeta}> | ||
| {generatedImages.length} image{generatedImages.length !== 1 ? 's' : ''} | ||
| {generatedImages.length === 1 ? 'image' : 'images'} |
There was a problem hiding this comment.
The number of generated images is no longer displayed on this card, which seems like a minor regression. It's helpful for the user to see the count at a glance. I suggest re-adding the count to the text.
| {generatedImages.length === 1 ? 'image' : 'images'} | |
| {generatedImages.length} {generatedImages.length === 1 ? 'image' : 'images'} |
There was a problem hiding this comment.
Fixed in commit 7662470 — the gallery card now shows {count} {image|images} (e.g. '2 images', '1 image').
- Remove unused react-native-onboarding-checklist file dependency (CI fix) - Replace setTimeout with InteractionManager for Step 16 spotlight - Move inline styles to StyleSheet (ChatInput, OnboardingSheet, ModelsScreen) - Reduce ChatsListScreen line count below max-lines limit - Compact handleStepPress pending map to reduce HomeScreen function length - Remove unused createONNXImageModel import from test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ression - Extract shared test mocks into spotlightMocks.tsx to reduce duplicated lines - Replace if-chain with pendingMap lookup in handleStepPress to reduce cognitive complexity - Remove non-null assertion in HomeScreen STEP_INDEX_MAP access - Restore image count display on gallery card (was showing only "images" without count) - Sync handleStepPress test with actual implementation (missing setPendingSpotlight for step 15) - Update HomeScreen tests for new count format Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This is an impressive and substantial pull request that delivers a complete onboarding spotlight system, flattens the app's navigation architecture, and adds comprehensive test coverage. The navigation refactor is a great architectural improvement that simplifies the codebase and enables cross-tab features like the spotlight tour. The onboarding flows are well-designed, with a good mix of chained and reactive steps to guide the user effectively. The addition of over 175 new tests across 11 suites demonstrates a strong commitment to quality and maintainability. I have one suggestion for refactoring and have identified one potential bug to ensure the spotlight logic is robust across all scenarios. Overall, this is an excellent contribution.
| useEffect(() => { | ||
| const pending = consumePendingSpotlight(); | ||
| if (pending === 3) { | ||
| // Chain: step 3 (ChatInput) → step 12 (VoiceRecordButton) | ||
| pendingNextRef.current = VOICE_HINT_STEP_INDEX; | ||
| step3ShownRef.current = false; | ||
| const task = InteractionManager.runAfterInteractions(() => { | ||
| step3ShownRef.current = true; | ||
| goTo(3); | ||
| }); | ||
| return () => task.cancel(); | ||
| } else if (pending !== null) { | ||
| const task = InteractionManager.runAfterInteractions(() => goTo(pending)); | ||
| return () => task.cancel(); | ||
| } | ||
| }, []); // eslint-disable-line react-hooks/exhaustive-deps | ||
|
|
||
| // Track whether we're in the middle of chaining to avoid premature cleanup | ||
| const chainingRef = useRef(false); | ||
|
|
||
| // When the spotlight tour stops after step 3, fire the chained step 12 | ||
| useEffect(() => { | ||
| if (current === undefined && step3ShownRef.current && pendingNextRef.current !== null) { | ||
| step3ShownRef.current = false; | ||
| chainingRef.current = true; | ||
| const next = pendingNextRef.current; | ||
| pendingNextRef.current = null; | ||
| // Switch AttachStep index — need time for new AttachStep to mount + measure layout | ||
| setChatSpotlight(next); | ||
| setTimeout(() => { | ||
| chainingRef.current = false; | ||
| goTo(next); | ||
| }, 800); | ||
| } else if (current === undefined && !chainingRef.current && !step3ShownRef.current && pendingNextRef.current === null) { | ||
| // Tour stopped and no chain pending — clear spotlight | ||
| setChatSpotlight(null); | ||
| } | ||
| }, [current, goTo]); | ||
|
|
||
| // Consume pending spotlights on focus (handles reused screen instances where | ||
| // the mount-only useEffect above won't re-fire after navigation). | ||
| useFocusEffect( | ||
| useCallback(() => { | ||
| const pending = consumePendingSpotlight(); | ||
| if (pending !== null) { | ||
| const task = InteractionManager.runAfterInteractions(() => goTo(pending)); | ||
| return () => task.cancel(); | ||
| } | ||
| }, [goTo]), | ||
| ); |
There was a problem hiding this comment.
There are two separate effects consuming pending spotlights: a useEffect on mount and a useFocusEffect. This can lead to inconsistent behavior, as the special chaining logic for pending === 3 is only present in the mount effect. If the screen is re-focused with pending === 3, the useFocusEffect will handle it but won't set up the chain to step 12.
To ensure consistent behavior, it's best to consolidate all spotlight consumption logic into a single useFocusEffect, which runs on both initial mount and subsequent focuses.
| useEffect(() => { | |
| const pending = consumePendingSpotlight(); | |
| if (pending === 3) { | |
| // Chain: step 3 (ChatInput) → step 12 (VoiceRecordButton) | |
| pendingNextRef.current = VOICE_HINT_STEP_INDEX; | |
| step3ShownRef.current = false; | |
| const task = InteractionManager.runAfterInteractions(() => { | |
| step3ShownRef.current = true; | |
| goTo(3); | |
| }); | |
| return () => task.cancel(); | |
| } else if (pending !== null) { | |
| const task = InteractionManager.runAfterInteractions(() => goTo(pending)); | |
| return () => task.cancel(); | |
| } | |
| }, []); // eslint-disable-line react-hooks/exhaustive-deps | |
| // Track whether we're in the middle of chaining to avoid premature cleanup | |
| const chainingRef = useRef(false); | |
| // When the spotlight tour stops after step 3, fire the chained step 12 | |
| useEffect(() => { | |
| if (current === undefined && step3ShownRef.current && pendingNextRef.current !== null) { | |
| step3ShownRef.current = false; | |
| chainingRef.current = true; | |
| const next = pendingNextRef.current; | |
| pendingNextRef.current = null; | |
| // Switch AttachStep index — need time for new AttachStep to mount + measure layout | |
| setChatSpotlight(next); | |
| setTimeout(() => { | |
| chainingRef.current = false; | |
| goTo(next); | |
| }, 800); | |
| } else if (current === undefined && !chainingRef.current && !step3ShownRef.current && pendingNextRef.current === null) { | |
| // Tour stopped and no chain pending — clear spotlight | |
| setChatSpotlight(null); | |
| } | |
| }, [current, goTo]); | |
| // Consume pending spotlights on focus (handles reused screen instances where | |
| // the mount-only useEffect above won't re-fire after navigation). | |
| useFocusEffect( | |
| useCallback(() => { | |
| const pending = consumePendingSpotlight(); | |
| if (pending !== null) { | |
| const task = InteractionManager.runAfterInteractions(() => goTo(pending)); | |
| return () => task.cancel(); | |
| } | |
| }, [goTo]), | |
| ); | |
| useFocusEffect( | |
| useCallback(() => { | |
| const pending = consumePendingSpotlight(); | |
| if (pending === 3) { | |
| // Chain: step 3 (ChatInput) → step 12 (VoiceRecordButton) | |
| pendingNextRef.current = VOICE_HINT_STEP_INDEX; | |
| step3ShownRef.current = false; | |
| const task = InteractionManager.runAfterInteractions(() => { | |
| step3ShownRef.current = true; | |
| goTo(3); | |
| }); | |
| return () => task.cancel(); | |
| } else if (pending !== null) { | |
| const task = InteractionManager.runAfterInteractions(() => goTo(pending)); | |
| return () => task.cancel(); | |
| } | |
| }, [goTo]), | |
| ); |
There was a problem hiding this comment.
The mount-only useEffect handles the step 3→12 chain (which requires tracking current and pendingNextRef). The useFocusEffect handles all other pending spotlights including step 15 (imageDraw) which arrives via setPendingSpotlight from other screens. Step 3 will always arrive on mount (first navigation to ChatScreen), not on re-focus, so the separation is intentional. If step 3 somehow arrived via re-focus, it would still fire correctly — it just wouldn't chain to step 12, which is acceptable since the chain is a nice-to-have UX touch.
| const pendingMap: Record<string, number> = { | ||
| downloadedModel: DOWNLOAD_FILE_STEP_INDEX, loadedModel: MODEL_PICKER_STEP_INDEX, | ||
| sentMessage: CHAT_INPUT_STEP_INDEX, exploredSettings: MODEL_SETTINGS_STEP_INDEX, | ||
| createdProject: PROJECT_EDIT_STEP_INDEX, | ||
| }; |
There was a problem hiding this comment.
For better performance and code clarity, the pendingMap object should be defined as a constant outside the HomeScreen component. Since its value is static and does not depend on component props or state, there's no need to recreate it on every render within the useCallback hook.
const PENDING_MAP: Record<string, number> = {
downloadedModel: DOWNLOAD_FILE_STEP_INDEX,
loadedModel: MODEL_PICKER_STEP_INDEX,
sentMessage: CHAT_INPUT_STEP_INDEX,
exploredSettings: MODEL_SETTINGS_STEP_INDEX,
createdProject: PROJECT_EDIT_STEP_INDEX,
};
export const HomeScreen: React.FC<HomeScreenProps> = ({ navigation }) => {
// ... existing component code ...
const handleStepPress = useCallback((stepId: string) => {
closeSheet();
// ... existing logic for 'triedImageGen' ...
const tab = STEP_TAB_MAP[stepId];
const stepIndex = STEP_INDEX_MAP[stepId];
// For multi-step flows, queue the continuation step.
if (PENDING_MAP[stepId] !== undefined) {
setPendingSpotlight(PENDING_MAP[stepId]);
}
// ... rest of the function ...
}, [/* ... existing dependencies ... */]);
// ... rest of the component ...
};
There was a problem hiding this comment.
Good point — however the pendingMap is already inside a useCallback so it's only created when the callback is invoked (on user press), not on every render. Moving it outside the component would be a micro-optimization with no measurable impact. Keeping it close to its usage site aids readability.
| // Device info | ||
| onboardingChecklist: OnboardingChecklist; | ||
| checklistDismissed: boolean; | ||
| completeChecklistStep: (key: string) => void; |
There was a problem hiding this comment.
To improve type safety and prevent potential runtime errors from typos, the key parameter for completeChecklistStep should be strongly typed as keyof OnboardingChecklist instead of a generic string.
| completeChecklistStep: (key: string) => void; | |
| completeChecklistStep: (key: keyof OnboardingChecklist) => void; |
There was a problem hiding this comment.
Agreed — will address in a follow-up PR. The current string type works correctly at runtime and all call sites use valid keys. Narrowing to keyof OnboardingChecklist is a good improvement for IDE autocompletion.
| hasSeenCacheTypeNudge: boolean; | ||
| setHasSeenCacheTypeNudge: (v: boolean) => void; | ||
| shownSpotlights: Record<string, boolean>; | ||
| markSpotlightShown: (key: string) => void; |
There was a problem hiding this comment.
For better type safety, the key for markSpotlightShown should be a specific string literal union type rather than a generic string. This will prevent invalid keys from being used and provide better autocompletion in the IDE.
Consider defining a type like type SpotlightKey = 'imageLoad' | 'imageNewChat' | 'imageDraw' | 'imageSettings'; and using it here.
| markSpotlightShown: (key: string) => void; | |
| markSpotlightShown: (key: 'imageLoad' | 'imageNewChat' | 'imageDraw' | 'imageSettings') => void; |
There was a problem hiding this comment.
Same as above — will narrow the type in a follow-up. All current call sites use valid keys.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
1 participant
Summary
This PR delivers the complete onboarding spotlight system with reactive spotlights, flattens the app's navigation architecture, and adds comprehensive test coverage (175 new tests across 11 suites).
Key changes:
Navigation refactor: Flatten all sub-stack navigators (ChatsStack, ProjectsStack, ModelsStack, SettingsStack) into the RootStack. Tabs now point directly to screen components, eliminating nested navigation complexity and enabling SpotlightTour to work across all screens.
Expanded onboarding flows: Add spotlight steps 11–16 covering model picker highlight (Flow 2), voice input hint chaining (Flow 3), and full image generation guidance (Flow 4: load model → new chat → draw → settings). Includes a reactive spotlight system with
shownSpotlightstracking for one-shot contextual hints.Comprehensive tests: 11 new test suites spanning unit tests (tooltip content, step press handling, chat screen chaining, reactive conditions, flow validation), integration tests (full flow lifecycle, cross-flow interactions, reset behavior), and RNTL render-based tests (spotlight rendering on HomeScreen, ChatsListScreen, ChatScreen, ModelSettingsScreen, ProjectEditScreen).
CI & docs: Dynamic test count badge via CI, test summary table in README, updated codebase guide, cleaned up TODO.
Type of Change
Screenshots / Screen Recordings
N/A — spotlight system is overlay-based and tested via automated tests. Navigation refactor is structural with no visual change.
Checklist
General
Testing
npm test)React Native Specific
SPACING/TYPOGRAPHYconstants from the themeuseThemedStylespattern (not inline or staticStyleSheet.create)Related Issues
Follow-up: Flow 4 image model download spotlight fix will be addressed in a separate PR.
Additional Notes
MaybeAttachStepwrapper prevents ChatInput remounting during spotlight step chainsAttachStepmounting (one at a time) prevents waypoint dots/lines artifactstriedImageGencompletion now requires actual image generation, not just download