Skip to content

Native Android App Shortcuts Integration#138

Open
KunalPatilCode wants to merge 5 commits intoAOSSIE-Org:mainfrom
KunalPatilCode:feature/app-shortcuts
Open

Native Android App Shortcuts Integration#138
KunalPatilCode wants to merge 5 commits intoAOSSIE-Org:mainfrom
KunalPatilCode:feature/app-shortcuts

Conversation

@KunalPatilCode
Copy link

@KunalPatilCode KunalPatilCode commented Jan 19, 2026

Closes #137

📝 Description

This pull request introduces native Android App Shortcuts to Ell-ena, enabling users to quickly navigate to frequently used sections directly from the launcher by long-pressing the app icon.

The goal of this change is to reduce friction in common user workflows and align Ell-ena with modern Android UX standards. By leveraging platform-native shortcuts, users can access key areas of the app without first navigating through the internal UI, resulting in faster interactions and an improved overall experience.

This implementation is designed to work seamlessly for both cold starts and background states, while preserving user intent and handling authentication edge cases gracefully.

🔧 Changes Made

Added native Android App Shortcuts for:

  • 📊 Dashboard

  • 📅 Calendar

  • 💬 Chat

  • 👤 Profile

  • Implemented deep-link based routing to ensure correct navigation from launcher shortcuts

  • Ensured compatibility with both cold start and resumed app states

  • Maintained a clean separation between Flutter and native Android code

  • Followed Android best practices for labels, icons, and shortcut configuration

📷 Screenshots or Visual Changes (if applicable)

Screen.Recording.2026-01-19.194142.mp4

🤝 Collaboration

Collaborated with: (None)

✅ Checklist

  • I have read the contributing guidelines.
  • I have added tests or manually verified that the feature works as expected.
  • I have added necessary documentation where applicable.
  • Any dependent changes have been merged and published in downstream modules.

Summary by CodeRabbit

  • New Features

    • Launcher shortcuts for Dashboard, Calendar, Workspace, Chat, and Profile with native-to-app handling on cold start and while running.
    • In-app shortcut service to capture pending shortcuts and route after startup/auth.
  • Chores

    • Added localized shortcut labels/resources and platform plugin registrations (printing, speech-to-text, permissions).
    • Routing and startup flow refactoring to support shortcut-driven navigation.
  • Bug Fixes

    • Theme/manifest adjustments to improve startup and navigation reliability.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 19, 2026

📝 Walkthrough

Walkthrough

Adds Android launcher shortcuts and native-to-Flutter routing: manifest meta-data and res/xml/shortcuts.xml for five shortcuts, a MethodChannel-enabled MainActivity that parses intents and forwards routes, a new AppShortcutsService in Dart, and plumbing across startup/auth/home screens to consume shortcut context.

Changes

