Merge pull request #85 from flutter-news-app-full-source-code/refactor/ad-config-ui-role-based

Refactor/ad config UI role based

Author: fulleni

lib/app_configuration/widgets/ad_config_form.dart

@@ -3,10 +3,9 @@ import 'package:flutter/material.dart';
 import 'package:flutter_news_app_web_dashboard_full_source_code/l10n/l10n.dart';
 
 /// {@template ad_config_form}
-/// A form widget for configuring ad settings based on user role.
+/// A form widget for configuring global ad settings.
 ///
-/// This widget uses a [TabBar] to allow selection of an [AppUserRole]
-/// and then conditionally renders the relevant input fields for that role.
+/// This widget primarily controls the global enable/disable switch for ads.
 /// {@endtemplate}
 class AdConfigForm extends StatefulWidget {
   /// {@macro ad_config_form}
@@ -26,96 +25,7 @@ class AdConfigForm extends StatefulWidget {
   State<AdConfigForm> createState() => _AdConfigFormState();
 }
 
-class _AdConfigFormState extends State<AdConfigForm>
-    with SingleTickerProviderStateMixin {
-  late TabController _tabController;
-  late final Map<AppUserRole, TextEditingController> _adFrequencyControllers;
-  late final Map<AppUserRole, TextEditingController>
-  _adPlacementIntervalControllers;
-
-  @override
-  void initState() {
-    super.initState();
-    _tabController = TabController(
-      length: AppUserRole.values.length,
-      vsync: this,
-    );
-    _initializeControllers();
-  }
-
-  @override
-  void didUpdateWidget(covariant AdConfigForm oldWidget) {
-    super.didUpdateWidget(oldWidget);
-    if (widget.remoteConfig.adConfig != oldWidget.remoteConfig.adConfig) {
-      _updateControllers();
-    }
-  }
-
-  void _initializeControllers() {
-    final adConfig = widget.remoteConfig.adConfig;
-    _adFrequencyControllers = {
-      for (final role in AppUserRole.values)
-        role:
-            TextEditingController(
-                text: _getAdFrequency(adConfig, role).toString(),
-              )
-              ..selection = TextSelection.collapsed(
-                offset: _getAdFrequency(adConfig, role).toString().length,
-              ),
-    };
-    _adPlacementIntervalControllers = {
-      for (final role in AppUserRole.values)
-        role:
-            TextEditingController(
-                text: _getAdPlacementInterval(adConfig, role).toString(),
-              )
-              ..selection = TextSelection.collapsed(
-                offset: _getAdPlacementInterval(
-                  adConfig,
-                  role,
-                ).toString().length,
-              ),
-    };
-  }
-
-  void _updateControllers() {
-    final adConfig = widget.remoteConfig.adConfig;
-    for (final role in AppUserRole.values) {
-      final newFrequencyValue = _getAdFrequency(adConfig, role).toString();
-      if (_adFrequencyControllers[role]?.text != newFrequencyValue) {
-        _adFrequencyControllers[role]?.text = newFrequencyValue;
-        _adFrequencyControllers[role]?.selection = TextSelection.collapsed(
-          offset: newFrequencyValue.length,
-        );
-      }
-
-      final newPlacementIntervalValue = _getAdPlacementInterval(
-        adConfig,
-        role,
-      ).toString();
-      if (_adPlacementIntervalControllers[role]?.text !=
-          newPlacementIntervalValue) {
-        _adPlacementIntervalControllers[role]?.text = newPlacementIntervalValue;
-        _adPlacementIntervalControllers[role]?.selection =
-            TextSelection.collapsed(
-              offset: newPlacementIntervalValue.length,
-            );
-      }
-    }
-  }
-
-  @override
-  void dispose() {
-    _tabController.dispose();
-    for (final controller in _adFrequencyControllers.values) {
-      controller.dispose();
-    }
-    for (final controller in _adPlacementIntervalControllers.values) {
-      controller.dispose();
-    }
-    super.dispose();
-  }
-
+class _AdConfigFormState extends State<AdConfigForm> {
   @override
   Widget build(BuildContext context) {
     final adConfig = widget.remoteConfig.adConfig;
@@ -138,38 +48,4 @@ class _AdConfigFormState extends State<AdConfigForm>
       ],
     );
   }
-
-  int _getAdFrequency(AdConfig config, AppUserRole role) {
-    switch (role) {
-      case AppUserRole.guestUser:
-        return config.feedAdConfiguration.frequencyConfig.guestAdFrequency;
-      case AppUserRole.standardUser:
-        return config
-            .feedAdConfiguration
-            .frequencyConfig
-            .authenticatedAdFrequency;
-      case AppUserRole.premiumUser:
-        return config.feedAdConfiguration.frequencyConfig.premiumAdFrequency;
-    }
-  }
-
-  int _getAdPlacementInterval(AdConfig config, AppUserRole role) {
-    switch (role) {
-      case AppUserRole.guestUser:
-        return config
-            .feedAdConfiguration
-            .frequencyConfig
-            .guestAdPlacementInterval;
-      case AppUserRole.standardUser:
-        return config
-            .feedAdConfiguration
-            .frequencyConfig
-            .authenticatedAdPlacementInterval;
-      case AppUserRole.premiumUser:
-        return config
-            .feedAdConfiguration
-            .frequencyConfig
-            .premiumAdPlacementInterval;
-    }
-  }
 }

