-
Notifications
You must be signed in to change notification settings - Fork 44
feat(auth): improve UX, validation & edge-case handling for forgot/reset password #85
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
base: master
Are you sure you want to change the base?
feat(auth): improve UX, validation & edge-case handling for forgot/reset password #85
Conversation
Summary of ChangesHello @sahilmane69, 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, despite its title suggesting improvements to authentication flows, primarily introduces a new frontend feature: a 'DatasetUpload' component. This component allows users to select and upload training data files, providing visual feedback on the upload status and handling potential network or server errors. Additionally, the '.gitignore' file has been updated to properly exclude 'node_modules/'. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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 introduces a new DatasetUpload component, enhancing the frontend's user experience for file uploads by incorporating loading states, basic validation, and feedback mechanisms. Additionally, it correctly adds node_modules/ to the .gitignore file. The changes are well-scoped to the frontend as described.
| formData.append('file', file) | ||
|
|
||
| try { | ||
| const response = await fetch('/api/v1/training-datasets/upload', { |
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.
The API endpoint /api/v1/training-datasets/upload is hardcoded directly in the component. For better maintainability and flexibility across different environments (development, staging, production), it's recommended to manage API endpoints through environment variables or a centralized configuration file. This allows for easier updates without modifying the component's source code.
| if (response.ok) { | ||
| setStatus({ type: 'success', message: 'Dataset uploaded successfully!' }) | ||
| setFile(null) | ||
| } else { | ||
| setStatus({ type: 'error', message: 'Failed to upload dataset. Please try again.' }) |
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.
The error messages for failed uploads are quite generic ('Failed to upload dataset. Please try again.' and 'Network error. Please check your connection.'). While these are good defaults, it would significantly improve the user experience and debugging if more specific error messages from the backend could be parsed and displayed. For example, if the backend returns a JSON object with an error field, you could display that content.
Consider checking response.json() for more detailed error information when response.ok is false.
| <input | ||
| type="file" | ||
| onChange={handleFileChange} | ||
| disabled={isUploading} | ||
| className="block w-full text-sm text-gray-400 | ||
| file:mr-4 file:py-2 file:px-4 | ||
| file:rounded-full file:border-0 | ||
| file:text-sm file:font-semibold | ||
| file:bg-indigo-600 file:text-white | ||
| hover:file:bg-indigo-700 | ||
| disabled:opacity-50 disabled:cursor-not-allowed" |
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.
There is no client-side validation for the file type. While backend validation is crucial for security, adding client-side validation (e.g., checking file.type or file.name extension) can provide immediate feedback to the user, improving the user experience by preventing them from attempting to upload unsupported file types. You could also use the accept attribute on the input tag to suggest allowed file types to the browser.
| const handleFileChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| if (e.target.files && e.target.files[0]) { | ||
| setFile(e.target.files[0]) | ||
| setStatus(null) |
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.
Clearing the status immediately when a new file is selected (setStatus(null)) might remove important feedback from a previous upload attempt (e.g., a 'failed' message) before the user has fully processed it. It might be better to clear the status only when a new upload process is initiated (e.g., at the beginning of handleSubmit) or provide a clear 'dismiss' option for the status message.
This PR improves the frontend UX and validation for the Forgot Password and Reset Password flows.
It adds better loading states, form validation, clear success/error feedback, and safe handling of missing or invalid reset tokens. The changes are frontend-only and intentionally scoped to avoid any overlap with backend logic.
Related Issue: #58