Cohort / File(s) Summary
Android manifest & shortcuts
android/app/src/main/AndroidManifest.xml, android/app/src/main/res/xml/shortcuts.xml, android/app/src/main/res/values/strings.xml
Commented-out io.flutter.embedding.android.NormalTheme meta-data; added <meta-data android:name="android.app.shortcuts" android:resource="@xml/shortcuts"/>; new shortcuts.xml defining five launcher shortcuts and corresponding string resources.
Android native bridging (MainActivity)
android/app/src/main/kotlin/org/aossie/ell_ena/MainActivity.kt
Replaced simple FlutterActivity behavior with MethodChannel "app_shortcuts": parses cold-start and onNewIntent intents (deep links and screen extra), stores pendingRoute, and invokes Flutter via MethodChannel to request navigation.
Flutter shortcut service
lib/services/app_shortcuts_service.dart
New service exposing MethodChannel client, initialization/getPendingShortcut APIs, callback registration, route↔index mapping, navigate helpers, and native communication helpers.
App bootstrap & routing
lib/main.dart, lib/screens/splash_screen.dart, lib/screens/home/home_screen.dart
Initialize AppShortcutsService early; consume pending shortcuts on startup; merge shortcut context into route arguments; map routes to tab indices and dispatch navigation to Home or Login flows.
Auth & onboarding flows
lib/screens/auth/login_screen.dart, lib/screens/auth/signup_screen.dart, lib/screens/auth/verify_otp_screen.dart
Added optional arguments payloads to auth screens; propagate pending shortcut context through signup/OTP flows; unify OTP navigation and route-to-index mapping for post-auth navigation.
UI & styles
android/app/src/main/res/values/styles.xml, lib/screens/*
Set NormalTheme windowBackground to transparent and applied minor UI/formatting/validator cleanup across several screens.
Desktop plugin registrants
linux/.../generated_plugin_registrant.cc, linux/.../generated_plugins.cmake, macos/Flutter/GeneratedPluginRegistrant.swift, windows/.../generated_plugin_registrant.cc, windows/.../generated_plugins.cmake
Added/registered printing, speech_to_text, and permission_handler plugins in generated registrants and plugin lists for desktop platforms.

Sequence Diagram(s)

sequenceDiagram
    participant Launcher as Launcher (User)
    participant Android as Android System
    participant MainActivity as MainActivity (native)
    participant NativeChannel as MethodChannel (native)
    participant DartService as AppShortcutsService (Dart)
    participant Flutter as Flutter Navigator

    Launcher->>Android: Tap shortcut (app://shortcut/<name>, extras: screen=N)
    Android->>MainActivity: Deliver intent (cold start or onNewIntent)
    MainActivity->>MainActivity: handleIntent() parse route/screen -> set pendingRoute
    MainActivity->>NativeChannel: respond to "getInitialRoute" or invoke "navigate" with route
    NativeChannel->>DartService: deliver initial/push route
    DartService->>DartService: store pending shortcut or call registered handler
    DartService->>Flutter: if running, trigger callback -> Flutter Navigator navigates (HomeScreen with args {screen:N, initial_route:...})
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰 I hop to the icon, a nibble and twitch,
Five tiny paths in a single quick switch.
Native whispers to Flutter, a gentle guide,
Routes like carrots—swift steps inside.
Hooray—shortcuts make my hop delight!

🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Several changes are out of scope for issue #137: plugin registrations for Printing and SpeechToText added to Linux, macOS, and Windows platforms; styles.xml theme color modification; no mention in the feature request of these plugins or theme changes. Remove plugin registrations unrelated to app shortcuts (Printing, SpeechToText) from generated files. Clarify or revert the styles.xml windowBackground change if not part of shortcuts implementation. Confirm scope alignment with issue #137.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'Native Android App Shortcuts Integration' directly and clearly summarizes the main change—adding Android app shortcuts. It is concise, specific, and aligned with the primary objective.
Linked Issues check ✅ Passed All coding objectives from issue #137 are met: app shortcuts for Dashboard, Calendar, Chat, and Profile are implemented with labels and icons; deep-link routing supports cold starts and background states; native-Flutter separation is maintained; authentication edge cases are handled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🤖 Fix all issues with AI agents
In `@android/app/src/main/kotlin/org/aossie/ell_ena/MainActivity.kt`:
- Around line 20-24: onNewIntent currently calls handleIntent(intent) but never
notifies Flutter of the updated pendingRoute; after calling handleIntent(intent)
invoke sendRouteToFlutter() so the newly parsed route is sent to Flutter's
navigation handler (call sendRouteToFlutter() immediately after
handleIntent(intent) in onNewIntent, keeping the existing setIntent(intent)
update).
- Around line 70-77: In sendRouteToFlutter(), the catch block currently swallows
exceptions from methodChannel?.invokeMethod("navigate", pendingRoute); update
the catch to log a warning using Android's Log.w (e.g., Log.w("MainActivity",
"Failed to send pending route to Flutter", e)) so failures to dispatch
pendingRoute are observable; keep clearing pendingRoute only after a successful
invoke or leave behavior as-is but ensure the exception and pendingRoute value
are included in the log for debugging.

In `@android/app/src/main/res/xml/shortcuts.xml`:
- Line 18: The shortcut entries Dashboard, Calendar, Workspace, and Profile in
shortcuts.xml incorrectly declare the messaging category
android.shortcut.conversation; remove the <categories
android:name="android.shortcut.conversation" /> element from those non-chat
<shortcut> elements (leave the Chat shortcut's category intact), or omit the
categories entirely for those shortcuts so only the actual Chat shortcut retains
android.shortcut.conversation.

In `@lib/main.dart`:
- Around line 70-71: Remove the duplicate, race-prone call to
AppShortcutsService.getPendingShortcut() from onGenerateRoute so the pending
shortcut (which is cleared on read) isn't consumed multiple times; instead, read
the shortcut once at app startup (where SplashScreen currently reads it) or
centralize the single read and pass the resulting value down the widget tree
(e.g., via an inherited widget, provider, or constructor params) so
SplashScreen, LoginScreen, VerifyOTPScreen and any other consumers use that
single read rather than calling getPendingShortcut() directly.

In `@lib/screens/auth/login_screen.dart`:
- Around line 88-105: homeScreenArgs is being assigned directly from
widget.arguments and then mutated, which can unexpectedly alter the caller's
map; instead create a shallow copy before modifying: replace the direct
assignment of homeScreenArgs = widget.arguments ?? {} with a new mutable map
copy (e.g., using the spread operator) so subsequent mutations (adding 'screen'
and 'initial_route' when pendingShortcut != null, and the later homeScreenArgs
checks) only affect the local variable in LoginScreen and not the original
widget.arguments; update references around pendingShortcut and _getScreenIndex
accordingly to operate on the copied map.

In `@lib/screens/auth/signup_screen.dart`:
- Around line 77-90: The current SignupScreen code builds a Map named userData
containing sensitive fields (notably _passwordController.text) and then prints
it; remove or redact any logging of userData or its sensitive fields (especially
the password) before navigation to VerifyOTPScreen. Locate the userData
construction and the print('[SignupScreen] Navigate to VerifyOTPScreen with
userData: $userData') call in SignupScreen and either delete the print entirely
or replace it with a non-sensitive log (e.g., only log teamName and adminName or
a generic message), and ensure any added widget.arguments are not printed if
they may contain secrets.

In `@lib/screens/auth/verify_otp_screen.dart`:
- Around line 137-140: The code unsafely casts widget.userData['initial_args']
to Map<String, dynamic> which can throw a TypeError; update the VerifyOTPScreen
logic to perform a safe type check and conversion: first check that
widget.userData.containsKey('initial_args') and that
widget.userData['initial_args'] is a Map (e.g., using `is Map` or `is
Map<dynamic, dynamic>`), then convert/cast it safely to Map<String, dynamic>
(for example via `.cast<String, dynamic>()`) before assigning to homeScreenArgs,
and otherwise handle the non-map case (skip assignment or set a default) to
avoid runtime exceptions.
- Around line 40-43: Remove the debug print statements in VerifyOTPScreen's
initState that print widget.verifyType and widget.userData (and any similar
print() calls at lines mentioned), and either delete them or replace them with
guarded debug logging (e.g., use debugPrint or wrap in a kDebugMode check) so
sensitive values are not emitted in production; search for print(...) in
VerifyOTPScreen (including initState and the other spots around the referenced
prints) and update those calls accordingly to use non-production-safe logging or
remove them.

In `@lib/screens/home/home_screen.dart`:
- Around line 71-74: AppShortcutsService currently stores the HomeScreen
callback in its static _onShortcutPressed via AppShortcutsService.init(...) but
offers no cleanup; add a static void dispose() method on AppShortcutsService
that sets _onShortcutPressed = null (or clears the reference) and then call
AppShortcutsService.dispose() inside HomeScreen.dispose() so the retained
callback doesn't keep the old HomeScreen instance alive.

In `@lib/screens/splash_screen.dart`:
- Around line 66-71: widget.arguments may be an unmodifiable Map and you
currently mutate it via homeScreenArgs; instead create a new mutable copy before
adding shortcut fields. In the splash screen code where you set Map<String,
dynamic>? homeScreenArgs = widget.arguments ?? {}; change the logic to
shallow-copy widget.arguments (e.g., new Map from widget.arguments) so that when
you reference screenIndex and initialShortcut and assign
homeScreenArgs['screen'] and homeScreenArgs['initial_route'] you’re mutating a
new Map not the original widget.arguments.
🧹 Nitpick comments (11)
lib/main.dart (1)

69-83: Consider extracting getScreenIndex to avoid repeated closure allocation.

The getScreenIndex function is defined inside onGenerateRoute, causing a new closure to be created on every navigation event. While not a significant performance issue, extracting it as a top-level or static function improves clarity and avoids unnecessary allocations.

Suggested refactor
+int _getScreenIndexFromShortcut(String? shortcut) {
+  switch (shortcut) {
+    case 'dashboard': return 0;
+    case 'calendar': return 1;
+    case 'workspace': return 2;
+    case 'chat': return 3;
+    case 'profile': return 4;
+    default: return 0;
+  }
+}
+
 class MyApp extends StatelessWidget {
   // ...
   onGenerateRoute: (settings) {
     final initialShortcut = AppShortcutsService.getPendingShortcut();
-        
-    int getScreenIndex(String? shortcut) {
-      switch (shortcut) {
-        case 'dashboard': return 0;
-        case 'calendar': return 1;
-        case 'workspace': return 2;
-        case 'chat': return 3;
-        case 'profile': return 4;
-        default: return 0;
-      }
-    }
     
     switch (settings.name) {
       case '/home':
         // ...
-        'screen': getScreenIndex(initialShortcut),
+        'screen': _getScreenIndexFromShortcut(initialShortcut),
lib/screens/home/home_screen.dart (2)

97-101: Redundant type check and missing const keyword.

The condition _screens[1] is CalendarScreen is always true since _screens[1] is initialized as CalendarScreen() in _initializeScreens(). The check is redundant.

Also, CalendarScreen() should use const for consistency with _initializeScreens().

Suggested fix
       case 'calendar':
         index = 1;
-        // Reset calendar screen if needed
-        if (_screens[1] is CalendarScreen) {
-          setState(() {
-            _screens[1] = CalendarScreen();
-          });
-        }
+        // Reset calendar screen
+        setState(() {
+          _screens[1] = const CalendarScreen();
+        });
         break;

87-125: Multiple setState calls cause redundant rebuilds.

The _handleShortcut method calls setState multiple times (once for screen reset at lines 98-100 or 109-111, and again at lines 121-123). This triggers multiple widget rebuilds. Consider consolidating into a single setState call.

Also, ChatScreen() at line 110 should use const.

Suggested consolidated implementation
 void _handleShortcut(String route) {
-  int index = 0;
-  
-  switch (route) {
-    case 'dashboard':
-      index = 0;
-      break;
-    case 'calendar':
-      index = 1;
-      // Reset calendar screen if needed
-      if (_screens[1] is CalendarScreen) {
-        setState(() {
-          _screens[1] = CalendarScreen();
-        });
-      }
-      break;
-    case 'workspace':
-      index = 2;
-      break;
-    case 'chat':
-      index = 3;
-      // Reset chat screen if needed
-      setState(() {
-        _screens[3] = ChatScreen();
-      });
-      break;
-    case 'profile':
-      index = 4;
-      break;
-    default:
-      index = 0;
-  }
-
-  if (mounted) {
-    setState(() {
-      _selectedIndex = index;
-    });
-  }
+  if (!mounted) return;
+  
+  setState(() {
+    switch (route) {
+      case 'dashboard':
+        _selectedIndex = 0;
+        break;
+      case 'calendar':
+        _selectedIndex = 1;
+        _screens[1] = const CalendarScreen();
+        break;
+      case 'workspace':
+        _selectedIndex = 2;
+        break;
+      case 'chat':
+        _selectedIndex = 3;
+        _screens[3] = const ChatScreen();
+        break;
+      case 'profile':
+        _selectedIndex = 4;
+        break;
+      default:
+        _selectedIndex = 0;
+    }
+  });
 }
lib/screens/auth/verify_otp_screen.dart (2)

165-176: Duplicate _getScreenIndex logic across multiple files.

This screen index mapping logic is duplicated in main.dart, home_screen.dart, and here. This violates DRY and creates maintenance burden when adding/reordering screens.

Consider centralizing this in AppShortcutsService as a static method that all files can reference.

Suggested approach

Add to AppShortcutsService:

static int getScreenIndex(String? route) {
  switch (route) {
    case 'dashboard': return 0;
    case 'calendar': return 1;
    case 'workspace': return 2;
    case 'chat': return 3;
    case 'profile': return 4;
    default: return 0;
  }
}

Then replace local implementations with AppShortcutsService.getScreenIndex(route).


158-162: Inconsistent navigation pattern.

This uses Navigator.of(context).pushReplacement directly, while other navigation in this file uses NavigationService().navigateTo (line 86). Consider using NavigationService for consistency, or document why direct navigation is preferred here.

lib/screens/auth/login_screen.dart (2)

33-36: Guard debug logging to avoid release log noise.

Consider wrapping with kDebugMode and using debugPrint so logs don’t ship in production.

💡 Suggested tweak
+import 'package:flutter/foundation.dart';
...
-    print('[LoginScreen] initState called with arguments: ${widget.arguments}');
+    if (kDebugMode) {
+      debugPrint('[LoginScreen] initState called with arguments: ${widget.arguments}');
+    }

143-155: Consider centralizing shortcut route → index mapping.

This mapping is duplicated in multiple screens; a shared helper in AppShortcutsService would prevent drift.

lib/screens/splash_screen.dart (2)

29-31: Guard debug logging in release builds.

💡 Suggested tweak
+import 'package:flutter/foundation.dart';
...
-    print('[SplashScreen] initState called with arguments: ${widget.arguments}');
+    if (kDebugMode) {
+      debugPrint('[SplashScreen] initState called with arguments: ${widget.arguments}');
+    }

103-114: Consider centralizing shortcut route → index mapping.

This duplicate mapping also exists in login; extracting into a shared utility reduces drift risk.

lib/screens/auth/signup_screen.dart (1)

37-38: Guard debug logging in release builds.

💡 Suggested tweak
+import 'package:flutter/foundation.dart';
...
-    print('[SignupScreen] initState called with arguments: ${widget.arguments}');
+    if (kDebugMode) {
+      debugPrint('[SignupScreen] initState called with arguments: ${widget.arguments}');
+    }
lib/services/app_shortcuts_service.dart (1)

77-82: Buffer shortcuts if handler isn’t ready yet.

If a navigate call arrives before init is wired, the route is dropped. Consider caching it to _pendingShortcut.

♻️ Suggested adjustment
   static void _handleShortcut(String route) {
-    if (_onShortcutPressed != null) {
-      _onShortcutPressed!(route);
-    }
+    if (_onShortcutPressed != null) {
+      _onShortcutPressed!(route);
+    } else {
+      _pendingShortcut = route;
+    }
   }

Comment on lines +70 to +71
// Get initial shortcut if any
final initialShortcut = AppShortcutsService.getPendingShortcut();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, find the AppShortcutsService file
find . -type f -name "*shortcuts*service*" -o -type f -name "*app_shortcuts*"

Repository: AOSSIE-Org/Ell-ena

Length of output: 103


🏁 Script executed:

# Get the context around lines 70-71 in lib/main.dart
echo "=== Context around lines 70-71 in lib/main.dart ===" 
cat -n lib/main.dart | sed -n '60,85p'

Repository: AOSSIE-Org/Ell-ena

Length of output: 1020


🏁 Script executed:

# Search for getPendingShortcut implementation
echo "=== Searching for getPendingShortcut implementation ===" 
rg -n -B 2 -A 15 "getPendingShortcut" lib/

Repository: AOSSIE-Org/Ell-ena

Length of output: 6805


🏁 Script executed:

# Get the complete onGenerateRoute implementation
echo "=== Full onGenerateRoute implementation in main.dart ===" 
cat -n lib/main.dart | sed -n '69,120p'

Repository: AOSSIE-Org/Ell-ena

Length of output: 2185


Remove duplicate getPendingShortcut() call—race condition will consume the shortcut before all callers can read it.

getPendingShortcut() clears the pending shortcut on read (line 26 in app_shortcuts_service.dart). Calling it in onGenerateRoute (line 71) creates a race condition: whichever screen/route initializes first will consume the shortcut, leaving null for all other callers.

This method is already called by SplashScreen (line 59), LoginScreen (line 84), and VerifyOTPScreen (line 131). Remove the call from onGenerateRoute or refactor to read the shortcut only once at app startup and pass it through the widget tree.

🤖 Prompt for AI Agents
In `@lib/main.dart` around lines 70 - 71, Remove the duplicate, race-prone call to
AppShortcutsService.getPendingShortcut() from onGenerateRoute so the pending
shortcut (which is cleared on read) isn't consumed multiple times; instead, read
the shortcut once at app startup (where SplashScreen currently reads it) or
centralize the single read and pass the resulting value down the widget tree
(e.g., via an inherited widget, provider, or constructor params) so
SplashScreen, LoginScreen, VerifyOTPScreen and any other consumers use that
single read rather than calling getPendingShortcut() directly.

Comment on lines 77 to 90
// FIX: Explicitly cast to Map<String, dynamic> or create as dynamic map
final Map<String, dynamic> userData = {
'teamName': _teamNameController.text,
'adminName': _nameController.text,
'password': _passwordController.text,
};

// Add any existing arguments
if (widget.arguments != null) {
userData['initial_args'] = widget.arguments;
}

print('[SignupScreen] Navigate to VerifyOTPScreen with userData: $userData');

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Do not log credentials or sensitive signup data.

userData includes plaintext passwords; logging it is a security risk and should be removed or redacted.

🔒 Suggested fix (remove sensitive logs)
-        print('[SignupScreen] Navigate to VerifyOTPScreen with userData: $userData');
...
-        print('[SignupScreen] Navigate to VerifyOTPScreen with userData: $userData');

Also applies to: 148-161

🤖 Prompt for AI Agents
In `@lib/screens/auth/signup_screen.dart` around lines 77 - 90, The current
SignupScreen code builds a Map named userData containing sensitive fields
(notably _passwordController.text) and then prints it; remove or redact any
logging of userData or its sensitive fields (especially the password) before
navigation to VerifyOTPScreen. Locate the userData construction and the
print('[SignupScreen] Navigate to VerifyOTPScreen with userData: $userData')
call in SignupScreen and either delete the print entirely or replace it with a
non-sensitive log (e.g., only log teamName and adminName or a generic message),
and ensure any added widget.arguments are not printed if they may contain
secrets.

Comment on lines 71 to 74
// Initialize shortcuts handler
AppShortcutsService.init((route) {
_handleShortcut(route);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if AppShortcutsService has a cleanup/dispose mechanism
echo "=== Checking for dispose/cleanup in AppShortcutsService ==="
rg -n -B 2 -A 10 'dispose|cleanup|removeListener|cancel' lib/services/app_shortcuts_service.dart

echo ""
echo "=== Checking init method implementation ==="
rg -n -A 15 'static.*init\(' lib/services/app_shortcuts_service.dart

Repository: AOSSIE-Org/Ell-ena

Length of output: 740


🏁 Script executed:

# Check HomeScreen for dispose method
echo "=== Checking HomeScreen dispose method ==="
rg -n -B 2 -A 15 'dispose\(' lib/screens/home/home_screen.dart

echo ""
echo "=== Check full AppShortcutsService file ==="
wc -l lib/services/app_shortcuts_service.dart

Repository: AOSSIE-Org/Ell-ena

Length of output: 741


🏁 Script executed:

cat -n lib/services/app_shortcuts_service.dart

Repository: AOSSIE-Org/Ell-ena

Length of output: 4063


Add cleanup method to AppShortcutsService and call it in HomeScreen.dispose().

AppShortcutsService.init() stores a callback reference in the static _onShortcutPressed variable without providing a cleanup mechanism. When HomeScreen is disposed and recreated, this callback reference persists in memory, keeping the old HomeScreen instance alive and causing a memory leak. Add a static dispose() method to AppShortcutsService that clears _onShortcutPressed and call it from HomeScreen.dispose().

🤖 Prompt for AI Agents
In `@lib/screens/home/home_screen.dart` around lines 71 - 74, AppShortcutsService
currently stores the HomeScreen callback in its static _onShortcutPressed via
AppShortcutsService.init(...) but offers no cleanup; add a static void dispose()
method on AppShortcutsService that sets _onShortcutPressed = null (or clears the
reference) and then call AppShortcutsService.dispose() inside
HomeScreen.dispose() so the retained callback doesn't keep the old HomeScreen
instance alive.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@lib/screens/splash_screen.dart`:
- Around line 80-86: The code attempts to construct LoginScreen with a
non-existent arguments parameter (builder: (context) => LoginScreen(arguments:
homeScreenArgs)), causing a compile error; fix by removing the arguments named
parameter from that instantiation and instead either (a) add an arguments
field/constructor to the LoginScreen class and propagate homeScreenArgs through
LoginScreen (update LoginScreen constructor and any state handling to accept and
forward homeScreenArgs), or (b) drop the arguments from the Navigator
pushReplacement call and persist homeScreenArgs elsewhere (e.g., pass via a
shared navigation/state handler or save to a temporary variable) so LoginScreen
remains const LoginScreen({super.key}). Ensure you update the LoginScreen
constructor or the pushReplacement call consistently (symbols: LoginScreen,
homeScreenArgs, Navigator.pushReplacement/MaterialPageRoute).
♻️ Duplicate comments (1)
lib/screens/splash_screen.dart (1)

64-69: Map mutation issue persists.

This was flagged in a previous review. If widget.arguments is an unmodifiable map (common with Flutter route arguments), mutating it at lines 67-68 will throw a runtime error.

🐛 Apply the previously suggested fix
-      Map<String, dynamic> homeScreenArgs = widget.arguments ?? {};
-      if (screenIndex != null) {
-        homeScreenArgs['screen'] = screenIndex;
-        homeScreenArgs['initial_route'] = initialShortcut;
-      }
+      final Map<String, dynamic> homeScreenArgs = {
+        ...?widget.arguments,
+        if (screenIndex != null) 'screen': screenIndex,
+        if (screenIndex != null) 'initial_route': initialShortcut,
+      };
🧹 Nitpick comments (3)
android/app/src/main/AndroidManifest.xml (2)

31-34: Correct shortcuts declaration, but indentation is inconsistent.

The shortcuts meta-data is correctly placed within the <activity> block as required by Android. However, the indentation differs from the sibling NormalTheme meta-data element above (lines 27-30), which impacts readability.

🔧 Suggested formatting fix
-              <meta-data
-                android:name="android.app.shortcuts"
-                android:resource="@xml/shortcuts"/>
-
+            <meta-data
+              android:name="android.app.shortcuts"
+              android:resource="@xml/shortcuts"
+              />

51-52: Remove extraneous whitespace.

These lines introduce unnecessary blank lines and trailing spaces. Consider cleaning up for consistency.

lib/screens/splash_screen.dart (1)

48-49: Consider reducing the splash delay.

A 3-second delay is notably long for a splash screen and may negatively impact perceived app responsiveness. Modern UX guidelines typically recommend 1-2 seconds maximum. The comment says "small delay" but 3 seconds contradicts this.

♻️ Suggested change
-    // Give a small delay to ensure everything is initialized
-    Timer(const Duration(seconds: 3), _checkSession);
+    // Brief delay for animations to settle before navigation
+    Timer(const Duration(milliseconds: 1500), _checkSession);

- Removed sensitive logging from SignupScreen
- Fixed unsafe cast & removed debug prints in VerifyOTPScreen
- Added cleanup to HomeScreen to prevent memory leaks
- Fixed arguments handling in LoginScreen
- Updated MainActivity.kt to handle shortcuts correctly and added logging
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
lib/screens/auth/signup_screen.dart (2)

2-21: Resolve merge conflict markers; file won’t compile as‑is.

The file still contains unresolved conflict markers around imports/constructor and the primary action button. These must be removed before merge.

🛠️ Example resolution for import/constructor + button
-import 'package:flutter/material.dart';
-<<<<<<< HEAD
-import 'package:font_awesome_flutter/font_awesome_flutter.dart';
-=======
->>>>>>> 1c9161b (fix: remove sensitive logging and unsafe casts, add cleanup to prevent memory leaks)
+import 'package:flutter/material.dart';
+import 'package:font_awesome_flutter/font_awesome_flutter.dart';
...
-class SignupScreen extends StatefulWidget {
-<<<<<<< HEAD
-  const SignupScreen({super.key});
-=======
-  final Map<String, dynamic>? arguments;
-
-  const SignupScreen({super.key, this.arguments});
->>>>>>> 1c9161b (fix: remove sensitive logging and unsafe casts, add cleanup to prevent memory leaks)
-}
+class SignupScreen extends StatefulWidget {
+  final Map<String, dynamic>? arguments;
+  const SignupScreen({super.key, this.arguments});
+}
...
-          onPressed: _isLoading
-              ? null
-<<<<<<< HEAD
-              : (_tabController.index == 0
-                  ? _handleJoinTeam
-                  : _handleCreateTeam),
-=======
-              : (_tabController.index == 0 ? _handleJoinTeam : _handleCreateTeam),
->>>>>>> 1c9161b (fix: remove sensitive logging and unsafe casts, add cleanup to prevent memory leaks)
+          onPressed: _isLoading
+              ? null
+              : (_tabController.index == 0 ? _handleJoinTeam : _handleCreateTeam),

Also applies to: 412-418


470-474: Preserve arguments when routing back to LoginScreen.

This keeps shortcut intent if the user switches from Sign Up to Sign In.

🛠️ Suggested fix
-            NavigationService().navigateToReplacement(
-              const LoginScreen(),
-            );
+            NavigationService().navigateToReplacement(
+              LoginScreen(arguments: widget.arguments),
+            );
🤖 Fix all issues with AI agents
In `@lib/screens/auth/login_screen.dart`:
- Around line 6-22: Resolve and remove all Git conflict markers (<<<<<<<,
=======, >>>>>>>) in this file and keep the intended new logic: ensure the
import for AppShortcutsService is present, the LoginScreen class includes the
final constructor signature with final Map<String, dynamic>? arguments, and
merge the initState/login success/error/Google sign-in/sign-up blocks to
preserve the shortcut merge logic, the _getScreenIndex helper, and the existing
Google sign-in flow; locate and fix conflicts around the LoginScreen
constructor, initState, the method that handles login success/error, the Google
sign-in handler, and navigation to signup/team selection so the file compiles
and the new shortcuts/arguments behavior remains intact.
- Around line 96-113: homeScreenArgs is constructed from widget.arguments and
pendingShortcut but never passed to HomeScreen, so the shortcut intent is lost;
update the call to NavigationService().navigateToReplacement(...) to pass
homeScreenArgs into HomeScreen (use the HomeScreen constructor that accepts
arguments or add one if missing), referencing the computed variables
homeScreenArgs, pendingShortcut and the helper _getScreenIndex and ensure
widget.arguments remains merged; keep the mounted check and remove the debug
print once fixed.
♻️ Duplicate comments (2)
lib/screens/home/home_screen.dart (1)

106-112: Dispose AppShortcutsService callback to avoid leaking HomeScreen.

AppShortcutsService.init(_handleShortcut) likely retains a callback reference. Without cleanup, a disposed HomeScreen can be kept alive.

🛠️ Suggested fix
  `@override`
  void dispose() {
    _messageController.dispose();
    _scrollController.dispose();
    _fabController.dispose();
+   AppShortcutsService.dispose();
    super.dispose();
  }
lib/screens/auth/signup_screen.dart (1)

99-103: Remove logging of userData; it includes plaintext passwords.

userData contains _passwordController.text. Logging it is a security risk.

🛠️ Suggested fix
-        print('[SignupScreen] Navigate to VerifyOTPScreen with userData: $userData');

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@lib/screens/auth/login_screen.dart`:
- Around line 117-121: Replace the user-facing SnackBar that currently
interpolates e.toString() with a non-sensitive, generic error message (e.g.,
"Login failed. Please try again.") in the SnackBar shown in login_screen.dart
(where the SnackBar is created inside the login handler / _login method), and
move the detailed error logging to a separate debug/log statement (use existing
logger or print the caught exception and stack trace) so the full exception
remains in logs but is not shown to the user.
♻️ Duplicate comments (3)
lib/screens/auth/signup_screen.dart (1)

72-72: Critical: Do not log userData containing plaintext passwords.

Line 72 logs userData which includes the password field (set at lines 99 and 129). This exposes credentials in logs and is a serious security risk. Remove or redact sensitive fields before logging.

🔒 Suggested fix
-        print('[SignupScreen] Navigate to VerifyOTPScreen with userData: $userData');
+        // Removed: Do not log userData containing sensitive information

Or if logging is needed for debugging:

-        print('[SignupScreen] Navigate to VerifyOTPScreen with userData: $userData');
+        if (kDebugMode) {
+          debugPrint('[SignupScreen] Navigate to VerifyOTPScreen with email: ${_emailController.text}');
+        }
lib/screens/splash_screen.dart (1)

64-69: Avoid mutating a potentially unmodifiable arguments map.

Line 65 assigns widget.arguments ?? {} directly and then mutates it at lines 67-68. If widget.arguments is an unmodifiable map (e.g., from route settings), this will throw a runtime error. Create a copy first.

🐛 Safer merge
-      // Merge with any widget arguments
-      Map<String, dynamic> homeScreenArgs = widget.arguments ?? {};
-      if (screenIndex != null) {
-        homeScreenArgs['screen'] = screenIndex;
-        homeScreenArgs['initial_route'] = initialShortcut;
-      }
+      // Merge with any widget arguments (create a copy to avoid mutating original)
+      final Map<String, dynamic> homeScreenArgs = {
+        ...?widget.arguments,
+      };
+      if (screenIndex != null) {
+        homeScreenArgs['screen'] = screenIndex;
+        homeScreenArgs['initial_route'] = initialShortcut;
+      }
lib/screens/home/home_screen.dart (1)

64-65: Add cleanup for AppShortcutsService in dispose() to prevent memory leaks.

AppShortcutsService.init(_handleShortcut) stores a callback reference to this widget's method. When HomeScreen is disposed and recreated, the old callback persists, keeping the disposed widget alive. Add a cleanup call in dispose().

🐛 Suggested fix

First, add a dispose method to AppShortcutsService:

// In lib/services/app_shortcuts_service.dart
static void dispose() {
  _onShortcutPressed = null;
}

Then call it in HomeScreen.dispose():

 `@override`
 void dispose() {
   _messageController.dispose();
   _scrollController.dispose();
   _fabController.dispose();
+  AppShortcutsService.dispose();
   super.dispose();
 }
🧹 Nitpick comments (7)
lib/screens/auth/login_screen.dart (2)

33-35: Remove or guard debug print statements.

Multiple print() statements expose internal flow details to logs. Use debugPrint() wrapped in kDebugMode or remove before production.

♻️ Suggested fix
+import 'package:flutter/foundation.dart';
...
-    // Debug arguments
-    print('[LoginScreen] initState called with arguments: ${widget.arguments}');
+    // Debug arguments
+    if (kDebugMode) {
+      debugPrint('[LoginScreen] initState called with arguments: ${widget.arguments}');
+    }
...
-        print('[LoginScreen] Navigating to HomeScreen with args: $homeScreenArgs');
+        if (kDebugMode) {
+          debugPrint('[LoginScreen] Navigating to HomeScreen with args: $homeScreenArgs');
+        }
...
-      print('[LoginScreen] Login error: $e');
+      if (kDebugMode) {
+        debugPrint('[LoginScreen] Login error: $e');
+      }

Also applies to: 94-94, 114-114


128-143: Consider extracting _getScreenIndex to a shared utility.

This route-to-index mapping is duplicated in LoginScreen, HomeScreen, SplashScreen, and VerifyOTPScreen. Centralizing it in AppShortcutsService or a shared constants file would improve maintainability and reduce risk of drift.

♻️ Example centralization
// In lib/services/app_shortcuts_service.dart
class AppShortcutsService {
  // ... existing code ...
  
  static int? getScreenIndexForRoute(String? route) {
    switch (route) {
      case 'dashboard': return 0;
      case 'calendar': return 1;
      case 'workspace': return 2;
      case 'chat': return 3;
      case 'profile': return 4;
      default: return null;
    }
  }
}

Then replace local _getScreenIndex calls with AppShortcutsService.getScreenIndexForRoute(route).

lib/screens/home/home_screen.dart (2)

79-104: Minor: _handleShortcut duplicates route-to-index mapping.

This is the same pattern noted in LoginScreen. Consider using a shared utility method once extracted.


94-94: Missing const on ChatScreen() instantiation.

Line 94 creates ChatScreen() without const, while line 74 uses const ChatScreen(). For consistency and potential performance benefit, consider if const can be applied here (depends on ChatScreen constructor).

♻️ Suggested fix (if ChatScreen has const constructor)
       case 'chat':
         index = 3;
         // Refresh chat screen if needed
-        _screens[3] = ChatScreen();
+        _screens[3] = const ChatScreen();
         break;
lib/screens/auth/signup_screen.dart (2)

5-5: Remove unused import.

app_shortcuts_service.dart is imported but not used anywhere in this file. The shortcut handling is delegated to VerifyOTPScreen via initial_args.

♻️ Suggested fix
-import '../../services/app_shortcuts_service.dart';

35-36: Remove or guard debug print statement.

Same pattern as other files - use debugPrint with kDebugMode guard or remove before production.

lib/screens/auth/verify_otp_screen.dart (1)

133-148: Route-to-index mapping duplicated across files.

Same observation as other files - this helper is now in four places. Consolidating would reduce maintenance burden.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
lib/screens/auth/signup_screen.dart (2)

61-66: Remove duplicate signInWithOtp call.

signInWithOtp is called twice consecutively, which will send two verification emails to the user.

🐛 Suggested fix
     try {
-      await _supabaseService.client.auth.signInWithOtp(email: _emailController.text);
       // Only send signup email without creating user upfront
       await _supabaseService.client.auth.signInWithOtp(
         email: _emailController.text,
       );

142-176: Remove dead code after _sendOtpAndNavigate call.

After calling await _sendOtpAndNavigate(...) on line 142, the code from lines 143-166 duplicates the OTP sending and navigation logic. This code will execute after the navigation has already occurred, potentially causing double navigation or errors.

🐛 Suggested fix
       final userData = {
         'teamId': _teamIdController.text,
         'fullName': _nameController.text,
         'password': _passwordController.text,
       };

       await _sendOtpAndNavigate('signup_join', userData);
-      // Only send signup email without creating user upfront
-      await _supabaseService.client.auth.signInWithOtp(
-        email: _emailController.text,
-      );
-
-      if (mounted) {
-        ScaffoldMessenger.of(context).showSnackBar(
-          const SnackBar(
-            content: Text('Verification email sent. Please check your inbox.'),
-            backgroundColor: Colors.green,
-          ),
-        );
-
-        NavigationService().navigateTo(
-          VerifyOTPScreen(
-            email: _emailController.text,
-            verifyType: 'signup_join',
-            userData: {
-              'teamId': _teamIdController.text,
-              'fullName': _nameController.text,
-              'password': _passwordController.text,
-            },
-          ),
-        );
-      }
     } catch (e) {
lib/screens/splash_screen.dart (1)

149-170: Remove duplicate code blocks (likely merge artifact).

Lines 149-153 and 165-170 appear to be duplicates of the color array and Icon widget definitions, likely left over from a merge conflict.

🐛 Suggested fix
                       gradient: LinearGradient(
-                          colors: [Colors.green.shade400, Colors.green.shade700],
                          colors: [
                            Colors.green.shade400,
                            Colors.green.shade700
                          ],
                          begin: Alignment.topLeft,
                          end: Alignment.bottomRight,
                        ),
...
-                      child: const Icon(Icons.task_alt, size: 80, color: Colors.white),
                      child: const Icon(
                        Icons.task_alt,
                        size: 80,
                        color: Colors.white,
                      ),
🤖 Fix all issues with AI agents
In `@lib/screens/auth/signup_screen.dart`:
- Around line 117-121: There is a duplicate call to _supabaseService.teamExists
using _teamIdController.text in the try block (two consecutive declarations of
teamExists); remove the redundant call and keep a single await
_supabaseService.teamExists(_teamIdController.text) assignment to the teamExists
variable so the API is only called once, leaving any subsequent logic that uses
teamExists unchanged.
- Around line 380-385: The onPressed assignment contains a duplicated ternary
causing a syntax error; replace the duplicated block with a single expression so
onPressed reads: if _isLoading then null else choose between _handleJoinTeam and
_handleCreateTeam based on _tabController.index. Update the widget's onPressed
to use: _isLoading ? null : (_tabController.index == 0 ? _handleJoinTeam :
_handleCreateTeam), referencing the onPressed property and the functions
_handleJoinTeam, _handleCreateTeam and _tabController.

In `@lib/screens/splash_screen.dart`:
- Around line 48-52: Remove the duplicate Timer so _checkSession isn't invoked
twice: keep only one Timer invocation that schedules _checkSession (choose
either Timer(const Duration(seconds: 3), _checkSession) or Timer(const
Duration(milliseconds: 1500), () { _checkSession(); }) and delete the other),
ensuring there is a single scheduled call to _checkSession in the splash screen
code (look for usages of Timer, Duration, and the _checkSession method).
♻️ Duplicate comments (3)
lib/screens/auth/login_screen.dart (1)

115-123: Avoid exposing raw exception details in user-facing error messages.

e.toString() may leak sensitive implementation details (stack traces, internal paths, Supabase error internals) to users. Display a generic message instead.

🔒 Suggested fix
     } catch (e) {
-      print('[LoginScreen] Login error: $e');
+      debugPrint('[LoginScreen] Login error: $e');
       if (mounted) {
         ScaffoldMessenger.of(context).showSnackBar(
-          SnackBar(
-            content: Text('Login failed: ${e.toString()}'),
+          const SnackBar(
+            content: Text('Login failed. Please check your credentials and try again.'),
             backgroundColor: Colors.red,
           ),
         );
       }
lib/screens/auth/signup_screen.dart (1)

76-79: Do not log credentials or sensitive signup data.

userData includes plaintext passwords; logging it is a security risk. Remove or redact sensitive fields before logging.

🔒 Suggested fix
         // Merge existing arguments into userData
         if (widget.arguments != null) userData['initial_args'] = widget.arguments;

-        print('[SignupScreen] Navigate to VerifyOTPScreen with userData: $userData');
+        debugPrint('[SignupScreen] Navigating to VerifyOTPScreen');
lib/screens/splash_screen.dart (1)

67-72: Create a copy of widget.arguments before mutation.

Assigning widget.arguments ?? {} and then mutating it can fail if the original map is unmodifiable, or cause unintended side effects. Use a spread to create a mutable copy.

🐛 Suggested fix
-      Map<String, dynamic> homeScreenArgs = widget.arguments ?? {};
+      final Map<String, dynamic> homeScreenArgs = {
+        ...?widget.arguments,
+      };
       if (screenIndex != null) {
         homeScreenArgs['screen'] = screenIndex;
         homeScreenArgs['initial_route'] = initialShortcut;
       }
🧹 Nitpick comments (6)
android/app/src/main/AndroidManifest.xml (1)

27-33: Consider restoring NormalTheme meta-data for smooth startup.

Commenting out the NormalTheme meta-data may cause a visual flash or flicker during Flutter engine initialization, as the LaunchTheme will persist until the Flutter UI renders. The shortcuts meta-data can coexist with the theme meta-data.

Also, minor formatting inconsistency: the shortcuts meta-data has extra leading spaces compared to other meta-data blocks.

♻️ Suggested fix
-            <!-- <meta-data
-              android:name="io.flutter.embedding.android.NormalTheme"
-              android:resource="@style/NormalTheme"
-              /> -->
-              <meta-data
-                android:name="android.app.shortcuts"
-                android:resource="@xml/shortcuts"/>
+            <meta-data
+              android:name="io.flutter.embedding.android.NormalTheme"
+              android:resource="@style/NormalTheme"
+              />
+            <meta-data
+              android:name="android.app.shortcuts"
+              android:resource="@xml/shortcuts"/>
lib/screens/auth/login_screen.dart (3)

35-37: Replace print() with conditional debug logging.

Using print() in production can leak debugging information and pollute logs. Use debugPrint() or wrap in kDebugMode check for consistency with Flutter best practices.

♻️ Suggested fix
-    // Debug arguments
-    print('[LoginScreen] initState called with arguments: ${widget.arguments}');
+    // Debug arguments
+    debugPrint('[LoginScreen] initState called with arguments: ${widget.arguments}');

96-96: Remove or guard debug print before production.

Similar to the above, this debug print should use debugPrint() and ideally be removed or wrapped in a debug mode check before release.


130-144: Consider centralizing _getScreenIndex to avoid duplication.

This helper is duplicated in login_screen.dart and splash_screen.dart. Extract it to a shared utility (e.g., in AppShortcutsService or a dedicated helper) to maintain a single source of truth for route-to-index mapping.

lib/screens/auth/signup_screen.dart (1)

6-6: Remove unused import.

app_shortcuts_service.dart is imported but not used in this file.

♻️ Suggested fix
-import '../../services/app_shortcuts_service.dart';
lib/screens/splash_screen.dart (1)

75-75: Remove unused variable args.

The variable args is assigned but never used.

♻️ Suggested fix
-      final args = ModalRoute.of(context)?.settings.arguments;

Comment on lines 380 to 385
onPressed: _isLoading
? null
: (_tabController.index == 0 ? _handleJoinTeam : _handleCreateTeam),
: (_tabController.index == 0
? _handleJoinTeam
: _handleCreateTeam),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix duplicate ternary expression causing syntax error.

Lines 382-385 contain a duplicate ternary expression that will cause a compilation error.

🐛 Suggested fix
         CustomButton(
           text: _tabController.index == 0 ? 'Join Team' : 'Create Team',
           onPressed: _isLoading
               ? null
               : (_tabController.index == 0 ? _handleJoinTeam : _handleCreateTeam),
-              : (_tabController.index == 0
-                  ? _handleJoinTeam
-                  : _handleCreateTeam),
           isLoading: _isLoading,
         ),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
onPressed: _isLoading
? null
: (_tabController.index == 0 ? _handleJoinTeam : _handleCreateTeam),
: (_tabController.index == 0
? _handleJoinTeam
: _handleCreateTeam),
onPressed: _isLoading
? null
: (_tabController.index == 0 ? _handleJoinTeam : _handleCreateTeam),
🤖 Prompt for AI Agents
In `@lib/screens/auth/signup_screen.dart` around lines 380 - 385, The onPressed
assignment contains a duplicated ternary causing a syntax error; replace the
duplicated block with a single expression so onPressed reads: if _isLoading then
null else choose between _handleJoinTeam and _handleCreateTeam based on
_tabController.index. Update the widget's onPressed to use: _isLoading ? null :
(_tabController.index == 0 ? _handleJoinTeam : _handleCreateTeam), referencing
the onPressed property and the functions _handleJoinTeam, _handleCreateTeam and
_tabController.

- Remove duplicate teamExists call in signup screen
- Fix security issues by not exposing raw exceptions in error messages
- Fix duplicate timer in splash screen
- Avoid mutating unmodifiable map by copying arguments
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@lib/screens/splash_screen.dart`:
- Around line 135-160: The Image.asset call in splash_screen.dart uses a
non-existent path 'ELL-ena-logo/png/logo-removed-bg-cropped.png' and assets are
not declared in pubspec.yaml; fix by updating the Image.asset path to the
correct existing filename (e.g., the actual 'ELL-ena-logo/png/logo removed
bg.png' or rename the file to match), then add that asset (or its folder) under
the assets: section in pubspec.yaml so it is bundled; keep the existing
errorBuilder for safety but ensure the primary asset path used by Image.asset is
accurate and declared.
♻️ Duplicate comments (2)
lib/screens/splash_screen.dart (1)

98-115: Duplicate _getScreenIndex implementation.

Same mapping logic exists in login_screen.dart, home_screen.dart, and verify_otp_screen.dart. Consider centralizing in AppShortcutsService to avoid maintenance burden and potential inconsistencies.

lib/screens/auth/signup_screen.dart (1)

70-72: Do not log userData containing passwords.

This print statement logs the entire userData map which includes the password field (set at lines 103 and 134). This is a security risk as credentials could end up in logs. Either remove the log entirely or redact sensitive fields.

Suggested fix
         // Merge existing arguments into userData
         if (widget.arguments != null) userData['initial_args'] = widget.arguments;

-        print('[SignupScreen] Navigate to VerifyOTPScreen with userData: $userData');
+        // Log navigation without sensitive data
+        debugPrint('[SignupScreen] Navigate to VerifyOTPScreen for verifyType: $verifyType');
🧹 Nitpick comments (5)
lib/screens/auth/login_screen.dart (4)

33-35: Replace print with debugPrint or guard with kDebugMode.

Production builds should not emit debug logs. Use debugPrint (which is already used in splash_screen.dart) or wrap with kDebugMode check for consistency.

Suggested fix
-    // Debug arguments
-    print('[LoginScreen] initState called with arguments: ${widget.arguments}');
+    // Debug arguments
+    debugPrint('[LoginScreen] initState called with arguments: ${widget.arguments}');

94-94: Use debugPrint instead of print for debug logging.

Same as above - use debugPrint for consistency with other screens and proper production behavior.

-        print('[LoginScreen] Navigating to HomeScreen with args: $homeScreenArgs');
+        debugPrint('[LoginScreen] Navigating to HomeScreen with args: $homeScreenArgs');

114-114: Use debugPrint for error logging.

-      print('[LoginScreen] Login error: $e'); // Log for debugging
+      debugPrint('[LoginScreen] Login error: $e'); // Log for debugging

128-143: Consider extracting _getScreenIndex to a shared utility.

This exact mapping logic is duplicated in login_screen.dart, signup_screen.dart (via verify_otp_screen), splash_screen.dart, and home_screen.dart. Extract to AppShortcutsService or a shared utility to maintain a single source of truth for route-to-index mapping.

#!/bin/bash
# Verify duplication of _getScreenIndex across files
rg -n "_getScreenIndex" --type dart -A 15
lib/screens/auth/signup_screen.dart (1)

36-36: Replace print with debugPrint for debug logging.

Use debugPrint consistently across the codebase for production-safe logging.

Suggested fix
-    print('[SignupScreen] initState called with arguments: ${widget.arguments}');
+    debugPrint('[SignupScreen] initState called with arguments: ${widget.arguments}');
-      print('[SignupScreen] Error sending OTP: $e'); // Log for debugging
+      debugPrint('[SignupScreen] Error sending OTP: $e'); // Log for debugging
-      print('[SignupScreen] Error checking team: $e'); // Log for debugging
+      debugPrint('[SignupScreen] Error checking team: $e'); // Log for debugging

Also applies to: 83-83, 139-139

Comment on lines 135 to 160
Image.asset(
'ELL-ena-logo/png/logo-removed-bg-cropped.png',
width: 250,
height: 250,
errorBuilder: (context, error, stackTrace) {
return Container(
padding: const EdgeInsets.all(20),
decoration: BoxDecoration(
shape: BoxShape.circle,
gradient: LinearGradient(
colors: [Colors.green.shade400, Colors.green.shade700],
begin: Alignment.topLeft,
end: Alignment.bottomRight,
),
boxShadow: [
BoxShadow(
color: Colors.green.withOpacity(0.3),
blurRadius: 20,
spreadRadius: 5,
),
],
),
],
),
child: const Icon(
Icons.task_alt,
size: 80,
color: Colors.white,
),
child: const Icon(Icons.task_alt, size: 80, color: Colors.white),
);
},
),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the asset exists and is declared in pubspec.yaml
fd -t f "logo-removed-bg-cropped.png"
echo "---"
rg "ELL-ena-logo" pubspec.yaml
echo "---"
# Also check the overall directory structure for assets
ls -la | grep -i "asset\|pubspec"
echo "---"
# Check if there's an assets directory
find . -maxdepth 3 -type d -name "*asset*" -o -type d -name "ELL-ena-logo" 2>/dev/null

Repository: AOSSIE-Org/Ell-ena

Length of output: 276


🏁 Script executed:

#!/bin/bash
# Check what's in the ELL-ena-logo directory
echo "=== ELL-ena-logo directory contents ==="
find ./ELL-ena-logo -type f | head -20
echo ""
echo "=== pubspec.yaml assets section ==="
sed -n '/^assets:/,/^[a-z]/p' pubspec.yaml

Repository: AOSSIE-Org/Ell-ena

Length of output: 571


Asset path does not exist and is not declared in pubspec.yaml.

The file 'ELL-ena-logo/png/logo-removed-bg-cropped.png' does not exist in the project. The closest match is 'ELL-ena-logo/png/logo removed bg.png' (note the spaces instead of hyphens). Additionally, no assets are declared in the pubspec.yaml file, so even if the file path were correct, it would not be bundled with the app and the errorBuilder would always trigger in production.

Either update the path to match an existing file and add it to pubspec.yaml under the assets: section, or confirm this fallback is intentional.

🤖 Prompt for AI Agents
In `@lib/screens/splash_screen.dart` around lines 135 - 160, The Image.asset call
in splash_screen.dart uses a non-existent path
'ELL-ena-logo/png/logo-removed-bg-cropped.png' and assets are not declared in
pubspec.yaml; fix by updating the Image.asset path to the correct existing
filename (e.g., the actual 'ELL-ena-logo/png/logo removed bg.png' or rename the
file to match), then add that asset (or its folder) under the assets: section in
pubspec.yaml so it is bundled; keep the existing errorBuilder for safety but
ensure the primary asset path used by Image.asset is accurate and declared.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@android/app/src/main/AndroidManifest.xml`:
- Around line 27-30: Restore the commented-out NormalTheme meta-data in
AndroidManifest by re-adding the <meta-data> element that sets
android:name="io.flutter.embedding.android.NormalTheme" and
android:resource="@style/NormalTheme" so the launch-to-Flutter transition uses
the NormalTheme; this is unrelated to the app shortcuts change so remove the
comment markers around the NormalTheme meta-data (the block containing
NormalTheme) to prevent startup flicker.

In `@lib/screens/splash_screen.dart`:
- Around line 48-87: The scheduled Timer that calls _checkSession must be
tracked and canceled on widget disposal and _checkSession should verify the
State is still mounted before using context/Navigator; store the Timer returned
when scheduling the Timer(const Duration(milliseconds: 1500), _checkSession) in
a field (e.g., _sessionTimer), cancel it in dispose(), and at the start (or
before any Navigator.pushReplacement) of _checkSession check mounted (or if
(!mounted) return) to avoid navigating with a disposed context.
♻️ Duplicate comments (1)
lib/screens/auth/signup_screen.dart (1)

71-75: Remove logging of userData containing passwords.

Line 74 logs userData, which includes plaintext passwords (Lines 103–106 / 133–137). This leaks credentials into logs. Please remove or redact the log.

🔒 Safe fix (remove or redact)
-        print('[SignupScreen] Navigate to VerifyOTPScreen with userData: $userData');
+        // Avoid logging sensitive signup data (passwords, tokens).
🧹 Nitpick comments (3)
android/app/src/main/AndroidManifest.xml (2)

31-34: Shortcuts meta-data correctly placed; minor indentation nit.

The android.app.shortcuts declaration is correctly positioned inside the launcher activity. However, the indentation is inconsistent with the surrounding XML (uses extra leading spaces).

Optional: align indentation
-              <meta-data
-                android:name="android.app.shortcuts"
-                android:resource="@xml/shortcuts"/>
-
+            <meta-data
+                android:name="android.app.shortcuts"
+                android:resource="@xml/shortcuts"/>

51-52: Extraneous blank lines.

These extra blank lines between </activity> and the flutterEmbedding comment appear to be accidental whitespace additions.

lib/screens/splash_screen.dart (1)

98-114: Consider centralizing shortcut-to-index mapping.

_getScreenIndex is duplicated (also appears in LoginScreen). Centralizing this mapping (e.g., in AppShortcutsService) avoids drift when adding or renaming routes.

Comment on lines +27 to +30
<!-- <meta-data
android:name="io.flutter.embedding.android.NormalTheme"
android:resource="@style/NormalTheme"
/>
/> -->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Unintended change: NormalTheme meta-data commented out.

This appears unrelated to the app shortcuts feature. The NormalTheme meta-data ensures a smooth visual transition from the launch splash to the Flutter UI. Commenting it out can cause a jarring flash or flicker during app startup.

If this was intentional, please clarify the reasoning; otherwise, restore it.

Suggested fix
-            <!-- <meta-data
+            <meta-data
               android:name="io.flutter.embedding.android.NormalTheme"
               android:resource="@style/NormalTheme"
-              /> -->
+              />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<!-- <meta-data
android:name="io.flutter.embedding.android.NormalTheme"
android:resource="@style/NormalTheme"
/>
/> -->
<meta-data
android:name="io.flutter.embedding.android.NormalTheme"
android:resource="@style/NormalTheme"
/>
🤖 Prompt for AI Agents
In `@android/app/src/main/AndroidManifest.xml` around lines 27 - 30, Restore the
commented-out NormalTheme meta-data in AndroidManifest by re-adding the
<meta-data> element that sets
android:name="io.flutter.embedding.android.NormalTheme" and
android:resource="@style/NormalTheme" so the launch-to-Flutter transition uses
the NormalTheme; this is unrelated to the app shortcuts change so remove the
comment markers around the NormalTheme meta-data (the block containing
NormalTheme) to prevent startup flicker.

Comment on lines +48 to 87
// Give a small delay to ensure everything is initialized
Timer(const Duration(milliseconds: 1500), _checkSession);
}

Future<void> _checkSession() async {
try {
final currentUser = _supabaseService.client.auth.currentUser;

final args = ModalRoute.of(context)?.settings.arguments;
// Get initial shortcut from AppShortcutsService
final initialShortcut = AppShortcutsService.getPendingShortcut();
debugPrint('[SplashScreen] Initial shortcut: $initialShortcut');

// Convert shortcut to screen index
int? screenIndex = _getScreenIndex(initialShortcut);
debugPrint('[SplashScreen] Converted screen index: $screenIndex');

// Create a copy of arguments to avoid mutating potentially unmodifiable map
Map<String, dynamic> homeScreenArgs = Map.from(widget.arguments ?? {});
if (screenIndex != null) {
homeScreenArgs['screen'] = screenIndex;
homeScreenArgs['initial_route'] = initialShortcut;
}

debugPrint('[SplashScreen] HomeScreen arguments: $homeScreenArgs');

if (currentUser != null) {
if (args != null && args is Map<String, dynamic>) {
Navigator.of(context).pushReplacement(
MaterialPageRoute(
builder: (context) => HomeScreen(arguments: args),
),
);
} else {
Navigator.of(context).pushReplacement(
MaterialPageRoute(
builder: (context) => const HomeScreen(),
),
);
}
// User is logged in, go to HomeScreen
Navigator.of(context).pushReplacement(
MaterialPageRoute(
builder: (context) => HomeScreen(arguments: homeScreenArgs),
),
);
} else {
// User is not logged in, go to LoginScreen
Navigator.of(context).pushReplacement(
MaterialPageRoute(
builder: (context) => const LoginScreen(),
builder: (context) => LoginScreen(arguments: homeScreenArgs),
),
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard delayed navigation against disposed state.

Line 48 schedules _checkSession via Timer but the timer isn’t canceled and _checkSession navigates without a mounted guard. If the widget is disposed before the timer fires, this can throw when using context.

🛠️ Suggested fix (track/cancel timer + mounted guard)
 class _SplashScreenState extends State<SplashScreen>
     with SingleTickerProviderStateMixin {
   late AnimationController _animationController;
   late Animation<double> _fadeAnimation;
   late Animation<double> _scaleAnimation;
   final _supabaseService = SupabaseService();
+  Timer? _startupTimer;
 ...
-    Timer(const Duration(milliseconds: 1500), _checkSession);
+    _startupTimer = Timer(const Duration(milliseconds: 1500), _checkSession);
   }
 
   Future<void> _checkSession() async {
     try {
+      if (!mounted) return;
       final currentUser = _supabaseService.client.auth.currentUser;
 ...
-      if (currentUser != null) {
+      if (!mounted) return;
+      if (currentUser != null) {
         Navigator.of(context).pushReplacement(
 ...
       } else {
         Navigator.of(context).pushReplacement(
 ...
       }
     } catch (e) {
       debugPrint('[SplashScreen] Error checking session: $e');
       Navigator.of(context).pushReplacement(
 ...
     }
   }
 ...
   void dispose() {
+    _startupTimer?.cancel();
     _animationController.dispose();
     super.dispose();
   }
🤖 Prompt for AI Agents
In `@lib/screens/splash_screen.dart` around lines 48 - 87, The scheduled Timer
that calls _checkSession must be tracked and canceled on widget disposal and
_checkSession should verify the State is still mounted before using
context/Navigator; store the Timer returned when scheduling the Timer(const
Duration(milliseconds: 1500), _checkSession) in a field (e.g., _sessionTimer),
cancel it in dispose(), and at the start (or before any
Navigator.pushReplacement) of _checkSession check mounted (or if (!mounted)
return) to avoid navigating with a disposed context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Proposal: Native Android App Shortcuts for Faster Navigation

1 participant