web: improve login form UX (paste, visibility, Enter submit) - #108
web: improve login form UX (paste, visibility, Enter submit)#108ddc-baiye wants to merge 1 commit into
Conversation
Summary of ChangesHello @ddc-baiye, 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 enhances the user experience of the login page by introducing several quality-of-life improvements. These changes, which are entirely confined to the UI, allow operators to more efficiently and comfortably log in by providing options for pasting tokens, toggling token visibility, and submitting the form with the Enter key, all without affecting any backend logic or API contracts. 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.
Code Review
This pull request significantly improves the user experience on the login page by adding features like pasting a token from the clipboard, toggling token visibility, and submitting the form by pressing Enter. The implementation is well-done. I've provided a couple of suggestions to further refine the code, mainly around error handling in the paste functionality and improving code structure for better maintainability.
| const handlePaste = async () => { | ||
| try { | ||
| if (typeof navigator !== 'undefined' && navigator.clipboard && typeof navigator.clipboard.readText === 'function') { | ||
| const text = await navigator.clipboard.readText(); | ||
| setToken(text || ''); | ||
| setTokenError(''); | ||
| } | ||
| } catch (_) {} | ||
| }; |
There was a problem hiding this comment.
The handlePaste function currently swallows errors silently with an empty catch block. This can make debugging difficult if clipboard access fails for any reason (e.g., user denies permission, browser incompatibility). It's better to at least log the error to the console for debugging purposes. Additionally, the check for navigator.clipboard.readText can be simplified using optional chaining.
const handlePaste = async () => {
try {
if (navigator.clipboard?.readText) {
const text = await navigator.clipboard.readText();
setToken(text);
setTokenError('');
}
} catch (err) {
console.error('Failed to paste from clipboard:', err);
}
};
| ), | ||
| endAdornment: ( | ||
| <InputAdornment position="end"> | ||
| <IconButton aria-label="paste token" onClick={handlePaste} edge="end"> |
There was a problem hiding this comment.
The edge="end" prop on an IconButton within an InputAdornment is intended for the very last element to ensure correct padding and alignment. Since there are two IconButtons here, only the second one (the visibility toggle) should have this prop. Please remove edge="end" from the paste button's IconButton for proper visual spacing.
<IconButton aria-label="paste token" onClick={handlePaste}>
| onChange={(e) => { setToken(e.target.value); if (tokenError) setTokenError(''); }} | ||
| onKeyDown={(e) => { if (e.key === 'Enter') handleLogin(); }} |
There was a problem hiding this comment.
For better readability and maintainability, it's a good practice to extract inline event handlers with logic into separate named functions. This keeps the JSX cleaner and makes the component's logic easier to understand and test.
You could define these handlers within the LoginPage component:
const handleTokenChange = (e: React.ChangeEvent<HTMLInputElement>) => {
setToken(e.target.value);
if (tokenError) {
setTokenError('');
}
};
const handleKeyDown = (e: React.KeyboardEvent) => {
if (e.key === 'Enter') {
handleLogin();
}
};And then use them in the TextField as onChange={handleTokenChange} and onKeyDown={handleKeyDown}.
8c4068d to
7d8708c
Compare
Signed-off-by: ddc-baiye <dongdong.chen@bluedotai.cn>
7d8708c to
a65f001
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ddc-baiye The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Type:
kind feature
What this PR does / why we need it
Improves the login page UX by adding clipboard paste for token, show/hide token visibility, and Enter-to-submit. This reduces friction for operators without touching global request/middleware logic.
Special notes for your reviewer
UI-only change in the login page; no backend or API contracts affected. Scope limited to modules/web/src/app/login/page.tsx.
Which issue(s) this PR fixes
NONE
Release note
Enhance login form usability
with paste, visibility toggle,
and Enter-to-submit