-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Improve onboarding flow with guided navigation and microphone button #4033
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
- Replace "Get Started" button with purple microphone icon on speech profile page - Add onboarding text and arrows guiding users through conversations -> tasks -> memories flow - Add arrow pointing to brain icon on memories page - Hide "Your 2025 Wrapped" banner temporarily - Track onboarding completion via brain map opening or view count threshold 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
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.
Code Review
This pull request introduces a new onboarding flow to guide users through the main features of the app. The changes are well-structured, adding new widgets for different onboarding steps and using SharedPreferences to track completion. My review focuses on improving maintainability and fixing potential UI issues related to responsiveness. I've identified a few areas for improvement: replacing a magic number with a constant (referencing a rule on consistency in constant values), removing redundant conditional checks, addressing hardcoded UI values that affect responsiveness, and refactoring duplicated code. Overall, this is a great addition to improve user experience.
| const SizedBox(height: 8), | ||
| // Arrow pointing down to memories tab (3rd icon, right of center) | ||
| Padding( | ||
| padding: const EdgeInsets.only(left: 220), |
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.
Using a hardcoded padding value left: 220 is not responsive and will cause UI issues on different screen sizes. The arrow will not be positioned correctly under the 'Memories' tab. Please use a responsive approach, for example by using MediaQuery.of(context).size.width to calculate the padding dynamically.
| const SizedBox(height: 8), | ||
| // Arrow pointing down to tasks tab (2nd icon) | ||
| Padding( | ||
| padding: const EdgeInsets.only(left: 85), |
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.
Using a hardcoded padding value left: 85 is not responsive and will cause UI issues on different screen sizes. The arrow will not be positioned correctly under the 'Tasks' tab. Please use a responsive approach, for example by using MediaQuery.of(context).size.width to calculate the padding dynamically.
| alignment: Alignment.centerRight, | ||
| child: Padding( | ||
| // Position arrow under the brain button (first icon after search) | ||
| padding: const EdgeInsets.only(right: 44), |
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.
Using a hardcoded padding value right: 44 is not responsive and will cause UI issues on different screen sizes. The arrow will not be positioned correctly under the brain button. Please use a responsive approach, for example by using MediaQuery.of(context).size.width to calculate the padding dynamically or using an Align widget.
| // Home onboarding - track completion | ||
| bool get hasOpenedBrainMap => getBool('hasOpenedBrainMap', defaultValue: false); | ||
|
|
||
| set hasOpenedBrainMap(bool value) => saveBool('hasOpenedBrainMap', value); | ||
|
|
||
| int get homeOnboardingViewCount => getInt('homeOnboardingViewCount', defaultValue: 0); | ||
|
|
||
| set homeOnboardingViewCount(int value) => saveInt('homeOnboardingViewCount', value); | ||
|
|
||
| void incrementHomeOnboardingViewCount() { | ||
| homeOnboardingViewCount = homeOnboardingViewCount + 1; | ||
| } | ||
|
|
||
| bool get isHomeOnboardingCompleted => hasOpenedBrainMap || homeOnboardingViewCount >= 10; |
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.
The value 10 used as the view count threshold is a magic number. It should be extracted into a named constant to improve readability and maintainability.
| // Home onboarding - track completion | |
| bool get hasOpenedBrainMap => getBool('hasOpenedBrainMap', defaultValue: false); | |
| set hasOpenedBrainMap(bool value) => saveBool('hasOpenedBrainMap', value); | |
| int get homeOnboardingViewCount => getInt('homeOnboardingViewCount', defaultValue: 0); | |
| set homeOnboardingViewCount(int value) => saveInt('homeOnboardingViewCount', value); | |
| void incrementHomeOnboardingViewCount() { | |
| homeOnboardingViewCount = homeOnboardingViewCount + 1; | |
| } | |
| bool get isHomeOnboardingCompleted => hasOpenedBrainMap || homeOnboardingViewCount >= 10; | |
| // Home onboarding - track completion | |
| static const int homeOnboardingViewCountThreshold = 10; | |
| bool get hasOpenedBrainMap => getBool('hasOpenedBrainMap', defaultValue: false); | |
| set hasOpenedBrainMap(bool value) => saveBool('hasOpenedBrainMap', value); | |
| int get homeOnboardingViewCount => getInt('homeOnboardingViewCount', defaultValue: 0); | |
| set homeOnboardingViewCount(int value) => saveInt('homeOnboardingViewCount', value); | |
| void incrementHomeOnboardingViewCount() { | |
| homeOnboardingViewCount = homeOnboardingViewCount + 1; | |
| } | |
| bool get isHomeOnboardingCompleted => hasOpenedBrainMap || homeOnboardingViewCount >= homeOnboardingViewCountThreshold; |
References
- Ensure consistency in constant values used for calculations across different parts of the codebase, especially when those calculations affect user-facing metrics like progress reporting. Define a single, well-defined constant and reuse it to avoid inaccuracies.
| if (!SharedPreferencesUtil().isHomeOnboardingCompleted) | ||
| const SliverToBoxAdapter(child: TasksOnboardingWidget()), |
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.
The visibility of TasksOnboardingWidget is controlled by this if condition. However, the widget itself contains a similar check, making this one redundant. To improve encapsulation and reduce code duplication, it's better to let the widget manage its own visibility and remove the check here. This principle also applies to HomeOnboardingWidget in conversations_page.dart and MemoriesOnboardingWidget in memories/page.dart.
| if (!SharedPreferencesUtil().isHomeOnboardingCompleted) | |
| const SliverToBoxAdapter(child: TasksOnboardingWidget()), | |
| const SliverToBoxAdapter(child: TasksOnboardingWidget()), |
| class _ArrowDownPainter extends CustomPainter { | ||
| @override | ||
| void paint(Canvas canvas, Size size) { | ||
| final paint = Paint() | ||
| ..color = Colors.grey.shade600 | ||
| ..strokeWidth = 2 | ||
| ..style = PaintingStyle.stroke | ||
| ..strokeCap = StrokeCap.round; | ||
|
|
||
| // Curved line going down | ||
| final path = Path(); | ||
| path.moveTo(size.width * 0.5, 0); | ||
| path.quadraticBezierTo( | ||
| size.width * 0.6, | ||
| size.height * 0.5, | ||
| size.width * 0.5, | ||
| size.height * 0.85, | ||
| ); | ||
|
|
||
| canvas.drawPath(path, paint); | ||
|
|
||
| // Arrow head pointing down | ||
| final arrowPath = Path(); | ||
| arrowPath.moveTo(size.width * 0.5 - 6, size.height * 0.72); | ||
| arrowPath.lineTo(size.width * 0.5, size.height * 0.85); | ||
| arrowPath.lineTo(size.width * 0.5 + 6, size.height * 0.72); | ||
|
|
||
| canvas.drawPath(arrowPath, paint); | ||
| } | ||
|
|
||
| @override | ||
| bool shouldRepaint(covariant CustomPainter oldDelegate) => false; | ||
| } |
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.
The _ArrowDownPainter class is duplicated in app/lib/pages/conversations/widgets/home_onboarding_widget.dart. To follow the DRY (Don't Repeat Yourself) principle and improve maintainability, this class should be extracted into a shared file (e.g., in a widgets or utils directory) and imported where needed. The leading underscore in the class name should also be removed to make it public.
Summary
Test plan
🤖 Generated with Claude Code