Refactor/app identity pivot into a news aggregator #220
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Update core package ref from 643aa38 to 3779a8b in pubspec.lock - Update core package ref from 643aa38 to 3779a8b in pubspec.yaml
Updates the app-level events to align with the recent core package refactoring. The `AppSettingsChanged` event now carries the renamed `AppSettings` model instead of `UserAppSettings`. Additionally, the `AppUserAppSettingsRefreshed` event has been renamed to `AppAppSettingsRefreshed` to maintain consistency with the new model name, ensuring the entire event layer conforms to the updated data contracts.
Refactors the `AppState` to incorporate the renamed `AppSettings` model and the new hierarchical `RemoteConfig` structure. The state now holds an `AppSettings` object, and all related getters (`themeMode`, `fontFamily`, `flexScheme`, etc.) have been updated to access their values from the new nested `displaySettings` and `feedSettings` models. The `headlineImageStyle` getter has been renamed to `feedItemImageStyle` and its return type changed to `FeedItemImageStyle` to match the core enum rename, ensuring the app's global state is fully aligned with the updated data contracts.
…cture Updates the `AppBloc` to be fully compatible with the latest core package changes. The BLoC's constructor and state are now initialized with the renamed `AppSettings` model. Event handlers, particularly `_onAppSettingsChanged` and `_onAppPeriodicConfigFetchRequested`, have been refactored to use the new `AppSettings` model and access the new hierarchical `RemoteConfig` structure (e.g., `remoteConfig.app.maintenance.isUnderMaintenance`). Logic for handling push notifications has also been updated to work with the refactored `PushNotificationPayload` model, ensuring the app's central state orchestrator correctly manages the new data contracts.
Updates the root `App` widget and its state to align with the core package refactoring. The `App` widget's constructor now accepts the renamed `AppSettings` model. In `_AppViewState`, the deep-linking logic for push notifications has been updated to parse the new, more explicit `PushNotificationPayload` structure, correctly extracting `contentType` and `contentId`. The logic for displaying the `UpdateRequiredPage` has also been updated to access the version information from the new hierarchical `RemoteConfig` path (`remoteConfig.app.update`), ensuring the main app view correctly handles the new data models.
Updates the `AppInitializationPage` to align with the `UserAppSettings` to `AppSettings` model rename. The widget now correctly receives and provides the `DataRepository<AppSettings>` to the main `App` widget upon successful initialization. This change ensures that the application's entry point correctly handles and propagates the new settings model, maintaining type consistency throughout the startup sequence.
Updates the `AppInitializer` service to align with the core package refactoring. The `initializeApp` and `handleUserTransition` methods have been updated to fetch and manage the renamed `AppSettings` model. All checks for maintenance mode and forced updates now use the new hierarchical `RemoteConfig` paths (e.g., `remoteConfig.app.maintenance.isUnderMaintenance` and `remoteConfig.app.update`). This ensures the entire application startup and user transition logic is consistent with the new, more structured configuration models.
Updates the `DemoDataInitializerService` to create and manage `AppSettings` objects instead of the old `UserAppSettings`. The service now correctly clones fixture data into the new `AppSettings` model for new demo users, ensuring that the demo environment's data initialization process is aligned with the core model refactoring.
Updates the `DemoDataMigrationService` to handle the migration of the renamed `AppSettings` model when an anonymous demo user signs in. This change ensures that the data migration process for the demo environment correctly operates on the new `AppSettings` data structure, maintaining consistency with the core model updates.
Updates the application's `bootstrap` function to align with the core package refactoring. Dependency injection for `DataRepository<UserAppSettings>` has been changed to `DataRepository<AppSettings>`, and the `DemoDataInitializerService` and `DemoDataMigrationService` are now correctly instantiated with the new type. Logic for selecting the `PushNotificationService` and checking for maintenance mode has been updated to use the new hierarchical `RemoteConfig` structure (e.g., `remoteConfig.features.pushNotifications` and `remoteConfig.app.maintenance`), ensuring the entire dependency setup is consistent with the updated data contracts.
- Remove 'hide AppStatus' from core package import - Update DataRepository type from UserAppSettings to AppSettings
Updates the `SavedHeadlinesPage` to be compatible with the core model refactoring. The logic for determining the headline's image style now correctly accesses `appState.settings.feedSettings.feedItemImageStyle` instead of the obsolete `feedPreferences` path. The `HeadlineImageStyle` enum has also been replaced with the renamed `FeedItemImageStyle`, ensuring the view correctly interprets and displays saved headlines according to the new, unified settings structure.
Updates the `InAppNotificationCenterPage` to correctly handle deep-linking from notifications using the refactored `PushNotificationPayload` model. The logic now accesses the `contentType` and `contentId` directly from the payload object instead of parsing a generic `data` map. This change makes the navigation logic more robust, type-safe, and aligned with the standardized push notification contract.
Updates the `InAppNotificationListItem` to align with the refactored `PushNotificationPayload`, which no longer includes a `body` field. The widget has been simplified to a two-line list tile, displaying only the notification title and the relative timestamp. This ensures the UI correctly renders the available notification data without causing errors.
Updates the `EntityDetailsPage` to align with the core package refactoring. The widget now correctly reads the `feedItemImageStyle` from the new `appState.feedItemImageStyle` getter. The logic for loading ads has also been updated to access the `AdConfig` from the new hierarchical path (`remoteConfig.features.ads`), ensuring that both user settings and remote configurations are consumed correctly.
Updates the `EntityDetailsBloc` to be compatible with the latest core package changes. All data fetching and ad injection logic now correctly accesses the `feedItemImageStyle` from the new `appState.feedItemImageStyle` getter. References to the ad configuration have been updated to use the new hierarchical `RemoteConfig` path (`remoteConfig.features.ads`), ensuring the BLoC operates correctly with the updated data contracts.
- Remove 'hide AppStatus' directive from core package import - This change simplifies the import statement and removes unnecessary hiding of AppStatus
Renames HeadlineImageStyle to FeedItemImageStyle to align with core package updates.
Renames HeadlineImageStyle to FeedItemImageStyle to align with core package updates.
Adapts AdMobAdProvider to use the new FeedItemImageStyle enum for native
and banner ad sizing, aligning with core package updates.
…le ads
Adapts AdService to the new hierarchical AdConfig structure, specifically
accessing ad configurations via `features.ads`. Removes the now-obsolete
`getInArticleAd` method and simplifies `_loadInlineAd` by removing
in-article specific parameters and logic, aligning with the core package
ad configuration simplification. Updates enum usage to FeedItemImageStyle.
Adjusts InterstitialAdManager to access ad configurations via the new
`remoteConfig.features.ads.navigationAdConfiguration` path and uses the
`internalNavigationsBeforeShowingInterstitialAd` property for frequency
tracking, aligning with core package updates.
Adapts AdmobInlineAdWidget to use the renamed FeedItemImageStyle enum.
Removes the now-obsolete `bannerAdShape` parameter, as in-article ad
logic has been removed from the AdService, simplifying the widget's API.
Adapts DemoBannerAdWidget to use the renamed FeedItemImageStyle enum.
Removes the now-obsolete `bannerAdShape` parameter to align with the
removal of in-article ad logic.
Renames HeadlineImageStyle to FeedItemImageStyle to align with core
package updates.
Updates FeedAdLoaderWidget to access the ad configuration via the new
`remoteConfig.features.ads` path. Adapts to use `FeedItemImageStyle`
from the `AppBloc`'s `FeedSettings` for determining ad size, aligning
with the core package refactor.
…ionPayload
Updates the `FirebasePushNotificationService` to correctly construct the new standardized `PushNotificationPayload`. This involves mapping the incoming `RemoteMessage` data to the new explicit fields (`notificationId`, `notificationType`, `contentType`, `contentId`) and removing the now-obsolete `body` and `data` fields from the payload constructor, ensuring compatibility with the updated `core` package.
…ionPayload
Updates the `OneSignalPushNotificationService` to correctly construct the new standardized `PushNotificationPayload`. This change maps the `additionalData` from the OneSignal notification to the new explicit fields (`notificationId`, `notificationType`, `contentType`, `contentId`) and removes the obsolete `body` and `data` fields from the payload constructor, ensuring alignment with the `core` package update.
- Add null safety check for widget lifecycle state in user subscription - Add null safety check for push notification subscription
- Add 'use_build_context_synchronously' to ignore list in analysis options - This change allows ignoring warnings related to using BuildContext synchronously
- Make the loading dialog resilient to context changes - Use unawaited to show the dialog without waiting for it to complete - Refactor dialog logic to use a single try-finally block - Ensure the dialog is dismissed properly even if the context changes
- Remove unused colorScheme variable - Delete trailing button code from all HeadlineTile widgets - Simplify headline tile creation based on image style
- Remove optional trailing widget - Add more actions icon button - Implement HeadlineActionsBottomSheet on icon press - Adjust date text padding
- Remove unnecessary 'tile' variable and simplify the switch statement - Directly return the appropriate HeadlineTile widget in each case - Remove redundant IconButton (trailing) which was not in use
- Remove trailing parameter and related code from HeadlineTileImageStart, HeadlineTileImageTop, and HeadlineTileTextOnly widgets - This change simplifies the headline tile widgets by removing the optional trailing widget, which was not being used
…sues - Add Directionality widget around NotificationIndicator in both DiscoverSliverAppBar and FeedSliverAppBar - Use Directionality.of(context) to inherit the correct text direction - This change ensures proper layout behavior for right-to-left languages
- Replace more_vert icon with more_horiz for better alignment - Use Positioned.directional and Directionality.of() to ensure correct positioning in different locales - Wrap PopupMenuButton in Directionality widget for correct text direction handling
…undant code - Update NotificationIndicator to use Positioned.directional for better alignment - Remove redundant Directionality widget in discover_sliver_app_bar.dart and feed_sliver_app_bar.dart - Adjust positioning values for the indicator in notification_indicator.dart
- Replaces login icon with sync icon for users who are not logged in - This visual change highlights the action of "syncing" or "linking" accounts - Improves user interface consistency with common UI patterns
Summary of ChangesHello @fulleni, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a substantial refactoring of the application's core structure, focusing on a more robust and flexible news aggregation experience. It overhauls how application settings and remote configurations are managed, particularly for ad display and user preferences. A major change is the removal of the dedicated article details page, with content now opening directly via URLs, managed by a new centralized handler that also integrates ad display logic. These changes aim to improve maintainability, user experience, and the overall content delivery model. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a major refactoring that pivots the application from a self-contained news reader to a news aggregator. Key changes include removing the in-app article details page in favor of opening original article URLs via a new HeadlineTapHandler. This handler also introduces more granular control over interstitial ads by distinguishing between internal and external navigation. The UI has been improved by centralizing headline actions like bookmarking and sharing into a new bottom sheet, decluttering the feed tiles. The data models for remote configuration, app settings, and push notifications have been significantly refactored, and these changes are applied consistently throughout the codebase. Overall, this is a well-executed and substantial refactoring. My main concern is the decision to globally disable the use_build_context_synchronously lint rule, which poses a risk to the app's stability.
- Modify Arabic title from "عرض الموجز" to "الموجز" - Update English description for feed settings section
- Remove WidgetsFlutterBinding.ensureInitialized() from bootstrap process - Add condition to skip Firebase initialization in demo environment - Update main.dart to reflect new initialization logic
- Remove the FilledButton widget from the DemoInterstitialAdDialog - The button functionality is now provided by the continue icon button
- Add logic to count unread notifications when the app starts - Update state to reflect unread notifications status - Implement error handling for notification count retrieval
- Added flutter_inappwebview: 6.1.5 to pubspec.yaml - Updated pubspec.lock with new dependencies
- Implement InAppBrowser as a modal widget using InAppWebView - Include custom app bar with close button and progress indicator - Restrict navigation to initial domain to maintain focus - Add show method for easy presentation as a modal dialog
… navigation - Replace url_launcher package with custom InAppBrowser for internal navigation - Update HeadlineTapHandler to use InAppBrowser for internal navigation - Maintain external application launch for external navigation - Remove dependency on url_launcher_string.dart
Replace IconButton with InkWell + Icon to precisely control padding and constraints, eliminating the default minimum touch target size that causes height misalignment on native platforms.
fulleni
deleted the
refactor/app-identity-pivot-into-a-news-aggregator
branch
github-project-automation
bot
moved this from Backlog
to Done
in Flutter News App Project
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Status
READY
Description
This pull request introduces a substantial refactoring of the application's core structure, focusing on a more robust and flexible news aggregation experience. It overhauls how application settings and remote configurations are managed, particularly for ad display and user preferences. A major change is the removal of the dedicated article details page, with content now opening directly via URLs, managed by a new centralized handler that also integrates ad display logic. These changes aim to improve maintainability, user experience, and the overall content delivery model.
Type of Change