From ee09a95caddc94aba074ce54c92f31fff21f23ca Mon Sep 17 00:00:00 2001 From: Bretton <36870434+BrettM86@users.noreply.github.com> Date: Tue, 21 Jul 2026 21:58:18 -0700 Subject: [PATCH] fix(compose): save reply drafts on system back and harden shell back-guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Section-4 second-opinion findings (3 reviewers): - ReplyScreen saves the draft on any pop (PopScope), not just the Cancel button — system/predictive back no longer silently discards typed text; submit's pop is guarded so a cleared draft can't resurrect - ReplyScreen follows the comments-provider cache contract (acquire in initState, release in dispose) so LRU eviction can't dispose a provider its Consumers still listen to; auth listener removal no longer does an ancestor lookup in dispose - shell back-guard re-checks Create-tab + dirty before switching tabs, so a pop blocked by any future descendant PopScope can't yank to Feed; IndexedStack/create-tab coupling pinned by an assert - profile block seed runs only when the profile loaded without error - Play-video overlay semantics reflect the loading-disabled state - tests: back-press matrix for the shell (4), reply-draft round-trip (5), compose dirty-callback + URL-capitalization assertions Co-Authored-By: Claude Fable 5 --- lib/screens/compose/reply_screen.dart | 202 ++++++++++------- lib/screens/home/create_post_screen.dart | 137 +++++------ lib/screens/home/main_shell_screen.dart | 199 ++++++++-------- lib/screens/home/profile_screen.dart | 34 +-- lib/widgets/post_card.dart | 3 + test/screens/create_post_screen_test.dart | 68 +++++- test/screens/main_shell_screen_test.dart | 260 +++++++++++++++++++++ test/screens/reply_screen_test.dart | 265 ++++++++++++++++++++++ 8 files changed, 901 insertions(+), 267 deletions(-) create mode 100644 test/screens/main_shell_screen_test.dart create mode 100644 test/screens/reply_screen_test.dart diff --git a/lib/screens/compose/reply_screen.dart b/lib/screens/compose/reply_screen.dart index d54bf7d..f8f4b05 100644 --- a/lib/screens/compose/reply_screen.dart +++ b/lib/screens/compose/reply_screen.dart @@ -12,6 +12,7 @@ import '../../models/comment.dart'; import '../../models/post.dart'; import '../../providers/auth_provider.dart'; import '../../providers/comments_provider.dart'; +import '../../services/comments_provider_cache.dart'; import '../../utils/facet_detector.dart'; import '../../widgets/comment_thread.dart'; import '../../widgets/post_card.dart'; @@ -50,7 +51,8 @@ class ReplyScreen extends StatefulWidget { final ThreadViewComment? comment; /// Callback when user submits reply - final Future Function(String content, List facets) onSubmit; + final Future Function(String content, List facets) + onSubmit; /// CommentsProvider for draft save/restore and time updates final CommentsProvider commentsProvider; @@ -70,6 +72,8 @@ class _ReplyScreenState extends State with WidgetsBindingObserver { double _lastKeyboardHeight = 0; Timer? _bannerDismissTimer; FlutterView? _cachedView; + CommentsProviderCache? _commentsCache; + AuthProvider? _authProvider; @override void initState() { @@ -78,6 +82,13 @@ class _ReplyScreenState extends State with WidgetsBindingObserver { _textController.addListener(_onTextChanged); _focusNode.addListener(_onFocusChanged); + // Pin the provider in the cache (acquire/release contract from + // CommentsProviderCache): without a pin, LRU eviction could dispose + // widget.commentsProvider while _ContextPreview's Consumer is still + // listening — e.g. if the owning detail route is removed under this + // reply route. + _acquireProviderPin(); + // Restore draft and autofocus after frame is built WidgetsBinding.instance.addPostFrameCallback((_) { if (mounted) { @@ -103,9 +114,24 @@ class _ReplyScreenState extends State with WidgetsBindingObserver { _cachedView = View.of(context); } + void _acquireProviderPin() { + try { + _commentsCache = context.read() + ..acquireProvider( + postUri: widget.commentsProvider.postUri, + postCid: widget.commentsProvider.postCid, + ); + } on ProviderNotFoundException { + // Expected in tests - the cache may not be available + } + } + void _setupAuthListener() { try { - context.read().addListener(_onAuthChanged); + // Keep a reference so dispose() can remove the listener without an + // ancestor lookup (context.read is unsafe on a deactivated element) + _authProvider = context.read() + ..addListener(_onAuthChanged); } on ProviderNotFoundException { // Expected in tests - AuthProvider may not be available } on Exception catch (e) { @@ -201,11 +227,9 @@ class _ReplyScreenState extends State with WidgetsBindingObserver { @override void dispose() { _bannerDismissTimer?.cancel(); - try { - context.read().removeListener(_onAuthChanged); - } on Exception { - // AuthProvider may not be available - } + _authProvider?.removeListener(_onAuthChanged); + // Release the cache pin so the provider becomes evictable again. + _commentsCache?.releaseProvider(widget.commentsProvider.postUri); WidgetsBinding.instance.removeObserver(this); _textController.dispose(); _focusNode.dispose(); @@ -330,93 +354,105 @@ class _ReplyScreenState extends State with WidgetsBindingObserver { // Provide CommentsProvider to descendant widgets (Consumer in _ContextPreview) return ChangeNotifierProvider.value( value: widget.commentsProvider, - child: GestureDetector( - onTap: () { - // Dismiss keyboard when tapping outside - FocusManager.instance.primaryFocus?.unfocus(); + // System back / predictive back must save the draft just like the + // app-bar Cancel button. Never save in dispose() instead: after a + // successful submit the controller still holds the submitted text and + // a dispose-save would resurrect the just-cleared draft — hence the + // !_isSubmitting guard (submit pops while _isSubmitting is true). + child: PopScope( + onPopInvokedWithResult: (didPop, result) { + if (didPop && !_isSubmitting) { + _saveDraft(); + } }, - child: Scaffold( - backgroundColor: AppColors.background, - resizeToAvoidBottomInset: false, // Thunder approach - appBar: AppBar( - backgroundColor: AppColors.background, - surfaceTintColor: Colors.transparent, - foregroundColor: AppColors.textPrimary, - elevation: 0, - automaticallyImplyLeading: false, - leading: TextButton( - onPressed: _handleCancel, - child: const Text( - 'Cancel', - style: TextStyle(color: AppColors.textPrimary, fontSize: 16), + child: GestureDetector( + onTap: () { + // Dismiss keyboard when tapping outside + FocusManager.instance.primaryFocus?.unfocus(); + }, + child: Scaffold( + backgroundColor: AppColors.background, + resizeToAvoidBottomInset: false, // Thunder approach + appBar: AppBar( + backgroundColor: AppColors.background, + surfaceTintColor: Colors.transparent, + foregroundColor: AppColors.textPrimary, + elevation: 0, + automaticallyImplyLeading: false, + leading: TextButton( + onPressed: _handleCancel, + child: const Text( + 'Cancel', + style: TextStyle(color: AppColors.textPrimary, fontSize: 16), + ), + ), + leadingWidth: 80, ), - ), - leadingWidth: 80, - ), - body: Column( - children: [ - // Scrollable content area (Thunder style) - Expanded( - child: SingleChildScrollView( - controller: _scrollController, - padding: const EdgeInsets.only(bottom: 16), - child: Column( - children: [ - // Post or comment preview - _buildContext(), - - const SizedBox(height: 8), - - // Divider between post and text input - Container(height: 1, color: AppColors.border), - - // Text input - no background box, types directly into - // main area - Padding( - padding: const EdgeInsets.all(16), - child: TextField( - controller: _textController, - focusNode: _focusNode, - maxLines: null, - minLines: 8, - keyboardType: TextInputType.multiline, - textCapitalization: TextCapitalization.sentences, - textInputAction: TextInputAction.newline, - style: const TextStyle( - color: AppColors.textPrimary, - fontSize: 16, - height: 1.4, - ), - decoration: const InputDecoration( - hintText: 'Say something...', - hintStyle: TextStyle( - color: AppColors.textSecondary, - fontSize: 16, + body: Column( + children: [ + // Scrollable content area (Thunder style) + Expanded( + child: SingleChildScrollView( + controller: _scrollController, + padding: const EdgeInsets.only(bottom: 16), + child: Column( + children: [ + // Post or comment preview + _buildContext(), + + const SizedBox(height: 8), + + // Divider between post and text input + Container(height: 1, color: AppColors.border), + + // Text input - no background box, types directly into + // main area + Padding( + padding: const EdgeInsets.all(16), + child: TextField( + controller: _textController, + focusNode: _focusNode, + maxLines: null, + minLines: 8, + keyboardType: TextInputType.multiline, + textCapitalization: TextCapitalization.sentences, + textInputAction: TextInputAction.newline, + style: const TextStyle( + color: AppColors.textPrimary, + fontSize: 16, + height: 1.4, + ), + decoration: const InputDecoration( + hintText: 'Say something...', + hintStyle: TextStyle( + color: AppColors.textSecondary, + fontSize: 16, + ), + border: InputBorder.none, + contentPadding: EdgeInsets.zero, + ), ), - border: InputBorder.none, - contentPadding: EdgeInsets.zero, ), - ), + ], ), - ], + ), ), - ), - ), - // Divider - simple straight line like posts and comments - Container(height: 1, color: AppColors.border), + // Divider - simple straight line like posts and comments + Container(height: 1, color: AppColors.border), - _ReplyToolbar( - hasText: _hasText, - isSubmitting: _isSubmitting, - onImageTap: _handleImageTap, - onMentionTap: _handleMentionTap, - onSubmit: _handleSubmit, + _ReplyToolbar( + hasText: _hasText, + isSubmitting: _isSubmitting, + onImageTap: _handleImageTap, + onMentionTap: _handleMentionTap, + onSubmit: _handleSubmit, + ), + ], ), - ], + ), ), ), - ), ); } diff --git a/lib/screens/home/create_post_screen.dart b/lib/screens/home/create_post_screen.dart index 97c48dc..72269d4 100644 --- a/lib/screens/home/create_post_screen.dart +++ b/lib/screens/home/create_post_screen.dart @@ -41,7 +41,11 @@ const int kContentMaxLength = 10000; /// - Loading states and error handling /// - Keyboard handling with scroll support class CreatePostScreen extends StatefulWidget { - const CreatePostScreen({this.onNavigateToFeed, this.onDirtyChanged, super.key}); + const CreatePostScreen({ + this.onNavigateToFeed, + this.onDirtyChanged, + super.key, + }); /// Callback to navigate to feed tab (used when in tab navigation) final VoidCallback? onNavigateToFeed; @@ -80,6 +84,9 @@ class _CreatePostScreenState extends State } /// True when any text field holds user input (a draft worth protecting). + /// + /// Deliberately considers trimmed text only: community, NSFW, and language + /// selections are cheap to redo and alone are not worth blocking back for. bool get _hasUnsavedInput { return _titleController.text.trim().isNotEmpty || _bodyController.text.trim().isNotEmpty || @@ -143,9 +150,7 @@ class _CreatePostScreenState extends State Future _selectCommunity() async { final result = await Navigator.push( context, - MaterialPageRoute( - builder: (context) => const CommunityPickerScreen(), - ), + MaterialPageRoute(builder: (context) => const CommunityPickerScreen()), ); if (result != null && mounted) { @@ -180,9 +185,7 @@ class _CreatePostScreenState extends State if (url.isNotEmpty) { // Validate URL final uri = Uri.tryParse(url); - if (uri == null || - !uri.hasScheme || - (!uri.scheme.startsWith('http'))) { + if (uri == null || !uri.hasScheme || (!uri.scheme.startsWith('http'))) { if (mounted) { ScaffoldMessenger.of(context).showSnackBar( SnackBar( @@ -200,9 +203,10 @@ class _CreatePostScreenState extends State embed = ExternalEmbedInput( uri: url, - title: _titleController.text.trim().isNotEmpty - ? _titleController.text.trim() - : null, + title: + _titleController.text.trim().isNotEmpty + ? _titleController.text.trim() + : null, ); } @@ -214,16 +218,18 @@ class _CreatePostScreenState extends State // Detect link facets in the body content final bodyContent = _bodyController.text.trim(); - final facets = bodyContent.isNotEmpty - ? FacetDetector.detectLinks(bodyContent) - : null; + final facets = + bodyContent.isNotEmpty + ? FacetDetector.detectLinks(bodyContent) + : null; // Create post final response = await apiService.createPost( community: _selectedCommunity!.did, - title: _titleController.text.trim().isNotEmpty - ? _titleController.text.trim() - : null, + title: + _titleController.text.trim().isNotEmpty + ? _titleController.text.trim() + : null, content: bodyContent.isNotEmpty ? bodyContent : null, facets: facets, embed: embed, @@ -245,10 +251,9 @@ class _CreatePostScreenState extends State await Navigator.push( context, MaterialPageRoute( - builder: (context) => PostDetailScreen( - post: optimisticPost, - isOptimistic: true, - ), + builder: + (context) => + PostDetailScreen(post: optimisticPost, isOptimistic: true), ), ); } @@ -309,9 +314,10 @@ class _CreatePostScreenState extends State type: EmbedTypes.external, external: ExternalEmbed( uri: url, - title: _titleController.text.trim().isNotEmpty - ? _titleController.text.trim() - : null, + title: + _titleController.text.trim().isNotEmpty + ? _titleController.text.trim() + : null, ), data: { r'$type': EmbedTypes.external, @@ -347,16 +353,12 @@ class _CreatePostScreenState extends State indexedAt: now, record: PostRecord( content: _bodyController.text.trim(), - title: _titleController.text.trim().isNotEmpty - ? _titleController.text.trim() - : null, - ), - stats: PostStats( - upvotes: 0, - downvotes: 0, - score: 0, - commentCount: 0, + title: + _titleController.text.trim().isNotEmpty + ? _titleController.text.trim() + : null, ), + stats: PostStats(upvotes: 0, downvotes: 0, score: 0, commentCount: 0), embed: embed, viewer: ViewerState(), ), @@ -373,8 +375,8 @@ class _CreatePostScreenState extends State // system back shell-wide. The shell owns back handling and only intercepts // when this tab is active with unsaved input (see onDirtyChanged). return Scaffold( - backgroundColor: AppColors.background, - appBar: AppBar( + backgroundColor: AppColors.background, + appBar: AppBar( backgroundColor: AppColors.background, surfaceTintColor: Colors.transparent, foregroundColor: AppColors.textPrimary, @@ -399,9 +401,10 @@ class _CreatePostScreenState extends State child: TextButton( onPressed: _isFormValid && !_isSubmitting ? _handleSubmit : null, style: TextButton.styleFrom( - backgroundColor: _isFormValid && !_isSubmitting - ? AppColors.primary - : AppColors.textSecondary.withValues(alpha: 0.3), + backgroundColor: + _isFormValid && !_isSubmitting + ? AppColors.primary + : AppColors.textSecondary.withValues(alpha: 0.3), foregroundColor: AppColors.textPrimary, padding: const EdgeInsets.symmetric( horizontal: 16, @@ -486,16 +489,12 @@ class _CreatePostScreenState extends State Row( children: [ // Language dropdown - Expanded( - child: _buildLanguageDropdown(), - ), + Expanded(child: _buildLanguageDropdown()), const SizedBox(width: 16), // NSFW toggle - Expanded( - child: _buildNsfwToggle(), - ), + Expanded(child: _buildNsfwToggle()), ], ), @@ -533,14 +532,13 @@ class _CreatePostScreenState extends State _selectedCommunity?.displayName ?? _selectedCommunity?.name ?? 'Select a community', - style: - TextStyle( - color: - _selectedCommunity != null - ? AppColors.textPrimary - : AppColors.textSecondary, - fontSize: 16, - ), + style: TextStyle( + color: + _selectedCommunity != null + ? AppColors.textPrimary + : AppColors.textSecondary, + fontSize: 16, + ), maxLines: 1, overflow: TextOverflow.ellipsis, ), @@ -560,18 +558,11 @@ class _CreatePostScreenState extends State Widget _buildUserInfo(String handle) { return Row( children: [ - const Icon( - Icons.person, - color: AppColors.textSecondary, - size: 16, - ), + const Icon(Icons.person, color: AppColors.textSecondary, size: 16), const SizedBox(width: 8), Text( '@$handle', - style: const TextStyle( - color: AppColors.textSecondary, - fontSize: 14, - ), + style: const TextStyle(color: AppColors.textSecondary, fontSize: 14), ), ], ); @@ -591,9 +582,11 @@ class _CreatePostScreenState extends State // For multiline fields, use newline action and multiline keyboard final isMultiline = minLines != null && minLines > 1; final effectiveKeyboardType = - keyboardType ?? (isMultiline ? TextInputType.multiline : TextInputType.text); + keyboardType ?? + (isMultiline ? TextInputType.multiline : TextInputType.text); final effectiveTextInputAction = - textInputAction ?? (isMultiline ? TextInputAction.newline : TextInputAction.next); + textInputAction ?? + (isMultiline ? TextInputAction.newline : TextInputAction.next); return TextField( controller: controller, @@ -604,10 +597,7 @@ class _CreatePostScreenState extends State keyboardType: effectiveKeyboardType, textInputAction: effectiveTextInputAction, textCapitalization: textCapitalization, - style: const TextStyle( - color: AppColors.textPrimary, - fontSize: 16, - ), + style: const TextStyle(color: AppColors.textPrimary, fontSize: 16), decoration: InputDecoration( hintText: hintText, hintStyle: const TextStyle(color: Color(0xFF5A6B7F)), @@ -624,10 +614,7 @@ class _CreatePostScreenState extends State ), focusedBorder: OutlineInputBorder( borderRadius: BorderRadius.circular(12), - borderSide: const BorderSide( - color: AppColors.primary, - width: 2, - ), + borderSide: const BorderSide(color: AppColors.primary, width: 2), ), contentPadding: const EdgeInsets.all(16), ), @@ -646,10 +633,7 @@ class _CreatePostScreenState extends State child: DropdownButton( value: _language, dropdownColor: AppColors.backgroundSecondary, - style: const TextStyle( - color: AppColors.textPrimary, - fontSize: 16, - ), + style: const TextStyle(color: AppColors.textPrimary, fontSize: 16), icon: const Icon( Icons.arrow_drop_down, color: AppColors.textSecondary, @@ -686,10 +670,7 @@ class _CreatePostScreenState extends State children: [ const Text( 'NSFW', - style: TextStyle( - color: AppColors.textPrimary, - fontSize: 16, - ), + style: TextStyle(color: AppColors.textPrimary, fontSize: 16), ), Transform.scale( scale: 0.8, diff --git a/lib/screens/home/main_shell_screen.dart b/lib/screens/home/main_shell_screen.dart index 6c5834d..a0d5a46 100644 --- a/lib/screens/home/main_shell_screen.dart +++ b/lib/screens/home/main_shell_screen.dart @@ -76,106 +76,125 @@ class _MainShellScreenState extends State { const ProfileScreen(), ], ); + // Guard the magic constant: _createTabIndex couples this children list, + // the back-guard in _wrapWithBackGuard, and the "plus" nav item indices. + assert( + body.children[_createTabIndex] is CreatePostScreen, + '_createTabIndex must point at CreatePostScreen in the IndexedStack', + ); // Tablet layout: NavigationRail on the left if (isTablet) { - return _wrapWithBackGuard(Scaffold( - body: Row( - children: [ - // Wrap NavigationRail in a colored container that extends to - // status bar, preventing content from bleeding behind it - Container( - color: const Color(0xFF0B0F14), - child: SafeArea( - right: false, - bottom: false, - child: NavigationRail( - selectedIndex: _selectedIndex, - onDestinationSelected: _onItemTapped, - backgroundColor: const Color(0xFF0B0F14), - indicatorColor: AppColors.primary.withValues(alpha: 0.2), - labelType: NavigationRailLabelType.all, - destinations: [ - NavigationRailDestination( - icon: BlueSkyIcon.homeSimple( - color: const Color(0xFFB6C2D2).withValues(alpha: 0.6), - ), - selectedIcon: - BlueSkyIcon.homeSimple(color: AppColors.primary), - label: const Text('Home'), - ), - NavigationRailDestination( - icon: Icon( - Icons.workspaces_outlined, - color: const Color(0xFFB6C2D2).withValues(alpha: 0.6), - ), - selectedIcon: - const Icon(Icons.workspaces, color: AppColors.primary), - label: const Text('Communities'), - ), - NavigationRailDestination( - icon: BlueSkyIcon.plus( - color: const Color(0xFFB6C2D2).withValues(alpha: 0.6), + return _wrapWithBackGuard( + Scaffold( + body: Row( + children: [ + // Wrap NavigationRail in a colored container that extends to + // status bar, preventing content from bleeding behind it + Container( + color: const Color(0xFF0B0F14), + child: SafeArea( + right: false, + bottom: false, + child: NavigationRail( + selectedIndex: _selectedIndex, + onDestinationSelected: _onItemTapped, + backgroundColor: const Color(0xFF0B0F14), + indicatorColor: AppColors.primary.withValues(alpha: 0.2), + labelType: NavigationRailLabelType.all, + destinations: [ + NavigationRailDestination( + icon: BlueSkyIcon.homeSimple( + color: const Color(0xFFB6C2D2).withValues(alpha: 0.6), + ), + selectedIcon: BlueSkyIcon.homeSimple( + color: AppColors.primary, + ), + label: const Text('Home'), + ), + NavigationRailDestination( + icon: Icon( + Icons.workspaces_outlined, + color: const Color(0xFFB6C2D2).withValues(alpha: 0.6), + ), + selectedIcon: const Icon( + Icons.workspaces, + color: AppColors.primary, + ), + label: const Text('Communities'), + ), + NavigationRailDestination( + icon: BlueSkyIcon.plus( + color: const Color(0xFFB6C2D2).withValues(alpha: 0.6), + ), + selectedIcon: BlueSkyIcon.plus( + color: AppColors.primary, + ), + label: const Text('Create'), + ), + NavigationRailDestination( + icon: BlueSkyIcon.bellOutline( + color: const Color(0xFFB6C2D2).withValues(alpha: 0.6), + ), + selectedIcon: BlueSkyIcon.bellFilled( + color: AppColors.primary, + ), + label: const Text('Notifications'), + ), + NavigationRailDestination( + icon: BlueSkyIcon.personSimple( + color: const Color(0xFFB6C2D2).withValues(alpha: 0.6), + ), + selectedIcon: BlueSkyIcon.personSimple( + color: AppColors.primary, + ), + label: const Text('Me'), + ), + ], ), - selectedIcon: BlueSkyIcon.plus(color: AppColors.primary), - label: const Text('Create'), - ), - NavigationRailDestination( - icon: BlueSkyIcon.bellOutline( - color: const Color(0xFFB6C2D2).withValues(alpha: 0.6), - ), - selectedIcon: - BlueSkyIcon.bellFilled(color: AppColors.primary), - label: const Text('Notifications'), - ), - NavigationRailDestination( - icon: BlueSkyIcon.personSimple( - color: const Color(0xFFB6C2D2).withValues(alpha: 0.6), - ), - selectedIcon: - BlueSkyIcon.personSimple(color: AppColors.primary), - label: const Text('Me'), - ), - ], ), ), - ), - const VerticalDivider( - width: 1, - thickness: 1, - color: Color(0xFF1A2433), - ), - Expanded(child: body), - ], + const VerticalDivider( + width: 1, + thickness: 1, + color: Color(0xFF1A2433), + ), + Expanded(child: body), + ], + ), ), - )); + ); } // Phone layout: Bottom navigation bar - return _wrapWithBackGuard(Scaffold( - body: body, - bottomNavigationBar: Container( - decoration: const BoxDecoration( - color: Color(0xFF0B0F14), - border: Border(top: BorderSide(color: Color(0xFF0B0F14), width: 0.5)), - ), - child: SafeArea( - child: SizedBox( - height: 48, - child: Row( - mainAxisAlignment: MainAxisAlignment.spaceAround, - children: [ - _buildNavItem(0, 'home', 'Home'), - _buildNavItem(1, 'communities', 'Communities'), - _buildNavItem(2, 'plus', 'Create'), - _buildNavItem(3, 'bell', 'Notifications'), - _buildNavItem(4, 'person', 'Me'), - ], + return _wrapWithBackGuard( + Scaffold( + body: body, + bottomNavigationBar: Container( + decoration: const BoxDecoration( + color: Color(0xFF0B0F14), + border: Border( + top: BorderSide(color: Color(0xFF0B0F14), width: 0.5), + ), + ), + child: SafeArea( + child: SizedBox( + height: 48, + child: Row( + mainAxisAlignment: MainAxisAlignment.spaceAround, + children: [ + _buildNavItem(0, 'home', 'Home'), + _buildNavItem(1, 'communities', 'Communities'), + _buildNavItem(2, 'plus', 'Create'), + _buildNavItem(3, 'bell', 'Notifications'), + _buildNavItem(4, 'person', 'Me'), + ], + ), ), ), ), ), - )); + ); } /// Shell-level back handling. @@ -186,12 +205,14 @@ class _MainShellScreenState extends State { /// backgrounded mid-compose. Everywhere else back behaves normally /// (backgrounds the app / pops the route). Widget _wrapWithBackGuard(Widget child) { - final protectDraft = - _selectedIndex == _createTabIndex && _composeHasDraft; + final protectDraft = _selectedIndex == _createTabIndex && _composeHasDraft; return PopScope( canPop: !protectDraft, onPopInvokedWithResult: (didPop, result) { - if (!didPop) { + // Re-check the draft-protection condition instead of inferring it + // from !didPop alone: a pop blocked by any other PopScope in the + // subtree must not yank the user to the Feed tab. + if (!didPop && _selectedIndex == _createTabIndex && _composeHasDraft) { _onNavigateToFeed(); } }, diff --git a/lib/screens/home/profile_screen.dart b/lib/screens/home/profile_screen.dart index 1fd474e..7b3e4b9 100644 --- a/lib/screens/home/profile_screen.dart +++ b/lib/screens/home/profile_screen.dart @@ -86,22 +86,26 @@ class _ProfileScreenState extends State { // Check mounted after async gap (CLAUDE.md requirement) if (!mounted) return; - // Seed block state from the profile's viewer data so block/unblock - // menus reflect the server-side block after an app restart (the - // seed never clobbers fresher in-session optimistic state). Only - // seed when a viewer object is present: an unauthenticated response - // omits it entirely, and that absence must not be read as "false". - final profile = profileProvider.profile; - final viewer = profile?.viewer; - if (profile != null && viewer != null && profile.did != authProvider.did) { - context.read().setInitialUserBlockState( - userDid: profile.did, - isBlocked: viewer.blocked, - ); - } - - // Only load posts if profile loaded successfully (no error) + // Only seed block state / load posts if the profile loaded successfully + // (no error) — a failed load can leave a stale cached profile whose + // viewer state must not be seeded. if (profileProvider.profileError == null) { + // Seed block state from the profile's viewer data so block/unblock + // menus reflect the server-side block after an app restart (the + // seed never clobbers fresher in-session optimistic state). Only + // seed when a viewer object is present: an unauthenticated response + // omits it entirely, and that absence must not be read as "false". + final profile = profileProvider.profile; + final viewer = profile?.viewer; + if (profile != null && + viewer != null && + profile.did != authProvider.did) { + context.read().setInitialUserBlockState( + userDid: profile.did, + isBlocked: viewer.blocked, + ); + } + await profileProvider.loadPosts(refresh: true); } } diff --git a/lib/widgets/post_card.dart b/lib/widgets/post_card.dart index c83d880..cffdd92 100644 --- a/lib/widgets/post_card.dart +++ b/lib/widgets/post_card.dart @@ -649,6 +649,9 @@ class _EmbedCardState extends State<_EmbedCard> { if (_isStreamableVideo) { return Semantics( button: true, + // Reflect the disabled tap handler while the video is loading so + // assistive tech doesn't advertise a dead button + enabled: !_isLoadingVideo, label: 'Play video', child: GestureDetector( onTap: _isLoadingVideo ? null : () => _showVideoPlayer(context), diff --git a/test/screens/create_post_screen_test.dart b/test/screens/create_post_screen_test.dart index 9acf686..097638c 100644 --- a/test/screens/create_post_screen_test.dart +++ b/test/screens/create_post_screen_test.dart @@ -54,13 +54,19 @@ void main() { fakeAuthProvider = FakeAuthProvider(); }); - Widget createTestWidget({VoidCallback? onNavigateToFeed}) { + Widget createTestWidget({ + VoidCallback? onNavigateToFeed, + ValueChanged? onDirtyChanged, + }) { return MultiProvider( providers: [ ChangeNotifierProvider.value(value: fakeAuthProvider), ], child: MaterialApp( - home: CreatePostScreen(onNavigateToFeed: onNavigateToFeed), + home: CreatePostScreen( + onNavigateToFeed: onNavigateToFeed, + onDirtyChanged: onDirtyChanged, + ), ), ); } @@ -219,6 +225,64 @@ void main() { expect(callbackCalled, true); }); + testWidgets('URL field does not auto-capitalize input', (tester) async { + await tester.pumpWidget(createTestWidget()); + await tester.pumpAndSettle(); + + // "Https://..." from auto-capitalization breaks backend unfurling and + // external link handling + final urlField = tester.widget( + find.widgetWithText(TextField, 'URL'), + ); + expect(urlField.textCapitalization, TextCapitalization.none); + }); + + testWidgets('reports dirty on first character and clean when cleared', ( + tester, + ) async { + final dirtyLog = []; + + await tester.pumpWidget( + createTestWidget(onDirtyChanged: dirtyLog.add), + ); + await tester.pumpAndSettle(); + + // Untouched form never reports dirty + expect(dirtyLog.contains(true), isFalse); + + // First character marks the composer dirty + await tester.enterText(find.widgetWithText(TextField, 'Title'), 'a'); + await tester.pumpAndSettle(); + expect(dirtyLog.last, isTrue); + + // Clearing the only text marks it clean again + await tester.enterText(find.widgetWithText(TextField, 'Title'), ''); + await tester.pumpAndSettle(); + expect(dirtyLog.last, isFalse); + }); + + testWidgets('whitespace-only input never reports dirty', (tester) async { + final dirtyLog = []; + + await tester.pumpWidget( + createTestWidget(onDirtyChanged: dirtyLog.add), + ); + await tester.pumpAndSettle(); + + // Dirty is trimmed-text-only: whitespace is not a draft worth + // protecting (and community/NSFW/language selections are deliberately + // excluded from dirty tracking entirely) + await tester.enterText(find.widgetWithText(TextField, 'Title'), ' '); + await tester.pumpAndSettle(); + await tester.enterText( + find.widgetWithText(TextField, 'What are your thoughts?'), + '\n\n ', + ); + await tester.pumpAndSettle(); + + expect(dirtyLog.contains(true), isFalse); + }); + testWidgets('should have character limit on title field', (tester) async { await tester.pumpWidget(createTestWidget()); await tester.pumpAndSettle(); diff --git a/test/screens/main_shell_screen_test.dart b/test/screens/main_shell_screen_test.dart new file mode 100644 index 0000000..2932919 --- /dev/null +++ b/test/screens/main_shell_screen_test.dart @@ -0,0 +1,260 @@ +import 'dart:async'; + +import 'package:coves_flutter/models/feed_state.dart'; +import 'package:coves_flutter/providers/auth_provider.dart'; +import 'package:coves_flutter/providers/block_provider.dart'; +import 'package:coves_flutter/providers/community_subscription_provider.dart'; +import 'package:coves_flutter/providers/multi_feed_provider.dart'; +import 'package:coves_flutter/providers/user_profile_provider.dart'; +import 'package:coves_flutter/providers/vote_provider.dart'; +import 'package:coves_flutter/screens/home/main_shell_screen.dart'; +import 'package:coves_flutter/services/coves_api_service.dart'; +import 'package:coves_flutter/services/vote_service.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:provider/provider.dart'; + +// Fake AuthProvider for testing (see test/widgets/feed_screen_test.dart). +// Unauthenticated: FeedScreen shows the Discover empty state and +// ProfileScreen shows the sign-in prompt, so no network is touched. +class FakeAuthProvider extends AuthProvider { + @override + bool get isAuthenticated => false; + + @override + bool get isLoading => false; +} + +// Fake VoteProvider for testing +class FakeVoteProvider extends VoteProvider { + FakeVoteProvider() + : super( + voteService: VoteService( + sessionGetter: () async => null, + didGetter: () => null, + ), + authProvider: FakeAuthProvider(), + ); + + @override + bool isLiked(String postUri) => false; +} + +// Fake CommunitySubscriptionProvider that never touches the network +class FakeCommunitySubscriptionProvider extends CommunitySubscriptionProvider { + FakeCommunitySubscriptionProvider({required super.authProvider}); + + @override + Future loadSubscribedCommunities() async { + // No-op for testing - avoids network calls and pending timers + } +} + +// Fake MultiFeedProvider that never touches the network +class FakeMultiFeedProvider extends MultiFeedProvider { + FakeMultiFeedProvider() : super(FakeAuthProvider()); + + @override + FeedState getState(FeedType type) => FeedState.initial(); + + @override + Future loadFeed(FeedType type, {bool refresh = false}) async {} + + @override + Future retry(FeedType type) async {} + + @override + Future loadMore(FeedType type) async {} + + @override + void saveScrollPosition(FeedType type, double position) {} +} + +void main() { + group('MainShellScreen system back matrix', () { + late FakeAuthProvider fakeAuthProvider; + late FakeMultiFeedProvider fakeFeedProvider; + late FakeVoteProvider fakeVoteProvider; + late CommunitySubscriptionProvider subscriptionProvider; + late BlockProvider blockProvider; + late UserProfileProvider profileProvider; + late GlobalKey navigatorKey; + + setUp(() { + fakeAuthProvider = FakeAuthProvider(); + fakeFeedProvider = FakeMultiFeedProvider(); + fakeVoteProvider = FakeVoteProvider(); + subscriptionProvider = FakeCommunitySubscriptionProvider( + authProvider: fakeAuthProvider, + ); + blockProvider = BlockProvider( + apiService: CovesApiService(), + authProvider: fakeAuthProvider, + ); + profileProvider = UserProfileProvider(fakeAuthProvider); + navigatorKey = GlobalKey(); + }); + + tearDown(() { + subscriptionProvider.dispose(); + blockProvider.dispose(); + profileProvider.dispose(); + fakeVoteProvider.dispose(); + fakeFeedProvider.dispose(); + fakeAuthProvider.dispose(); + }); + + /// Pumps a base route and pushes MainShellScreen on top of it, so a + /// successful system back visibly pops (back to the base route) while a + /// blocked back leaves the shell in place. + Future pumpShell(WidgetTester tester) async { + // Force the phone layout (bottom navigation bar): shortestSide < 600 + tester.view.physicalSize = const Size(540, 960); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.resetPhysicalSize); + addTearDown(tester.view.resetDevicePixelRatio); + + await tester.pumpWidget( + MultiProvider( + providers: [ + ChangeNotifierProvider.value(value: fakeAuthProvider), + ChangeNotifierProvider.value( + value: fakeFeedProvider, + ), + ChangeNotifierProvider.value(value: fakeVoteProvider), + ChangeNotifierProvider.value( + value: subscriptionProvider, + ), + ChangeNotifierProvider.value(value: blockProvider), + ChangeNotifierProvider.value( + value: profileProvider, + ), + ], + child: MaterialApp( + navigatorKey: navigatorKey, + home: const Scaffold(body: Text('base route')), + ), + ), + ); + + unawaited( + navigatorKey.currentState!.push( + MaterialPageRoute(builder: (_) => const MainShellScreen()), + ), + ); + await tester.pump(); + // Let post-frame loads (communities/profile) resolve to error/empty + // states against the test HTTP client + await tester.pump(const Duration(seconds: 1)); + } + + /// Taps a bottom-nav item by its Semantics label ('Home', 'Create', ...) + Future tapNavItem(WidgetTester tester, String label) async { + await tester.tap( + find.byWidgetPredicate( + (widget) => widget is Semantics && widget.properties.label == label, + ), + ); + await tester.pump(); + } + + /// Simulates the Android system back button (same path as a real + /// hardware/gesture back: WidgetsApp -> root navigator maybePop). + Future systemBack(WidgetTester tester) async { + await tester.binding.handlePopRoute(); + await tester.pump(); + // Let a potential route pop transition run to completion (the zoom + // page transition takes 500ms) + await tester.pump(const Duration(milliseconds: 600)); + await tester.pump(); + await tester.pump(); + } + + /// The shell's tab IndexedStack (the first one under MainShellScreen - + /// DropdownButton in the composer owns a nested IndexedStack of its own) + int stackIndex(WidgetTester tester) { + final stack = tester.widget( + find + .descendant( + of: find.byType(MainShellScreen), + matching: find.byType(IndexedStack), + ) + .first, + ); + return stack.index!; + } + + Future dirtyComposer(WidgetTester tester) async { + await tester.enterText( + find.widgetWithText(TextField, 'Title'), + 'Unsaved draft', + ); + await tester.pump(); + } + + testWidgets('Create tab + dirty composer: back is intercepted and ' + 'lands on Feed tab', (tester) async { + await pumpShell(tester); + + await tapNavItem(tester, 'Create'); + expect(stackIndex(tester), 2); + await dirtyComposer(tester); + + await systemBack(tester); + + // Not popped - shell still on screen, draft alive in the IndexedStack + expect(find.byType(MainShellScreen), findsOneWidget); + expect(find.text('base route'), findsNothing); + // ...and the user landed on the Feed tab + expect(stackIndex(tester), 0); + }); + + testWidgets('Create tab + clean composer: back pops normally', ( + tester, + ) async { + await pumpShell(tester); + + await tapNavItem(tester, 'Create'); + expect(stackIndex(tester), 2); + + await systemBack(tester); + + expect(find.byType(MainShellScreen), findsNothing); + expect(find.text('base route'), findsOneWidget); + }); + + testWidgets('other tab + dirty composer: back pops normally ' + '(draft protection must not fire off the Create tab)', (tester) async { + await pumpShell(tester); + + // Dirty the composer, then switch away from the Create tab + await tapNavItem(tester, 'Create'); + await dirtyComposer(tester); + await tapNavItem(tester, 'Home'); + expect(stackIndex(tester), 0); + + await systemBack(tester); + + expect(find.byType(MainShellScreen), findsNothing); + expect(find.text('base route'), findsOneWidget); + }); + + testWidgets('after an intercepted back, a second back (now on Feed, ' + 'clean) pops', (tester) async { + await pumpShell(tester); + + await tapNavItem(tester, 'Create'); + await dirtyComposer(tester); + + // First back: intercepted, lands on Feed + await systemBack(tester); + expect(find.byType(MainShellScreen), findsOneWidget); + expect(stackIndex(tester), 0); + + // Second back: nothing to protect anymore, pops + await systemBack(tester); + expect(find.byType(MainShellScreen), findsNothing); + expect(find.text('base route'), findsOneWidget); + }); + }); +} diff --git a/test/screens/reply_screen_test.dart b/test/screens/reply_screen_test.dart new file mode 100644 index 0000000..cd6bfa7 --- /dev/null +++ b/test/screens/reply_screen_test.dart @@ -0,0 +1,265 @@ +import 'dart:async'; + +import 'package:coves_flutter/models/comment.dart'; +import 'package:coves_flutter/models/post.dart'; +import 'package:coves_flutter/providers/auth_provider.dart'; +import 'package:coves_flutter/providers/block_provider.dart'; +import 'package:coves_flutter/providers/comments_provider.dart'; +import 'package:coves_flutter/providers/vote_provider.dart'; +import 'package:coves_flutter/screens/compose/reply_screen.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; +import 'package:provider/provider.dart'; + +// Shared generated mockito mocks (real provider types) so provider lookups +// inside CommentThread/CommentCard resolve correctly. +import '../test_helpers/test_mocks.dart'; + +void main() { + const testPostUri = 'at://did:plc:test/social.coves.post.record/123'; + const testPostCid = 'test-post-cid'; + + late MockAuthProvider mockAuthProvider; + late MockCovesApiService mockApiService; + late MockVoteProvider mockVoteProvider; + late BlockProvider blockProvider; + late CommentsProvider commentsProvider; + late GlobalKey navigatorKey; + + setUp(() { + mockAuthProvider = MockAuthProvider(); + mockApiService = MockCovesApiService(); + mockVoteProvider = MockVoteProvider(); + blockProvider = BlockProvider( + apiService: mockApiService, + authProvider: mockAuthProvider, + ); + navigatorKey = GlobalKey(); + + // Signed-out rendering keeps CommentCard simple (no action menus) + when(mockAuthProvider.isAuthenticated).thenReturn(false); + when(mockVoteProvider.isLiked(any)).thenReturn(false); + when( + mockVoteProvider.getAdjustedScore(any, any), + ).thenAnswer((invocation) => invocation.positionalArguments[1] as int); + + // Real CommentsProvider over mocks: drafts are pure local state, so no + // API stubbing is needed (ReplyScreen never triggers loadComments) + commentsProvider = CommentsProvider( + mockAuthProvider, + postUri: testPostUri, + postCid: testPostCid, + apiService: mockApiService, + voteProvider: mockVoteProvider, + ); + }); + + tearDown(() { + commentsProvider.dispose(); + blockProvider.dispose(); + }); + + ThreadViewComment createComment(String uri) { + return ThreadViewComment( + comment: CommentView( + uri: uri, + cid: 'cid-$uri', + record: CommentRecord(content: 'Parent comment for $uri'), + createdAt: DateTime.parse('2025-01-01T12:00:00Z'), + indexedAt: DateTime.parse('2025-01-01T12:00:00Z'), + author: AuthorView( + did: 'did:plc:author', + handle: 'test.user', + displayName: 'Test User', + ), + post: CommentRef(uri: testPostUri, cid: testPostCid), + stats: const CommentStats(score: 10, upvotes: 12, downvotes: 2), + ), + ); + } + + /// Pumps a base route; ReplyScreen is pushed on top so pops are visible + Future pumpBase(WidgetTester tester) async { + // Without a mock handler, HapticFeedback.lightImpact() (awaited inside + // _handleSubmit) never completes in widget tests and submit would hang + tester.binding.defaultBinaryMessenger.setMockMethodCallHandler( + SystemChannels.platform, + (methodCall) async => null, + ); + addTearDown( + () => tester.binding.defaultBinaryMessenger.setMockMethodCallHandler( + SystemChannels.platform, + null, + ), + ); + + await tester.pumpWidget( + MultiProvider( + providers: [ + ChangeNotifierProvider.value(value: mockAuthProvider), + ChangeNotifierProvider.value(value: mockVoteProvider), + ChangeNotifierProvider.value(value: blockProvider), + ], + child: MaterialApp( + navigatorKey: navigatorKey, + home: const Scaffold(body: Text('base route')), + ), + ), + ); + } + + Future pushReply( + WidgetTester tester, { + required ThreadViewComment comment, + Future Function(String content, List facets)? onSubmit, + }) async { + unawaited( + navigatorKey.currentState!.push( + MaterialPageRoute( + builder: + (_) => ReplyScreen( + comment: comment, + commentsProvider: commentsProvider, + onSubmit: onSubmit ?? (content, facets) async {}, + ), + ), + ), + ); + // Settles the push animation, draft restore, and the delayed autofocus + await tester.pumpAndSettle(); + } + + /// The Send pill's tap target (nearest GestureDetector around 'Send') + GestureDetector sendButton(WidgetTester tester) { + return tester.widget( + find + .ancestor( + of: find.text('Send'), + matching: find.byType(GestureDetector), + ) + .first, + ); + } + + group('ReplyScreen draft round-trip', () { + testWidgets('type then Cancel saves the draft for the parent URI', ( + tester, + ) async { + final comment = createComment('at://did:plc:author/comment/1'); + await pumpBase(tester); + await pushReply(tester, comment: comment); + + await tester.enterText(find.byType(TextField), 'My draft reply'); + await tester.pump(); + + await tester.tap(find.text('Cancel')); + await tester.pumpAndSettle(); + + expect(find.text('base route'), findsOneWidget); + expect( + commentsProvider.getDraft(parentUri: comment.comment.uri), + 'My draft reply', + ); + }); + + testWidgets('re-pushing the same parentUri restores the draft and ' + 'enables Send', (tester) async { + final comment = createComment('at://did:plc:author/comment/1'); + await pumpBase(tester); + + await pushReply(tester, comment: comment); + await tester.enterText(find.byType(TextField), 'Restored draft'); + await tester.pump(); + await tester.tap(find.text('Cancel')); + await tester.pumpAndSettle(); + + await pushReply(tester, comment: comment); + + final textField = tester.widget(find.byType(TextField)); + expect(textField.controller!.text, 'Restored draft'); + expect(sendButton(tester).onTap, isNotNull); + }); + + testWidgets('successful submit clears the draft (and the pop must not ' + 'resurrect it)', (tester) async { + final comment = createComment('at://did:plc:author/comment/1'); + String? submitted; + await pumpBase(tester); + await pushReply( + tester, + comment: comment, + onSubmit: (content, facets) async { + submitted = content; + }, + ); + + await tester.enterText(find.byType(TextField), 'Ship it'); + await tester.pump(); + await tester.tap(find.text('Send')); + await tester.pumpAndSettle(); + + expect(submitted, 'Ship it'); + // Screen popped after success + expect(find.text('base route'), findsOneWidget); + // Draft cleared - the pop-time save must not have run (the controller + // still held the submitted text when the route popped) + expect(commentsProvider.getDraft(parentUri: comment.comment.uri), ''); + }); + + testWidgets('drafts for distinct parentUris are independent', ( + tester, + ) async { + final comment1 = createComment('at://did:plc:author/comment/1'); + final comment2 = createComment('at://did:plc:author/comment/2'); + await pumpBase(tester); + + await pushReply(tester, comment: comment1); + await tester.enterText(find.byType(TextField), 'draft one'); + await tester.pump(); + await tester.tap(find.text('Cancel')); + await tester.pumpAndSettle(); + + await pushReply(tester, comment: comment2); + // The other comment's draft must not leak into this composer + final textField = tester.widget(find.byType(TextField)); + expect(textField.controller!.text, isEmpty); + + await tester.enterText(find.byType(TextField), 'draft two'); + await tester.pump(); + await tester.tap(find.text('Cancel')); + await tester.pumpAndSettle(); + + expect( + commentsProvider.getDraft(parentUri: comment1.comment.uri), + 'draft one', + ); + expect( + commentsProvider.getDraft(parentUri: comment2.comment.uri), + 'draft two', + ); + }); + + testWidgets('system back saves the draft (not just the Cancel button)', ( + tester, + ) async { + final comment = createComment('at://did:plc:author/comment/1'); + await pumpBase(tester); + await pushReply(tester, comment: comment); + + await tester.enterText(find.byType(TextField), 'Saved by system back'); + await tester.pump(); + + // Same path as a hardware/gesture back: root navigator maybePop + await tester.binding.handlePopRoute(); + await tester.pumpAndSettle(); + + expect(find.text('base route'), findsOneWidget); + expect( + commentsProvider.getDraft(parentUri: comment.comment.uri), + 'Saved by system back', + ); + }); + }); +} -- 2.51.2