-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor/auth flow and account linking #142
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
Conversation
- Create new top-level route for account linking - Implement modal presentation for account linking flow - Simplify authentication page by removing linking context parameters - Enhance route protection logic for better clarity and security - Update routing structure for improved maintainability
- Replace generic authentication route with specific account linking route - Simplify onPressed logic in ListTile widget
- Remove account linking functionality - Exclude anonymous sign-in button (to be added later) - Update UI and copy for new user onboarding
- Remove isLinkingContext parameter from RequestCodePage - Update back navigation to use context.pop() consistently - Modify successful request navigation to use relative paths - Remove commented-out demo email suggestion code
- Create a new page for anonymous users to link their account to an email - Display options for account linking in a modal style - Show loading indicator and error handling - Add localization support for page content
…tion handling - Add detailed comments on router setup, auth handling, and top-level routes - Clarify the purpose and structure of global article details route - Explain the reasoning behind using a top-level route for article details - Update documentation on authentication route and app lifecycle integration
- Remove hardcoded headline and subheadline strings - Replace static strings with localization (l10n) values - Update UI to use localized strings for better internationalization support
…ontext - Add conditional navigation based on current route name - Use named routes for account linking and standard authentication - Pass email as a path parameter to verification page
- Replace MaterialPage with CustomTransitionPage for modal bottom sheet presentation - Implement slide-up animation for the modal bottom sheet - Add barrier dismissible feature to allow closing the modal by tapping outside
…sheet - Replace Scaffold with DraggableScrollableSheet for a bottom sheet layout - Add drag handle and close button for improved user experience - Maintain visibility of the previous page underneath the sheet - Allow sheet to be dismissed by dragging down
- Wrap DraggableScrollableSheet with GestureDetector to dismiss modal by tapping scrim - Add second GestureDetector inside sheet to prevent closing on inner content taps - Improve modal styling and layout
- Change context.goNamed to context.pushNamed for account linking button - This modification ensures the correct navigation behavior in the account page
- Remove modal bottom sheet presentation - Replace with full-screen scaffold - Add AppBar for navigation - Simplify layout structure - Remove unnecessary gestures and animations
- Remove CustomTransitionPage for modal bottom sheet presentation - Revert to using a simpler builder function for AccountLinking route - This change reduces complexity and improves performance for account linking flow
…p_en.arb - Arabic: changed from "أنشئ حسابًا أو اربطه لحفظ تقدمك" to "أحفظ تقدمك" - English: changed from "Create or Link Account to Save Progress" to "Link Account"
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 significantly refactors the application's authentication and account linking mechanisms. By introducing a dedicated page for account linking and simplifying the main authentication page, the PR aims to improve the clarity, maintainability, and user experience of these critical flows. The underlying routing logic has been comprehensively updated to reflect this architectural change, ensuring correct navigation and state management across different user authentication statuses. 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
|
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 great refactoring of the authentication and account linking flows. Separating the account linking process into its own dedicated page and top-level route significantly simplifies the existing AuthenticationPage and clarifies the routing logic in go_router. The new redirect logic is much cleaner and easier to follow.
I have one suggestion in request_code_page.dart to improve decoupling and make the component more reusable by using a callback for navigation instead of inspecting the router's state. Overall, these are excellent changes that improve the maintainability and structure of the authentication feature.
- Remove incorrect/old translations in Arabic and English - Simplify messages to remove reference to "linking" an account - Focus on the benefit of signing up to access information across devices
Status
READY
Description
This pull request significantly refactors the application's authentication and account linking mechanisms. By introducing a dedicated page for account linking and simplifying the main authentication page, the PR aims to improve the clarity, maintainability, and user experience of these critical flows. The underlying routing logic has been comprehensively updated to reflect this architectural change, ensuring correct navigation and state management across different user authentication statuses.
Type of Change