-
Notifications
You must be signed in to change notification settings - Fork 50
feat(web): remove auth requirement to simplify onboarding #1404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Add tests for task-seeding.util - Add tests for CmdPaletteTutorial component - Add tests for OnboardingOverlay component - Add tests for AuthPrompt component - Add tests for useOnboardingOverlays hook
- Only seed tasks when not in test environment - Prevents seeded tasks from interfering with existing tests
- Fix keyboard event simulation in CmdPaletteTutorial test - Dispatch event to window instead of document - Properly set modifier key properties for cross-platform support
…rule - Check if COUNT is specified before adding extra dtstart instance - Improve timezone comparison to handle precision issues - Fixes test failure where 707 instances were generated instead of 706
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR simplifies the onboarding experience by allowing unauthenticated users to access the day view and interact with the application before signing up. The implementation introduces a progressive disclosure pattern with contextual onboarding overlays that guide users through key features.
- Adds three progressive onboarding components (OnboardingOverlay, CmdPaletteTutorial, AuthPrompt) that appear based on user interactions
- Enables guest access to the day view with task seeding for new users
- Updates routing logic to allow unauthenticated access to specific routes for users who haven't completed signup
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web/src/views/Day/view/DayViewContent.tsx | Integrates onboarding overlay system with date navigation tracking |
| packages/web/src/views/Day/hooks/tasks/useTaskEffects.ts | Adds comment noting task seeding happens in loader |
| packages/web/src/views/Day/hooks/onboarding/useOnboardingOverlays.ts | New hook managing the display logic and timing for three onboarding overlays |
| packages/web/src/views/Day/hooks/onboarding/useOnboardingOverlays.test.tsx | Comprehensive test suite for onboarding overlay hook |
| packages/web/src/views/Day/components/OnboardingOverlay/OnboardingOverlay.tsx | Welcome overlay component introducing users to the app |
| packages/web/src/views/Day/components/OnboardingOverlay/OnboardingOverlay.test.tsx | Tests for welcome overlay component |
| packages/web/src/views/Day/components/CmdPaletteTutorial/CmdPaletteTutorial.tsx | Tutorial overlay teaching users about the command palette |
| packages/web/src/views/Day/components/CmdPaletteTutorial/CmdPaletteTutorial.test.tsx | Tests for command palette tutorial |
| packages/web/src/views/Day/components/AuthPrompt/AuthPrompt.tsx | Prompt encouraging users to sign in after demonstrating engagement |
| packages/web/src/views/Day/components/AuthPrompt/AuthPrompt.test.tsx | Tests for authentication prompt |
| packages/web/src/socket/SocketProvider.tsx | Adds authentication check to conditionally connect socket |
| packages/web/src/routers/loaders.ts | Updates loaders to allow unauthenticated access to day view and seed initial tasks |
| packages/web/src/components/GuestLayout/GuestLayout.tsx | New layout component for unauthenticated users with global shortcuts |
| packages/web/src/common/utils/storage/task-seeding.util.ts | Utility to seed initial sample tasks for new users |
| packages/web/src/common/utils/storage/task-seeding.util.test.ts | Tests for task seeding utility |
| packages/web/src/common/constants/storage.constants.ts | Adds storage keys for onboarding state persistence |
| packages/web/src/auth/UserProvider.tsx | Updates to handle unauthenticated users and suppress expected errors |
| packages/backend/src/event/classes/gcal.event.rrule.ts | Improves recurring event date handling with more lenient timezone comparison |
packages/web/src/views/Day/hooks/onboarding/useOnboardingOverlays.ts
Outdated
Show resolved
Hide resolved
packages/web/src/views/Day/hooks/onboarding/useOnboardingOverlays.ts
Outdated
Show resolved
Hide resolved
packages/web/src/views/Day/hooks/onboarding/useOnboardingOverlays.ts
Outdated
Show resolved
Hide resolved
packages/web/src/views/Day/hooks/onboarding/useOnboardingOverlays.ts
Outdated
Show resolved
Hide resolved
- Extract useOnboardingOverlay hook with tests - Extract useCmdPaletteTutorial hook with tests - Extract useAuthPrompt hook with tests - Refactor useOnboardingOverlays to compose the three hooks - Add comprehensive tests for each hook - Fix timing issues in tests with fake timers and act
- Fix useOnboardingOverlay test by resetting mock before test - Fix useCmdPaletteTutorial test by using proper waitFor with real timers - Fix OnboardingOverlay component tests to match actual component text - All tests now passing
- Add CmdPaletteGuide component to provide step-by-step instructions for using the command palette. - Integrate guide into DayViewContent and NowView components, displaying it conditionally based on the current view. - Create custom hooks for managing guide state and detecting completion of each step. - Update storage constants to track completion status of the command palette guide. - Refactor OnboardingFlow to redirect to the day view upon completion, enhancing user experience.
- Add tests for CmdPaletteGuide component to validate rendering and functionality across different steps. - Implement tests for useCmdPaletteGuide, useStep1Detection, useStep2Detection, and useStep3Detection hooks to ensure correct behavior during onboarding. - Include checks for task creation, navigation detection, and description editing to enhance user experience during onboarding. - Ensure all new tests are passing and cover various scenarios for the command palette guide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 38 out of 38 changed files in this pull request and generated 13 comments.
packages/web/src/views/Day/components/OnboardingOverlay/OnboardingOverlay.tsx
Outdated
Show resolved
Hide resolved
packages/web/src/views/Day/components/OnboardingOverlay/OnboardingOverlay.tsx
Outdated
Show resolved
Hide resolved
packages/web/src/views/Day/components/OnboardingOverlay/OnboardingOverlay.tsx
Outdated
Show resolved
Hide resolved
packages/web/src/views/Day/hooks/onboarding/useCmdPaletteTutorial.test.tsx
Outdated
Show resolved
Hide resolved
packages/web/src/views/Onboarding/components/CmdPaletteGuide.test.tsx
Outdated
Show resolved
Hide resolved
packages/web/src/views/Onboarding/hooks/useCmdPaletteGuide.test.ts
Outdated
Show resolved
Hide resolved
packages/web/src/views/Onboarding/hooks/useStep1Detection.test.ts
Outdated
Show resolved
Hide resolved
packages/web/src/views/Onboarding/hooks/useStep3Detection.test.ts
Outdated
Show resolved
Hide resolved
…letteGuide - Deleted the OnboardingOverlay component and its associated tests to streamline onboarding functionality. - Updated the useOnboardingOverlay hook to manage visibility based on the CmdPaletteGuide's state. - Refactored DayViewContent and NowView components to utilize CmdPaletteGuide for onboarding instructions. - Adjusted local storage handling to track completion status of the command palette guide instead of the onboarding overlay. - Enhanced tests for onboarding overlays to reflect the new structure and ensure correct behavior across different user scenarios.
- Introduced new utility functions for managing completed steps in localStorage, including marking steps as completed, loading completed steps, and clearing them. - Updated the CmdPaletteGuide component and associated hooks to utilize the new step completion tracking, improving the onboarding experience. - Refactored step detection hooks to skip detection if a step is already completed, optimizing performance and user experience. - Added comprehensive tests for the new onboarding storage utilities to ensure correct functionality and data integrity. - Migrated existing completion flags to the new storage structure, ensuring backward compatibility.
…s tracking - Introduced a new schema for onboarding progress using Zod, consolidating storage management into a single structure. - Updated utility functions to handle onboarding progress, including getting and updating progress, ensuring data integrity. - Refactored components and hooks to utilize the new onboarding progress structure, replacing localStorage interactions with centralized updates. - Enhanced tests to validate the new onboarding progress functionality and ensure correct behavior across various scenarios. - Removed deprecated localStorage keys and migration logic, streamlining the onboarding experience.
- Refactored CmdPaletteGuide component to show dynamic welcome messages based on the current view (Day View or Now View).
- Updated tests to validate the correct rendering of welcome messages and ensure that the previous default message ("Welcome to Compass") is no longer displayed.
- Enhanced test coverage for different views to confirm the onboarding experience aligns with user expectations.
…experience - Added support for a new step (Step 4) in the CmdPaletteGuide component, providing users with additional instructions for editing reminders. - Updated the onboarding logic to accommodate the new step, including modifications to hooks and utility functions for step completion tracking. - Enhanced tests to validate the correct functionality of the new step and ensure comprehensive coverage of the onboarding process. - Adjusted progress indicators and messages to reflect the addition of the fourth step, improving user guidance throughout the onboarding experience.
…arity - Updated onboarding logic to utilize ONBOARDING_STEPS constants for step tracking, enhancing code readability and maintainability. - Refactored related hooks and components to replace numeric step identifiers with descriptive constants, ensuring consistency across the onboarding experience. - Enhanced tests to validate the new constant-based approach, confirming correct functionality and step completion tracking. - Adjusted utility functions for onboarding progress to align with the new structure, improving data integrity and clarity in step management.
…detection - Introduced new onboarding steps for CmdPaletteGuide, including steps 5 and 6, to guide users through using the command palette and navigating to the week view. - Updated related hooks and components to manage the new steps, ensuring a cohesive onboarding experience. - Enhanced utility functions for onboarding progress tracking, incorporating new steps into the existing structure. - Added comprehensive tests for the new steps and detection logic, validating correct functionality and user guidance throughout the onboarding process. - Refactored existing tests to accommodate the changes and ensure robust coverage of the onboarding experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 47 out of 47 changed files in this pull request and generated 5 comments.
packages/web/src/views/Day/hooks/onboarding/useOnboardingOverlay.test.tsx
Outdated
Show resolved
Hide resolved
packages/web/src/views/Day/hooks/onboarding/useCmdPaletteTutorial.test.tsx
Outdated
Show resolved
Hide resolved
packages/web/src/views/Onboarding/components/CmdPaletteGuide.test.tsx
Outdated
Show resolved
Hide resolved
packages/web/src/views/Onboarding/hooks/useStep1Detection.test.ts
Outdated
Show resolved
Hide resolved
packages/web/src/views/Onboarding/hooks/useStep3Detection.test.ts
Outdated
Show resolved
Hide resolved
…tep detection - Renamed onboarding storage utility functions for clarity and consistency, transitioning from `onboardingStorage.util` to `onboarding.storage.util`. - Consolidated step detection logic into a unified hook, enhancing maintainability and reducing redundancy across onboarding steps. - Updated related components and tests to reflect the new utility structure, ensuring comprehensive coverage and functionality. - Removed deprecated step detection hooks, streamlining the onboarding process and improving performance. - Enhanced tests for onboarding storage utilities to validate the new structure and ensure correct behavior across various scenarios.
- Adjusted step counts in CmdPaletteGuide tests and component to reflect the correct total of 5 steps instead of 6. - Removed the deprecated CMD_PALETTE_INFO step from onboarding constants and related logic, streamlining the onboarding process. - Updated tests to ensure accurate step completion tracking and validate the new structure.
…ccess message handling - Added CmdPaletteGuide component to CalendarView for improved onboarding experience. - Updated CmdPaletteGuide to manage success message visibility based on user interaction, allowing users to dismiss the message. - Enhanced step detection logic to prevent duplicate completions, ensuring accurate onboarding progress tracking.
…ance onboarding interactions - Added CmdPaletteGuide component to AuthenticatedLayout to improve user onboarding experience across authenticated routes. - Updated CmdPalette interactions in Calendar, Day, and Now views to reset onboarding progress and dispatch a restart event when "Re-do onboarding" is clicked. - Refactored related components and tests to ensure consistent onboarding behavior and validate the new functionality. - Enhanced onboarding storage utilities to support resetting progress, ensuring a seamless user experience during onboarding.
… messaging - Removed manual setting of syncing state in useGoogleAuth, now handled by SocketProvider upon import completion. - Updated SyncEventsOverlay text to clarify the event import process and user expectations during syncing. - Enhanced SocketProvider to display success messages and reload the page after successful event synchronization.
…ontext - Introduced a new SessionProvider component to manage authentication state and session lifecycle using SuperTokens. - Created a CompassSession interface to define session properties and methods for state management. - Updated imports to reflect new file structure for session-related components. - Added tests for Google authentication and session management to ensure reliability and proper functionality.
…tication state utilities - Eliminated console log statements from the event repository utility to reduce noise in the console output. - Removed logging from authentication state functions to streamline the code and enhance performance. - Updated tests to utilize a new CompassSession interface for improved session management consistency across components.
- Updated createAxiosError to accept an optional URL parameter for better error context. - Modified triggerErrorResponse to handle the URL parameter, allowing for more specific error handling. - Adjusted CompassApi response interceptor to gracefully handle 404 errors for /user/profile without triggering a sign-out or redirect, enabling UserProvider to manage the response appropriately. - Added a test case to verify that a 404 response for /user/profile does not invoke sign-out or redirection.
… mount - Introduced a new test case to ensure that localStorage is not overwritten with an empty array when the TaskProvider mounts, addressing a regression issue. - Updated the useTaskEffects hook to manage loading state more effectively, ensuring tasks are loaded correctly from localStorage without losing existing data. - Enhanced the logic to synchronize loaded state and prevent unnecessary overwrites during component re-renders.
…rror handling - Eliminated the logDatabaseOperation function from db-errors.util.ts to reduce console noise and improve performance. - Updated handleDatabaseError to remove console error logging, focusing on error handling without unnecessary output. - Adjusted event storage utility functions to remove calls to logDatabaseOperation, ensuring cleaner code and better management of database operations.
- Updated the startGoogleCalendarSync method to return the count of imported events and calendars. - Introduced CalendarImportCompleteModal to display import results and a dismiss option. - Added CalendarImportOverlay to indicate ongoing import processes. - Enhanced SocketProvider to handle import completion and trigger state updates without page reloads. - Updated Redux slice to manage importing state and results effectively. - Integrated new UI components into AuthenticatedLayout for improved user feedback during calendar imports.
- Integrated toast notifications to inform users when offline storage is unavailable during app initialization. - Updated the database initialization process to handle specific errors more gracefully, allowing the app to function in remote-only mode if local storage fails. - Removed unnecessary console logging from various storage and event repository functions to reduce console noise and improve performance. - Added null handling for import results in the Redux state to ensure consistency across the application.
- Refactored database initialization to use a new method that handles errors more effectively, allowing the app to continue functioning in remote-only mode if local storage fails. - Integrated a toast notification system to inform users of offline storage issues after app initialization. - Added unit tests for the new error handling and toast notification functions to ensure reliability and proper user feedback.
…ructure - Updated tests in SyncController to check that the import result is a JSON string containing properties for eventsCount and calendarsCount. - Refactored assertions to ensure proper handling of import results and improved test clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 159 out of 161 changed files in this pull request and generated 7 comments.
| endDate: string, | ||
| ) => { | ||
| return events.filter((event) => { | ||
| const safeEvents = Array.isArray(events) ? events : []; |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added defensive check for events array, but this suggests there might be a type safety issue upstream where events could be undefined or null. Consider fixing the type definition or caller to ensure events is always an array.
- Renamed PORT to WEB_PORT in playwright.config.ts for clarity. - Modified webpack.config.mjs to use WEB_PORT from environment variables, defaulting to "9080" if not set, ensuring consistent port usage across configurations.
| events = events.filter((event) => { | ||
| if (!event.startDate) return false; | ||
| const eventStart = dayjs(event.startDate); | ||
| return eventStart.isBetween(start, end, "day", "[]"); // inclusive on both ends |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multi-day events excluded by incorrect date filtering
High Severity
The loadEventsFromIndexedDB function only checks if an event's startDate is within the query range using eventStart.isBetween(start, end, "day", "[]"). This excludes multi-day events that start before the range but extend into it. For example, an event from Jan 1-5 would be missing when querying Jan 3-10. The existing EventInViewParser.isEventInView() correctly handles events that end within the view or span across it, but this new local storage filter does not, causing inconsistent behavior between LocalEventRepository and RemoteEventRepository.
…t management - Introduced a shared timeout constant for form operations to standardize wait times across tests. - Added a `retryUntil` function to consolidate retry patterns for actions until a specified condition is met, improving test reliability. - Refactored existing functions to utilize the new retry logic and ensure form elements are visible before interactions. - Enhanced keyboard shortcut dispatching with additional event properties for better simulation of user actions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 159 out of 161 changed files in this pull request and generated no new comments.
| <CmdPaletteGuide /> | ||
| <SyncEventsOverlay /> | ||
|
|
||
| {isImporting && <CalendarImportOverlay />} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate import overlays render simultaneously during sync
Medium Severity
During Google Calendar import, both SyncEventsOverlay and CalendarImportOverlay render at the same time. In useGoogleAuth.ts, both setIsSyncing(true) and dispatch(importGCalSlice.actions.importing(true)) are called together. Since isImporting = isSyncing || importing, and SyncEventsOverlay also renders when isSyncing is true, two full-screen backdrop-blur overlays appear simultaneously (z-50 and z-999), causing compounded visual blur effects and redundant UI elements.
Additional Locations (1)
| // For local repository, we just save the updated event | ||
| // The applyTo parameter is not relevant for local storage | ||
| await saveEventToIndexedDB(event as Event_Core); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LocalEventRepository.edit ignores the _id parameter
Low Severity
The edit method accepts an _id parameter but ignores it entirely, instead using event._id from the event object via saveEventToIndexedDB. This creates inconsistent behavior with RemoteEventRepository.edit which uses the _id parameter. If a caller passes _id: "abc" but the event has _id: "xyz", the original event "abc" remains unchanged while "xyz" is created or updated, potentially causing silent data inconsistency.
…agement - Introduced UserContext to manage user-related state and loading status. - Created UserProvider to fetch user profile data and integrate with PostHog for user identification. - Added tests for UserProvider to ensure correct behavior during user data loading and PostHog integration. - Implemented hooks for checking signup completion and skipping onboarding, enhancing user experience.
- Added useSession hook to provide easy access to session context within the application. - Updated various components and tests to utilize the new useSession hook, ensuring consistent session management across the codebase. - Refactored imports to replace previous useSession references with the new hook from the auth module.
… tests - Introduced CalendarImportCompleteModal to display import results for events and calendars, with customizable messages based on counts. - Implemented accessibility features and auto-dismiss functionality after 8 seconds. - Added comprehensive tests to ensure correct rendering and behavior of the modal, including dismiss actions and event handling. - Refactored event handling in the modal to improve user interaction experience.
- Deleted CheckBox component implementation, index, and styled files to streamline the codebase. - This change simplifies the component structure and removes unused code.
- Enhanced error handling in the createEvent and getSomedayEvents sagas to log errors when the database does not exist, providing better visibility during testing. - Updated import statements in saga utilities to use the correct redux-saga core effects module, ensuring compatibility and clarity in the codebase. - Refactored the editEvent saga to improve type safety and maintainability.
- Updated sagas in someday.sagas.ts to improve type safety by explicitly defining return types for generator functions. - Simplified action handling in sync.sagas.ts by creating no-argument action creators for success and error actions, enhancing code clarity. - Refactored sync.slice.ts to replace 'never' with 'undefined' in the async slice definition, improving type accuracy.
- Added import for seedInitialTasks utility to ensure initial tasks are seeded for specific dates when not in a test environment. - Removed redundant import statement to streamline the code and improve clarity in the loaders.ts file.
- Deleted Login component and associated styled file to streamline the codebase. - Updated Logout view to use a new styled container for layout, enhancing code clarity and structure. - Added order property to task test cases for consistency in task management tests.
…include order property - Removed the unused React import from NotFound.tsx to clean up the code. - Added order property to task test cases in AvailableTasks and FocusedTask components for consistency in task management tests.
| }; | ||
| ONBOARDING_PROGRESS: "compass.onboarding", | ||
| AUTH: "compass.auth", | ||
| } as const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing localStorage migration loses existing user preferences
Medium Severity
The localStorage keys were consolidated from three separate keys (compass.auth.hasCompletedSignup, compass.auth.skipOnboarding, compass.day.storage-info-seen) into a single compass.onboarding object without migration code. Existing users' stored preferences are orphaned and won't be read. Users who previously skipped onboarding would see it again, users who saw the storage warning would see it again, and signup completion status would be lost.
Note
Offline-first + auth-optional onboarding
compass-local),EventRepositoryabstraction (local/remote), and local Task repository; auto-sync local events to cloud post-auth and trigger refetchUNAUTHENTICATED_USER, relax UserProvider errors, simplify Axios 404 handling (redirects toDAY), addAuthStatelocal flag and onboarding storage unificationisSyncing; showCalendarImportOverlayand completion modal inAuthenticatedLayoutImport progress/result reporting
startGoogleCalendarSyncreturns{ eventsCount, calendarsCount };handleImportGCalEndemits JSON summary; adjust tests accordinglyimportResultsin Redux, display completion modal; keep syncing state until websocket endTesting and infra
fake-indexeddb)Misc
db-init,event.storage), onboarding/storage key consolidationCheckBoxcomponentdexie,dexie-react-hooks,fake-indexeddb,baseline-browser-mappingWritten by Cursor Bugbot for commit 5598452. This will update automatically on new commits. Configure here.