Refactor enhance app config UI #31
Conversation
- Added tabbed interface for better organization. - Replaced ExpansionTiles with TabBarView. - Improved section descriptions for clarity. - Updated UI for better user experience. - Added maintenance and force update sections.
- Improved descriptions for clarity - Renamed tabs for better UX - Added new sections for settings - Removed redundant descriptions - Updated translations for all languages
- Replaced hardcoded strings with l10n - Improved UI readability and maintainability - Updated tab labels and descriptions - Updated expansion tile titles and descriptions - Minor formatting improvements
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 introduces a significant refactor of the App Configuration user interface, transitioning from a single scrollable list of settings to a more organized tab-based navigation. This change aims to enhance usability by categorizing settings into intuitive sections, making it quicker for users to find and manage specific application configurations. The refactor also includes comprehensive updates to localization strings to align with the new UI structure and improve overall clarity.
Highlights
- UI Refactor: The App Configuration page has been significantly refactored to introduce a tab-based navigation system, enhancing organization and user experience.
- Categorized Settings: Settings are now logically grouped under new 'Feed', 'Advertisements', and 'General' tabs, making it easier for users to locate specific configurations.
- Improved Modularity: The previous 'App Operational Status' section has been split into distinct 'Maintenance Mode' and 'Force App Update' sections, improving code organization and clarity.
- Localization Updates: New and updated localization strings provide clearer titles and descriptions for the various configuration sections, aligning with the new UI structure and improving overall clarity.
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 is currently in preview and 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 to provide feedback.
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.
Code Review
The pull request refactors the App Configuration UI with tab-based navigation, improving organization and usability. Key issues identified include duplicate keys in .arb files, a missing warning message, and redundant code. Addressing these will enhance the UI's stability and quality.
| "forceUpdateDescription": "تكوين تحديثات التطبيق الإلزامية للمستخدمين.", | ||
| "@forceUpdateDescription": { | ||
| "description": "وصف قسم فرض تحديث التطبيق" | ||
| }, |
There was a problem hiding this comment.
This change introduces a new forceUpdateDescription key, but an older key with the same name already exists in this file. This creates a duplicate key in the .arb file, which is invalid and will likely cause localization generation to fail or behave unexpectedly. Please remove the old forceUpdateDescription key to resolve this.
| "forceUpdateDescription": "Configure mandatory app updates for users.", | ||
| "@forceUpdateDescription": { | ||
| "description": "Description for the Force App Update section" | ||
| }, |
There was a problem hiding this comment.
This change introduces a new forceUpdateDescription key, but an older key with the same name already exists in this file. This creates a duplicate key in the .arb file, which is invalid and will likely cause localization generation to fail or behave unexpectedly. Please remove the old forceUpdateDescription key to resolve this.
| children: [ | ||
| _buildAppStatusSection(context, remoteConfig), | ||
| _buildMaintenanceSection(context, remoteConfig), | ||
| const SizedBox(height: AppSpacing.lg), | ||
| _buildForceUpdateSection(context, remoteConfig), | ||
| ], |
There was a problem hiding this comment.
The refactoring split _buildAppStatusSection into more focused widgets, which is great for clarity. However, the important warning message l10n.appOperationalStatusWarning is no longer displayed. Given the critical nature of these settings, this warning should be restored and made visible within the "General" tab, perhaps above the two new ExpansionTile widgets, to ensure administrators are aware of the impact of their changes.
children: [
Text(
l10n.appOperationalStatusWarning,
style: Theme.of(context).textTheme.bodyMedium?.copyWith(
color: Theme.of(context).colorScheme.error,
fontWeight: FontWeight.bold,
),
),
const SizedBox(height: AppSpacing.lg),
_buildMaintenanceSection(context, remoteConfig),
const SizedBox(height: AppSpacing.lg),
_buildForceUpdateSection(context, remoteConfig),
],
| final currentMap = Map<FeedActionType, int>.from( | ||
| _getDaysMap(accountActionConfig), | ||
| ); | ||
| final updatedMap = Map<FeedActionType, int>.from(currentMap) | ||
| ..[actionType] = value; |
There was a problem hiding this comment.
There's a redundant map creation here. currentMap is created as a copy of the map from _getDaysMap, and then updatedMap is created as another copy of currentMap. This is inefficient. You can create the updatedMap directly from the result of _getDaysMap in a single step, which is more concise and performant.
final updatedMap = Map<FeedActionType, int>.from(
_getDaysMap(accountActionConfig),
)..update(actionType, (value) => value, ifAbsent: () => value);
Status
READY/IN DEVELOPMENT/HOLD
Description
Type of Change