fix(ux): show inline validation feedback for empty api key#859
fix(ux): show inline validation feedback for empty api key#859Abhi666-max wants to merge 2 commits into
Conversation
🚀 Thank You for Contributing to Late-MeetPlease ensure that:
Thank you for contributing 💙 |
github-actions
Bot
added
gssoc
|
👋 Thank you @Abhi666-max for your contribution to Late-Meet! ✅ Verified: You are assigned to the linked issue #858. Please review any automated suggestions or code review comments that may appear below! We will review your PR as soon as possible! Please consider starring the repository ⭐ to show your support! |
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a hidden inline error message element in the popup's setup view for API key validation. Updates popup.ts to display this error when an empty API key is submitted and hide it when the user begins typing in the input field. ChangesAPI key validation feedback
Estimated code review effort: 1 (Trivial) | ~5 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant PopupUI
participant PopupTS
User->>PopupUI: Click "save keys" with empty api-key-input
PopupTS->>PopupTS: Detect empty apiKey
PopupTS->>PopupUI: Set api-key-error display = block
User->>PopupUI: Start typing in api-key-input
PopupUI->>PopupTS: input event fired
PopupTS->>PopupUI: Set api-key-error display = none
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/popup.html (2)
67-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInline styling duplicates existing
passphrase-statuspattern.Consider extracting a shared
.field-errorCSS class instead of repeating inline styles (as done forpassphrase-statusabove), for consistency and easier theming later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/popup.html` around lines 67 - 69, The API key error message in the popup uses repeated inline styles instead of the shared styling pattern already used by passphrase-status. Update the api-key-error element in popup.html to use a reusable .field-error class and move the shared error presentation rules into the stylesheet so both fields stay consistent and easier to theme.
67-69: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd
role="alert"/aria-liveand associate error with input for screen readers.The error message is toggled purely via inline
displaystyling with no ARIA live region, so screen reader users won't be notified when it appears. Also, it isn't linked toapi-key-inputviaaria-describedby, so assistive tech won't announce it in field context.♿ Proposed fix
- <input type="password" id="api-key-input" class="setup-input" placeholder="sk-..." /> + <input + type="password" + id="api-key-input" + class="setup-input" + placeholder="sk-..." + aria-describedby="api-key-error" + />- <span id="api-key-error" style="color: `#ef4444`; font-size: 12px; display: none; margin-top: 4px;">API key cannot be - empty</span> + <span id="api-key-error" role="alert" style="color: `#ef4444`; font-size: 12px; display: none; margin-top: 4px;">API key cannot be + empty</span>Accessibility guidance recommends associating error text with inputs via
aria-describedbyand using a live region (e.g.role="alert") so dynamically shown errors are announced.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/popup.html` around lines 67 - 69, Add accessibility attributes to the API key validation message so it is announced by assistive tech: update the api-key-error element to use an alert/live region role, and link it to api-key-input with aria-describedby. Make sure the error remains toggled visually as before, but the input and error message are semantically connected so screen readers announce the message in context when it appears.src/popup.ts (2)
108-109: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider setting
aria-invalidon the input alongside showing the error.The error span is shown, but the associated
api-key-inputisn't marked invalid, so screen reader users navigating by field won't get the failure context. Pairing this with thearia-describedby/role="alert"suggestion inpopup.htmlwould close this gap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/popup.ts` around lines 108 - 109, The API key validation flow in popup.ts shows the error message but does not mark the input as invalid, so update the api-key-input handling in the same branch that toggles api-key-error to also set aria-invalid on the input when validation fails and clear it when the error is dismissed or the field becomes valid. Use the existing popup.ts validation logic and the api-key-input/api-key-error elements together so screen readers get the invalid-field context alongside the visible error state.
758-768: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse optional chaining per static analysis hint.
🔧 Proposed fix
- apiInputBox.addEventListener('input', () => { - const errorMsg = document.getElementById('api-key-error'); - if (errorMsg && errorMsg.style.display === 'block') { - errorMsg.style.display = 'none'; - } - }); + apiInputBox.addEventListener('input', () => { + const errorMsg = document.getElementById('api-key-error'); + if (errorMsg?.style.display === 'block') { + errorMsg.style.display = 'none'; + } + });As per static analysis hints, "Prefer using an optional chain expression instead, as it's more concise and easier to read."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/popup.ts` around lines 758 - 768, The `apiInputBox`/`api-key-error` DOM lookup and guard logic in `popup.ts` should be simplified using optional chaining as suggested by static analysis. Update the `apiInputBox` check and the `errorMsg` visibility check in the input listener to use optional chain expressions instead of nested `if` guards, while keeping the same behavior in the API key error-clearing flow.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/popup.html`:
- Around line 67-69: The API key error message in the popup uses repeated inline
styles instead of the shared styling pattern already used by passphrase-status.
Update the api-key-error element in popup.html to use a reusable .field-error
class and move the shared error presentation rules into the stylesheet so both
fields stay consistent and easier to theme.
- Around line 67-69: Add accessibility attributes to the API key validation
message so it is announced by assistive tech: update the api-key-error element
to use an alert/live region role, and link it to api-key-input with
aria-describedby. Make sure the error remains toggled visually as before, but
the input and error message are semantically connected so screen readers
announce the message in context when it appears.
In `@src/popup.ts`:
- Around line 108-109: The API key validation flow in popup.ts shows the error
message but does not mark the input as invalid, so update the api-key-input
handling in the same branch that toggles api-key-error to also set aria-invalid
on the input when validation fails and clear it when the error is dismissed or
the field becomes valid. Use the existing popup.ts validation logic and the
api-key-input/api-key-error elements together so screen readers get the
invalid-field context alongside the visible error state.
- Around line 758-768: The `apiInputBox`/`api-key-error` DOM lookup and guard
logic in `popup.ts` should be simplified using optional chaining as suggested by
static analysis. Update the `apiInputBox` check and the `errorMsg` visibility
check in the input listener to use optional chain expressions instead of nested
`if` guards, while keeping the same behavior in the API key error-clearing flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 53fa8646-7a8e-4d1f-9c80-00ce7c7bf910
📒 Files selected for processing (2)
src/popup.htmlsrc/popup.ts
|
|
This PR has been marked as stale due to inactivity. |
1 participant
Description
Added an inline validation error message to the popup UI. Previously, submitting an empty API key only triggered a shake animation without any text feedback. Now, an explicit "API key cannot be empty" message appears below the input to improve UX. Added an event listener to clear the error state as soon as the user starts typing.
Fixes #858
Type of Change
Checklist:
Summary by CodeRabbit