fix(a11y): Add inline error for UserAvatarEditor's URL input#36656
fix(a11y): Add inline error for UserAvatarEditor's URL input#36656dionisio-bot[bot] merged 11 commits intoRocketChat:developfrom
Conversation
|
|
Looks like this PR is ready to merge! 🎉 |
|
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #36656 +/- ##
===========================================
- Coverage 70.55% 70.15% -0.41%
===========================================
Files 3271 3274 +3
Lines 116782 116335 -447
Branches 21090 20818 -272
===========================================
- Hits 82393 81609 -784
+ Misses 32338 31434 -904
- Partials 2051 3292 +1241
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
WalkthroughThe UserAvatarEditor component now includes URL validation for avatar inputs. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Component
participant Validator
participant UI
User->>Component: Enter avatar URL
Component->>Component: Clear avatarUrlError
Component->>UI: Update input display
User->>Component: Click "Add URL"
Component->>Validator: Validate URL with isUrl()
alt URL is valid
Validator-->>Component: ✓ Valid
Component->>Component: Set avatar from URL
Component->>UI: Show avatar
else URL is invalid
Validator-->>Component: ✗ Invalid
Component->>Component: Set avatarUrlError
Component->>UI: Render FieldError
Component->>UI: Disable Add button
end
User->>Component: Edit URL input
Component->>Component: Clear avatarUrlError
Component->>UI: Enable Add button
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
Hi @ergot-rp looks like you used another account for one of the commits: https://github.com/ergotse Do you mind signing the CLA with that account? |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/client/components/avatar/UserAvatarEditor/UserAvatarEditor.tsx (1)
130-143: Add ARIA attributes to TextInput for proper error association.For WCAG compliance, the TextInput should have
aria-describedbyandaria-invalidattributes when an error is present. This ensures screen reader users are properly informed about validation errors.Apply this diff to add the required ARIA attributes:
<TextInput data-qa-id='UserAvatarEditorLink' id={imageUrlField} flexGrow={0} placeholder={t('Use_url_for_avatar')} value={avatarFromUrl} mis={4} onChange={handleAvatarFromUrlChange} + aria-invalid={!!avatarUrlError} + aria-describedby={avatarUrlError ? `${imageUrlField}-error` : undefined} /> {avatarUrlError && ( <FieldError aria-live='assertive' id={`${imageUrlField}-error`}> {avatarUrlError} </FieldError> )}
🧹 Nitpick comments (1)
apps/meteor/client/components/avatar/UserAvatarEditor/UserAvatarEditor.tsx (1)
108-108: Consider the error message clarity.The same error message "Invalid URL" will be displayed both when the URL format is invalid and when a valid URL fails to load the image. This might be slightly confusing to users, but if this aligns with the design requirements, it's acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/client/components/avatar/UserAvatarEditor/UserAvatarEditor.tsx(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
🔇 Additional comments (5)
apps/meteor/client/components/avatar/UserAvatarEditor/UserAvatarEditor.tsx (5)
2-2: LGTM!The
FieldErrorimport is correctly added and used for displaying validation errors.
41-41: LGTM!The error state is properly typed and initialized.
61-68: LGTM!The validation logic correctly checks the URL before setting the avatar and provides appropriate error feedback. This addresses the reviewer's previous feedback.
77-83: LGTM!Clearing the error when the user modifies the input provides good UX, allowing them to correct their mistakes.
119-119: LGTM!The disabled logic correctly prevents submission when there's no URL or when a validation error is present.
e64161a to
0b3b46c
Compare
Inline errors on profile page, more information in Figma https://www.figma.com/design/eCqiUBG2zmN7C0HrFxc7ji/Inline-errors-for-web-forms?node-id=50-2235&t=Qc1iBq6EYwRBeec6-0
URL for avatar: Invalid URL
Name: Name required
Username: Username required
Email: Email required
WCAG (3.3.1)
Summary by CodeRabbit
WA-54