Refatrcor auth hints for demo envirement #24
Conversation
This change updates the request code page to conditionally display a hint for the demo email address (`[email protected]`) when the application is running in the demo environment. A `BlocSelector` is used to check the current environment from the `AppBloc`, ensuring the hint is only shown when appropriate, guiding users during demonstrations.
This change updates the demo verification code hint on the email code verification page to use a hardcoded string. This makes its implementation consistent with the recently added demo email hint, ensuring both are displayed in the same manner during demonstrations.
This change adds new keys and their English translations to the `app_en.arb` file for the demo email and verification code hints. This centralizes the strings for easier management.
This change adds new keys and their Arabic translations to the `app_ar.arb` file for the demo email and verification code hints. This centralizes the strings for easier management.
This change updates the authentication UI pages to use the newly added localization keys (`demoEmailHint` and `demoCodeHint`) instead of hardcoded strings. This ensures that the demo hints are properly translated for both English and Arabic, improving the internationalization of the application.
The AppBloc was receiving the AppEnvironment in its constructor but was not setting it in the initial AppState. This resulted in `state.environment` being null throughout the app, causing conditional UI elements (like demo mode hints) to fail to appear. This change passes the environment to the AppState constructor during bloc initialization and removes the now-unused `_environment` field from the AppBloc.
![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 refactors and enhances the display of authentication hints specifically for the demo environment. It introduces new localized strings for demo email and verification code hints, and ensures these hints are conditionally displayed on the login and code verification pages based on the application's environment. Additionally, it includes a minor refactoring of how the application environment is managed within the AppBloc for improved clarity.
Highlights
- Conditional Demo Hints: Introduced conditional display of authentication hints (email and verification code) on the login and code verification pages. These hints are now shown only when the application is running in a demo environment, providing guidance for demo users.
- Localization Updates: Added new localization keys (
demoEmailHintanddemoCodeHint) to support the display of environment-specific demo hints in multiple languages (English and Arabic), ensuring a consistent user experience across locales. - App Environment Handling Refactor: Refactored the
AppBlocto streamline how the application environment is managed. TheAppEnvironmentis now passed directly to theAppStateduring initialization, removing a redundant field from the bloc itself.
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
Thank you for this refactoring. Moving the environment information into the AppState and using it to conditionally display demo hints is a good improvement for the codebase.
I've identified a couple of areas for improvement:
- In
email_code_verification_page.dart, the refactoring seems incomplete, which could lead to the demo hint being shown in incorrect environments. I've marked this as a high-priority issue. - In
request_code_page.dart, I've suggested moving a hardcoded demo email to a configuration file to improve maintainability.
Also, please note there are a couple of typos in the pull request title: "Refatrcor" should be "Refactor" and "envirement" should be "environment".
Please review the comments. I'm happy to discuss them further.
| Text( | ||
| l10n.demoVerificationCodeMessage('123456'), | ||
| l10n.demoCodeHint('123456'), | ||
| style: textTheme.bodyMedium?.copyWith( | ||
| color: colorScheme.secondary, | ||
| fontWeight: FontWeight.bold, |
There was a problem hiding this comment.
This refactoring appears to be incomplete. The hint for the demo verification code should only be displayed in the demo environment.
There are two issues here:
- Incorrect condition: This hint is likely displayed based on a condition like
kDebugMode(which is outside the diff but can be inferred from similar code), but it should be conditional onstate.environment == AppEnvironment.demo. Please use aBlocSelectorto check the environment, similar to the implementation inrequest_code_page.dart. This will fix the bug where the hint is shown in non-demo debug builds and hidden in release demo builds. - Hardcoded value: The verification code
'123456'is hardcoded. For better maintainability, consider moving this value intoAppConfig.
| return Padding( | ||
| padding: const EdgeInsets.only(top: AppSpacing.lg), | ||
| child: Text( | ||
| l10n.demoEmailHint('[email protected]'), |
There was a problem hiding this comment.
The demo email '[email protected]' is hardcoded. To improve maintainability and centralize configuration, please consider moving this value to AppConfig. This makes it easier to manage demo credentials without changing the UI code.
Status
READY/IN DEVELOPMENT/HOLD
Description
Type of Change