lib/app_configuration/widgets/article_ad_settings_form.dart

@@ -1,8 +1,10 @@
 import 'package:core/core.dart';
 import 'package:flutter/material.dart';
+import 'package:flutter_news_app_web_dashboard_full_source_code/l10n/app_localizations.dart';
 import 'package:flutter_news_app_web_dashboard_full_source_code/l10n/l10n.dart';
 import 'package:flutter_news_app_web_dashboard_full_source_code/shared/extensions/banner_ad_shape_l10n.dart';
 import 'package:flutter_news_app_web_dashboard_full_source_code/shared/extensions/in_article_ad_slot_type_l10n.dart';
+import 'package:flutter_news_app_web_dashboard_full_source_code/shared/extensions/app_user_role_l10n.dart';
 import 'package:ui_kit/ui_kit.dart';
 
 /// {@template article_ad_settings_form}
@@ -28,14 +30,28 @@ class ArticleAdSettingsForm extends StatefulWidget {
 
 class _ArticleAdSettingsFormState extends State<ArticleAdSettingsForm>
     with SingleTickerProviderStateMixin {
+  late TabController _tabController;
+
   @override
   void initState() {
     super.initState();
+    _tabController = TabController(
+      length: AppUserRole.values.length,
+      vsync: this,
+    );
   }
 
   @override
   void didUpdateWidget(covariant ArticleAdSettingsForm oldWidget) {
     super.didUpdateWidget(oldWidget);
+    // No specific controller updates needed here as the UI rebuilds based on
+    // the remoteConfig directly.
+  }
+
+  @override
+  void dispose() {
+    _tabController.dispose();
+    super.dispose();
   }
 
   @override
@@ -134,34 +150,129 @@ class _ArticleAdSettingsFormState extends State<ArticleAdSettingsForm>
               textAlign: TextAlign.start,
             ),
             const SizedBox(height: AppSpacing.lg),
-            ...articleAdConfig.inArticleAdSlotConfigurations.map(
-              (slotConfig) => SwitchListTile(
-                title: Text(slotConfig.slotType.l10n(context)),
-                value: slotConfig.enabled,
-                onChanged: (value) {
-                  final updatedSlots = articleAdConfig
-                      .inArticleAdSlotConfigurations
-                      .map(
-                        (e) => e.slotType == slotConfig.slotType
-                            ? e.copyWith(enabled: value)
-                            : e,
-                      )
-                      .toList();
-                  widget.onConfigChanged(
-                    widget.remoteConfig.copyWith(
-                      adConfig: adConfig.copyWith(
-                        articleAdConfiguration: articleAdConfig.copyWith(
-                          inArticleAdSlotConfigurations: updatedSlots,
-                        ),
+            Align(
+              alignment: AlignmentDirectional.centerStart,
+              child: SizedBox(
+                height: kTextTabBarHeight,
+                child: TabBar(
+                  controller: _tabController,
+                  tabAlignment: TabAlignment.start,
+                  isScrollable: true,
+                  tabs: AppUserRole.values
+                      .map((role) => Tab(text: role.l10n(context)))
+                      .toList(),
+                ),
+              ),
+            ),
+            const SizedBox(height: AppSpacing.lg),
+            SizedBox(
+              height: 250,
+              child: TabBarView(
+                controller: _tabController,
+                children: AppUserRole.values
+                    .map(
+                      (role) => _buildRoleSpecificFields(
+                        context,
+                        l10n,
+                        role,
+                        articleAdConfig,
                       ),
-                    ),
-                  );
-                },
+                    )
+                    .toList(),
               ),
             ),
           ],
         ),
       ],
     );
   }
