feat: sign in page frontend #121
Conversation
WalkthroughThe changes in the pull request focus on the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
src/routes/login.tsx (2)
Line range hint
21-33: Consider enhancing error handling and user experienceWhile the basic error handling is implemented, consider these improvements:
- Add a loading state during authentication
- Handle network errors separately from authentication errors
- Add input validation before submission
Here's a suggested implementation:
+ const [isLoading, setIsLoading] = useState(false); async function login(event: React.FormEvent<HTMLFormElement>) { event.preventDefault(); + if (!email || !password) { + setIsError(true); + return; + } + setIsLoading(true); + setIsError(false); try { const { data: { session }, error } = await supabase.auth.signInWithPassword({ email, password }); if (error !== null) { setPassword(''); setIsError(true); } if (session !== null) { window.location.href = redirectUrl } + } catch (error) { + setIsError(true); + setPassword(''); + } finally { + setIsLoading(false); + } }
92-99: Consider implementing proper internationalizationThe current Labels class uses hardcoded Chinese strings. Consider using a proper i18n solution:
- Consider using react-i18next or similar libraries for better internationalization support
- Move translations to separate JSON files
- Add language switching capability
Example implementation with react-i18next:
// locales/zh/common.json { "login": { "account": "信箱或手機號碼", "password": "密碼", "remember": "保持我的登入狀態", "login": "登入", "invalidCredentials": "登入失敗" } } // Using translations import { useTranslation } from 'react-i18next'; function LoginPage() { const { t } = useTranslation(); // Use t('login.account') instead of Labels.account }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/routes/login.tsx(3 hunks)
🔇 Additional comments (1)
src/routes/login.tsx (1)
Line range hint 1-18: LGTM: Clean state management implementation
The new state variables for handling remember-me functionality and error states are well-implemented using React's useState hook.
1989ONCE
requested review from
yeahlowflicker and
1989ONCE
and removed request for
yeahlowflicker
1989ONCE
dismissed stale reviews from yeahlowflicker and themself
via
b9317a1
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
src/routes/login.tsx (4)
Line range hint
21-33: Enhance error handling and add loading stateThe current implementation could benefit from:
- Input validation before submission
- Loading state during authentication
- More specific error handling
Consider implementing these improvements:
function LoginPage() { const [email, setEmail] = useState('') const [password, setPassword] = useState('') const [isRemember, setIsRemember] = useState(false); const [isError, setIsError] = useState(false); + const [isLoading, setIsLoading] = useState(false); + const [errorMessage, setErrorMessage] = useState(''); async function login(event: React.FormEvent<HTMLFormElement>) { event.preventDefault(); + + // Basic validation + if (!email.trim() || !password.trim()) { + setErrorMessage(Labels.emptyFields); + return; + } + + setIsLoading(true); + setIsError(false); + setErrorMessage(''); + try { const { data: { session }, error } = await supabase.auth.signInWithPassword({ email, password }); if (error) { setPassword(''); setIsError(true); + setErrorMessage(error.message); + return; } if (session) { window.location.href = redirectUrl; } } catch (error) { + setErrorMessage(Labels.unexpectedError); + setIsError(true); } finally { + setIsLoading(false); } }
73-81: Improve error message implementationThe current error message implementation can be enhanced:
- Use proper ARIA attributes
- Simplify the conditional rendering
- { - isError - ? ( - <span className='text-red-500 font-bold text-2xl'> - {isError ? Labels.wrongAccountOrPassword : ''} - </span> - ) - : <></> - } + {isError && ( + <div + role="alert" + className='text-red-500 font-bold text-2xl' + aria-live="polite" + > + {errorMessage || Labels.wrongAccountOrPassword} + </div> + )}
82-87: Enhance submit button accessibility and loading stateThe submit button should indicate loading state and be disabled during submission.
<button type='submit' - className='w-full h-20 rounded-md bg-green-600 text-white font-extrabold text-5xl' + className='w-full h-20 rounded-md bg-green-600 text-white font-extrabold text-5xl disabled:opacity-50' + disabled={isLoading} + aria-busy={isLoading} > - {Labels.login} + {isLoading ? Labels.loggingIn : Labels.login} </button>
92-99: Consider enhancing the Labels implementationWhile centralizing strings is good practice, consider:
- Moving Labels to a dedicated localization file
- Adding missing labels for loading and validation states
Create a new file
src/locales/zh.ts:export const zhTW = { account: '信箱或手機號碼', password: '密碼', remember: '保持我的登入狀態', login: '登入', loggingIn: '登入中...', wrongAccountOrPassword: '帳號或密碼錯誤', emptyFields: '請填寫所有欄位', unexpectedError: '發生未預期的錯誤', };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/routes/login.tsx(3 hunks)
🔇 Additional comments (1)
src/routes/login.tsx (1)
40-53: Add missing accessibility attributes to form inputs
The form inputs need proper accessibility attributes as mentioned in the previous review.
Description
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Screenshot Preview