Merge pull request #79 from flutter-news-app-full-source-code/fix-misc-ui-errors

fulleni · web-flow · commit a47702e05345 · 2025-08-15T18:22:39.000+01:00
Fix misc UI errors
diff --git a/lib/entity_details/bloc/entity_details_bloc.dart b/lib/entity_details/bloc/entity_details_bloc.dart
@@ -212,7 +212,10 @@ class EntityDetailsBloc extends Bloc<EntityDetailsEvent, EntityDetailsState> {
         feedItems: headlineResponse.items,
         user: currentUser,
         adConfig: remoteConfig.adConfig,
-        currentFeedItemCount: state.feedItems.length,
+        // Calculate the count of actual content items (headlines) already in the
+        // feed. This is crucial for the FeedDecoratorService to correctly apply
+        // ad placement rules across paginated loads.
+        processedContentItemCount: state.feedItems.whereType<Headline>().length,
       );
 
       emit(
diff --git a/lib/headlines-feed/bloc/headlines_feed_bloc.dart b/lib/headlines-feed/bloc/headlines_feed_bloc.dart
@@ -113,7 +113,10 @@ class HeadlinesFeedBloc extends Bloc<HeadlinesFeedEvent, HeadlinesFeedState> {
         feedItems: headlineResponse.items,
         user: currentUser,
         adConfig: remoteConfig.adConfig,
-        currentFeedItemCount: state.feedItems.length,
+        // Calculate the count of actual content items (headlines) already in the
+        // feed. This is crucial for the FeedDecoratorService to correctly apply
+        // ad placement rules across paginated loads.
+        processedContentItemCount: state.feedItems.whereType<Headline>().length,
       );
 
       emit(
diff --git a/lib/headlines-feed/view/headlines_feed_page.dart b/lib/headlines-feed/view/headlines_feed_page.dart
@@ -153,6 +153,20 @@ class _HeadlinesFeedPageState extends State<HeadlinesFeedPage> {
         ),
         body: BlocBuilder<HeadlinesFeedBloc, HeadlinesFeedState>(
           builder: (context, state) {
+            // Access the AppBloc to check for remoteConfig availability.
+            final appBlocState = context.watch<AppBloc>().state;
+
+            // If remoteConfig is not yet loaded, show a loading indicator.
+            // This handles the brief period after authentication but before
+            // the remote config is fetched, preventing null access errors.
+            if (appBlocState.remoteConfig == null) {
+              return LoadingStateWidget(
+                icon: Icons.settings_applications_outlined,
+                headline: l10n.headlinesFeedLoadingHeadline,
+                subheadline: l10n.pleaseWait,
+              );
+            }
+
             if (state.status == HeadlinesFeedStatus.initial ||
                 (state.status == HeadlinesFeedStatus.loading &&
                     state.feedItems.isEmpty)) {
@@ -175,14 +189,24 @@ class _HeadlinesFeedPageState extends State<HeadlinesFeedPage> {
 
             if (state.status == HeadlinesFeedStatus.success &&
                 state.feedItems.isEmpty) {
-              return FailureStateWidget(
-                exception: const UnknownException(
-                  'No headlines found matching your criteria.',
-                ),
-                onRetry: () => context.read<HeadlinesFeedBloc>().add(
-                  HeadlinesFeedFiltersCleared(),
+              return Center(
+                child: Column(
+                  mainAxisAlignment: MainAxisAlignment.center,
+                  children: [
+                    InitialStateWidget(
+                      icon: Icons.search_off,
+                      headline: l10n.headlinesFeedEmptyFilteredHeadline,
+                      subheadline: l10n.headlinesFeedEmptyFilteredSubheadline,
+                    ),
+                    const SizedBox(height: AppSpacing.lg),
+                    ElevatedButton(
+                      onPressed: () => context.read<HeadlinesFeedBloc>().add(
+                        HeadlinesFeedFiltersCleared(),
+                      ),
+                      child: Text(l10n.headlinesFeedClearFiltersButton),
+                    ),
+                  ],
                 ),
-                retryButtonText: l10n.headlinesFeedClearFiltersButton,
               );
             }
 
diff --git a/lib/headlines-search/bloc/headlines_search_bloc.dart b/lib/headlines-search/bloc/headlines_search_bloc.dart
@@ -112,7 +112,11 @@ class HeadlinesSearchBloc
                 feedItems: headlines,
                 user: currentUser,
                 adConfig: appConfig.adConfig,
-                currentFeedItemCount: successState.items.length,
+                // Calculate the count of actual content items (headlines) already in the
+                // feed. This is crucial for the FeedDecoratorService to correctly apply
+                // ad placement rules across paginated loads.
+                processedContentItemCount:
+                    successState.items.whereType<Headline>().length,
               );
               emit(
                 successState.copyWith(
diff --git a/lib/headlines-search/view/headlines_search_page.dart b/lib/headlines-search/view/headlines_search_page.dart
@@ -187,8 +187,7 @@ class _HeadlinesSearchViewState extends State<_HeadlinesSearchView> {
                 controller: _textController,
                 style: appBarTheme.titleTextStyle ?? textTheme.titleMedium,
                 decoration: InputDecoration(
-                  // TODO(fulleni): Create a similar localization extension for hint text.
-                  hintText: 'Search...',
+                  hintText: l10n.searchHintTextGeneric,
                   hintStyle: textTheme.bodyMedium?.copyWith(
                     color:
                         (appBarTheme.titleTextStyle?.color ??
diff --git a/lib/l10n/app_localizations.dart b/lib/l10n/app_localizations.dart
@@ -998,6 +998,12 @@ abstract class AppLocalizations {
   /// **'e.g., AI advancements, Mars rover...'**
   String get searchHintTextHeadline;
 
+  /// Generic hint text for search input fields
+  ///
+  /// In en, this message translates to:
+  /// **'Search...'**
+  String get searchHintTextGeneric;
+
   /// Hint text for searching categories
   ///
   /// In en, this message translates to:
diff --git a/lib/l10n/app_localizations_ar.dart b/lib/l10n/app_localizations_ar.dart
@@ -487,6 +487,9 @@ class AppLocalizationsAr extends AppLocalizations {
   String get searchHintTextHeadline =>
       'مثال: تطورات الذكاء الاصطناعي, مركبة المريخ...';
 
+  @override
+  String get searchHintTextGeneric => 'بحث...';
+
   @override
   String get searchHintTextCategory => 'مثال: تكنولوجيا, رياضة, مالية...';
 
diff --git a/lib/l10n/app_localizations_en.dart b/lib/l10n/app_localizations_en.dart
@@ -488,6 +488,9 @@ class AppLocalizationsEn extends AppLocalizations {
   @override
   String get searchHintTextHeadline => 'e.g., AI advancements, Mars rover...';
 
+  @override
+  String get searchHintTextGeneric => 'Search...';
+
   @override
   String get searchHintTextCategory => 'e.g., Technology, Sports, Finance...';
 
diff --git a/lib/l10n/arb/app_ar.arb b/lib/l10n/arb/app_ar.arb
@@ -618,6 +618,10 @@
     "@searchHintTextHeadline": {
         "description": "Hint text for searching headlines"
     },
+    "searchHintTextGeneric": "بحث...",
+    "@searchHintTextGeneric": {
+        "description": "Generic hint text for search input fields"
+    },
     "searchHintTextCategory": "مثال: تكنولوجيا, رياضة, مالية...",
     "@searchHintTextCategory": {
         "description": "Hint text for searching categories"
diff --git a/lib/l10n/arb/app_en.arb b/lib/l10n/arb/app_en.arb
@@ -619,6 +619,10 @@
     "@searchHintTextHeadline": {
         "description": "Hint text for searching headlines"
     },
+    "searchHintTextGeneric": "Search...",
+    "@searchHintTextGeneric": {
+        "description": "Generic hint text for search input fields"
+    },
     "searchHintTextCategory": "e.g., Technology, Sports, Finance...",
     "@searchHintTextCategory": {
         "description": "Hint text for searching categories"
diff --git a/lib/shared/services/feed_decorator_service.dart b/lib/shared/services/feed_decorator_service.dart
@@ -165,13 +165,13 @@ class FeedDecoratorService {
     required List<FeedItem> feedItems,
     required User? user,
     required AdConfig adConfig,
-    int currentFeedItemCount = 0,
+    int processedContentItemCount = 0,
   }) {
     return _injectAds(
       feedItems: feedItems,
       user: user,
       adConfig: adConfig,
-      currentFeedItemCount: currentFeedItemCount,
+      processedContentItemCount: processedContentItemCount,
     );
   }
 
@@ -343,12 +343,26 @@ class FeedDecoratorService {
     }
   }
 
-  /// Injects ads into a list of feed items based on frequency rules.
+  /// Injects ads into a list of [FeedItem]s based on frequency rules.
+  ///
+  /// This method ensures that ads are placed according to the `adPlacementInterval`
+  /// (initial buffer before the first ad) and `adFrequency` (subsequent ad spacing).
+  /// It correctly accounts for content items only, ignoring previously injected ads
+  /// when calculating placement.
+  ///
+  /// [feedItems]: The list of feed items (headlines, decorators) to inject ads into.
+  /// [user]: The current authenticated user, used to determine ad configuration.
+  /// [adConfig]: The remote configuration for ad display rules.
+  /// [processedContentItemCount]: The count of *content items* (non-ad, non-decorator)
+  ///   that have already been processed in previous feed loads/pages. This is
+  ///   crucial for maintaining correct ad placement across pagination.
+  ///
+  /// Returns a new list of [FeedItem] objects, interspersed with ads.
   List<FeedItem> _injectAds({
     required List<FeedItem> feedItems,
     required User? user,
     required AdConfig adConfig,
-    int currentFeedItemCount = 0,
+    int processedContentItemCount = 0,
   }) {
     final userRole = user?.appRole ?? AppUserRole.guestUser;
 
@@ -374,21 +388,31 @@ class FeedDecoratorService {
     }
 
     final result = <FeedItem>[];
-    var headlinesInBatch = 0;
+    // This counter tracks the absolute number of *content items* (headlines,
+    // topics, sources, countries) processed so far, including those from
+    // previous pages. This is key for accurate ad placement.
+    var currentContentItemCount = processedContentItemCount;
 
     for (final item in feedItems) {
       result.add(item);
-      headlinesInBatch++;
 
-      // Calculate the total number of items processed so far, including
-      // those from previous pages.
-      final totalItemsSoFar = currentFeedItemCount + result.length;
+      // Only increment the content item counter if the current item is
+      // a primary content type (not an ad or a decorator).
+      // This ensures ad placement is based purely on content density.
+      if (item is Headline ||
+          item is Topic ||
+          item is Source ||
+          item is Country) {
+        currentContentItemCount++;
+      }
 
-      // Check if an ad should be injected.
-      // The total number of items must be past the initial placement interval,
-      // AND the number of content items in this batch must meet the frequency.
-      if (totalItemsSoFar >= adPlacementInterval &&
-          headlinesInBatch % adFrequency == 0) {
+      // Check if an ad should be injected at the current position.
+      // An ad is injected if:
+      // 1. We have passed the initial placement interval.
+      // 2. The number of content items *after* the initial interval is a
+      //    multiple of the ad frequency.
+      if (currentContentItemCount >= adPlacementInterval &&
+          (currentContentItemCount - adPlacementInterval) % adFrequency == 0) {
         final adToInject = _getAdToInject();
         if (adToInject != null) {
           result.add(adToInject);