+
+  /// Builds role-specific configuration fields for in-article ad slots.
+  ///
+  /// This widget displays checkboxes for each [InArticleAdSlotType] for a
+  /// given [AppUserRole], allowing to enable/disable specific ad slots.
+  Widget _buildRoleSpecificFields(
+    BuildContext context,
+    AppLocalizations l10n,
+    AppUserRole role,
+    ArticleAdConfiguration config,
+  ) {
+    final roleSlots = config.visibleTo[role];
+
+    return Column(
+      children: [
+        SwitchListTile(
+          title: Text(l10n.enableInArticleAdsForRoleLabel(role.l10n(context))),
+          value: roleSlots != null,
+          onChanged: (value) {
+            final newVisibleTo =
+                Map<AppUserRole, Map<InArticleAdSlotType, bool>>.from(
+                  config.visibleTo,
+                );
+            if (value) {
+              // Default values when enabling for a role
+              newVisibleTo[role] = {
+                InArticleAdSlotType.aboveArticleContinueReadingButton: true,
+                InArticleAdSlotType.belowArticleContinueReadingButton: true,
+              };
+            } else {
+              newVisibleTo.remove(role);
+            }
+
+            widget.onConfigChanged(
+              widget.remoteConfig.copyWith(
+                adConfig: widget.remoteConfig.adConfig.copyWith(
+                  articleAdConfiguration: config.copyWith(
+                    visibleTo: newVisibleTo,
+                  ),
+                ),
+              ),
+            );
+          },
+        ),
+        if (roleSlots != null)
+          Padding(
+            padding: const EdgeInsets.symmetric(
+              horizontal: AppSpacing.lg,
+              vertical: AppSpacing.sm,
+            ),
+            child: Column(
+              children: [
+                // SwitchListTile for each InArticleAdSlotType
+                for (final slotType in InArticleAdSlotType.values)
+                  CheckboxListTile(
+                    title: Text(slotType.l10n(context)),
+                    value: roleSlots[slotType] ?? false,
+                    onChanged: (value) {
+                      final newRoleSlots = Map<InArticleAdSlotType, bool>.from(
+                        roleSlots,
+                      );
+                      if (value ?? false) {
+                        newRoleSlots[slotType] = true;
+                      } else {
+                        newRoleSlots.remove(slotType);
+                      }
+
+                      final newVisibleTo =
+                          Map<AppUserRole, Map<InArticleAdSlotType, bool>>.from(
+                            config.visibleTo,
+                          )..[role] = newRoleSlots;
+
+                      widget.onConfigChanged(
+                        widget.remoteConfig.copyWith(
+                          adConfig: widget.remoteConfig.adConfig.copyWith(
+                            articleAdConfiguration: config.copyWith(
+                              visibleTo: newVisibleTo,
+                            ),
+                          ),
+                        ),
+                      );
+                    },
+                  ),
+              ],
+            ),
+          ),
+      ],
+    );
+  }
 }