fix(web): render custom OAuth provider icons - #5720
Conversation
WalkthroughA new internal ChangesCustom OAuth Provider Icon Display
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@web/default/src/features/auth/components/oauth-providers.tsx`:
- Around line 54-57: The OAuth provider icon in the icon-rendering component is
being announced redundantly by assistive tech because the provider name is
already conveyed by the button label. Update the <img> in oauth-providers.tsx
(the icon used by the OAuth provider button) to be decorative by using an empty
alt text and hiding it from screen readers, while keeping the existing provider
label in the button component unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d6aa9aea-cfc0-4d47-9d97-0fb96992b6df
📒 Files selected for processing (1)
web/default/src/features/auth/components/oauth-providers.tsx
| <img | ||
| src={props.icon} | ||
| alt={props.name} | ||
| className='h-4 w-4 rounded-sm object-contain' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the icon decorative for assistive tech.
On Line 56, alt={props.name} duplicates the provider name already announced by the button label (Line 149). Use an empty alt and hide the icon from screen readers.
Suggested patch
- alt={props.name}
+ alt=''
+ aria-hidden='true'As per coding guidelines, “Use ARIA attributes (aria-label, aria-expanded, aria-hidden) as needed … provide text equivalents for important information.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <img | |
| src={props.icon} | |
| alt={props.name} | |
| className='h-4 w-4 rounded-sm object-contain' | |
| <img | |
| src={props.icon} | |
| alt='' | |
| aria-hidden='true' | |
| className='h-4 w-4 rounded-sm object-contain' |
🤖 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 `@web/default/src/features/auth/components/oauth-providers.tsx` around lines 54
- 57, The OAuth provider icon in the icon-rendering component is being announced
redundantly by assistive tech because the provider name is already conveyed by
the button label. Update the <img> in oauth-providers.tsx (the icon used by the
OAuth provider button) to be decorative by using an empty alt text and hiding it
from screen readers, while keeping the existing provider label in the button
component unchanged.
Source: Coding guidelines
51fdfc5 to
2b6f1df
Compare
1 participant
Summary
Render configured custom OAuth provider icons in the new auth UI.
Motivation
Custom OAuth provider metadata already includes an
iconfield, but the new UI did not pass it into the provider button, so custom providers were rendered without their configured icons.Refs #5639
Changes
CustomProviderIconcomponent for custom OAuth provider buttons.provider.iconas a safe<img>React node when present.onError.Testing
corepack pnpm --dir web/default exec tsc --noEmit --project tsconfig.app.json --pretty falsecorepack pnpm --dir web/default exec oxlint -c .oxlintrc.json src/features/auth/components/oauth-providers.tsxcorepack pnpm --dir web/default exec oxfmt --check src/features/auth/components/oauth-providers.tsxcorepack pnpm --dir web/default run typecheckcorepack pnpm --dir web/default run buildgit diff --checkNote: the full project lint has pre-existing unrelated warnings/errors; the modified file passes targeted lint cleanly.
Summary by CodeRabbit