-
Notifications
You must be signed in to change notification settings - Fork 0
Adapt ad config UI to the remote config updated object #75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adapt ad config UI to the remote config updated object #75
Conversation
- Update core.git resolved-ref from ed5aa8b4444d6e4627ae6ca9fb28058009e7ffd7 to 36be72f1b7bd22236eb0cdf97e5f2545c7a47d34 - Update pinput package from 5.0.1 to 5.0.2
- Add Arabic and English translations for new ad settings features - Reorganize interstitial ad settings structure in both languages - Introduce global ad enable/disable switch - Include feed-to-article interstitial ad ID and descriptions - Update user role interstitial frequency descriptions - Add transitions before interstitial ads labels and descriptions
- Implement InterstitialAdSettingsForm widget for configuring global interstitial ad settings - Add functionality to handle different user roles and their ad frequency settings - Include enable/disable toggle for interstitial ads - Implement dynamic form fields for transitions before showing interstitial ads
- Rename 'articleInterstitialAdId' to 'feedToArticleInterstitialAdId' - Add AbsorbPointer and Opacity to gray out form when ads are disabled - Adjust padding and alignment for better visual hierarchy - Reorganize widgets for improved code structure and readability
- Remove unused imports and code related to user roles and interstitial ads - Wrap form content with AbsorbPointer and Opacity widgets - Reorganize UI elements and spacing - Update switch label and logic for enabling article ads - Combine in-article ad slot placements into a single ExpansionTile
- Remove articles before interstitial ad settings - Add global ad enable/disable toggle - Gray out settings when ads are disabled - Reorganize layout for better clarity
…tings - Add AdConfigForm for global ad configuration - Include InterstitialAdSettingsForm in the ad settings section - Disable expansion of ad platform configurations if global ads are off - Adjust the layout to accommodate the new global ad switch
- Remove duplicate imports - Remove duplicate code for role-specific fields - Reduce complexity by removing unnecessary methods
- Add Arabic and English translations for AdMob and Local ad platform names - Introduce new localization keys for ad platform types
- Create new extension on AdPlatformType for localized string representations - Add cases for AdPlatformType.admob and AdPlatformType.local - Import necessary packages and use context.l10n for localization
- Import AdPlatformTypeL10n extension - Replace platform name with localized version in AdPlatformConfigForm
…are off - Add 'enabled' property to ExpansionTile to completely disable tiles when ads are off - This change ensures that ad platform tiles are not just non-expandable, but also disabled - The fix is applied to all ad platform tiles: AdPlatformConfig, FeedAdSettings, ArticleAdSettings, and InterstitialAdSettings
- Remove AbsorbPointer and Opacity widgets to improve performance - Simplify the layout by using a single Column widget - Maintain the existing functionality and UI structure
…city widgets - Removed AbsorbPointer and Opacity widgets that were used to disable the form - Simplified the layout by directly returning a Column widget - This change improves the code structure and may enhance performance slightly
- Remove AbsorbPointer and Opacity widgets that were used to disable the form - Keep the form structure and content intact - Update commit message to reflect the changes made to the code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @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 focuses on enhancing the maintainability and clarity of the application's ad configuration. It streamlines the user interface for managing advertisements by centralizing related settings and improving the logical flow of configuration options. The changes also include a structural reorganization of common UI elements to promote better code organization.
Highlights
- Ad Configuration UI Refactor: The ad configuration user interface has undergone a significant refactoring. Interstitial ad settings are now centralized in a new dedicated form, and a global switch to enable/disable all ads has been introduced. Other ad setting sections are now dynamically enabled or disabled based on this global switch.
- Interstitial Ad Settings Centralization: A new
InterstitialAdSettingsFormhas been created to consolidate all interstitial ad-related configurations, including enabling/disabling and user-role-based frequency settings, which were previously spread across multiple forms. - Localization and Naming Updates: Localization keys have been updated and new ones added to support the refactored ad settings, including a clearer naming convention for interstitial ad identifiers (e.g.,
feedToArticleInterstitialAdId). A new metric, 'Transitions Before Interstitial Ads,' has been introduced for interstitial ad frequency. - Shared UI Component Relocation: Several shared UI components related to selection pages (
searchable_selection_input,searchable_selection_page, and their associated BLoC files) have been structurally refactored by moving them to a new, more organized location within thelib/shared/widgets/directory.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request is a significant and well-executed refactoring of the ad configuration UI. It successfully breaks down large, complex widgets into smaller, more focused ones, such as the new InterstitialAdSettingsForm. This greatly improves maintainability and clarity. The introduction of localization for ad platform types and the cleanup of file structure are also welcome improvements. I have a couple of suggestions to further enhance the code quality by addressing some code duplication and a minor file formatting issue.
Status
READY
Description
This pull request focuses on enhancing the maintainability and clarity of the application's ad configuration. It streamlines the user interface for managing advertisements by centralizing related settings and improving the logical flow of configuration options. The changes also include a structural reorganization of common UI elements to promote better code organization.
Type of Change