refactor(webapp): harden GitLab icon factory with forwardRef, a11y, and tests - #940
Conversation
📝 WalkthroughWalkthroughRemoved deprecated GithubBadge re-export and refactored GitLab merge request icon components to forward refs, expand SVG prop support via Omit pattern, and conditionally apply accessibility attributes (aria-hidden/role) based on label presence. Added comprehensive test coverage and accessibility documentation story. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Pull request overview
Refactors the GitLab icon factory in the webapp provider layer to align with the @primer/octicons-react component surface (ref forwarding, SVG prop passthrough, conditional a11y), and adds targeted rendering tests plus a Storybook a11y demo; also removes a dead deprecated re-export.
Changes:
- Refactor
createGitLabIconto useforwardRef, spread native SVG props, and apply conditionalaria-hidden/rolebehavior. - Add a new Vitest + Testing Library test suite covering structure, sizing, a11y behavior, prop forwarding, and icon variant differences.
- Add a Storybook “Accessibility” story demonstrating decorative vs labelled icon usage; delete unused
GithubBadgeshim.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
webapp/src/lib/provider/gitlab-icons.tsx |
Harden GitLab icon factory with ref forwarding, SVG prop passthrough, and conditional accessibility attributes. |
webapp/src/lib/provider/__tests__/gitlab-icons.test.tsx |
Adds comprehensive render/a11y/prop-forwarding tests for the GitLab icons. |
webapp/src/lib/provider/provider-icons.stories.tsx |
Adds an Accessibility-focused Storybook example for GitLab icons. |
webapp/src/components/shared/GithubBadge.tsx |
Removes deprecated, unused re-export shim. |
…nd tests - Refactor `createGitLabIcon` to use `forwardRef`, rest-prop spreading, and conditional `aria-hidden` / `role="img"` based on labelling props, mirroring the `@primer/octicons-react` API surface. - Export `GitLabIconProps` extending `SVGProps<SVGSVGElement>` for type safety. - Add Accessibility story demonstrating decorative vs labelled icon usage. - Delete dead `GithubBadge.tsx` deprecated shim (zero imports). Closes #724 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
80da70d to
9c6ba83
Compare
📚 Documentation Preview
|
|
🎉 This PR is included in version 0.52.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Description
Hardens the
createGitLabIconfactory to match the@primer/octicons-reactAPI surface and cleans up dead code. This completes the quality bar for the GitLab visual identity work from the provider abstraction layer.Changes
gitlab-icons.tsx— Icon factory refactoredforwardRef<SVGSVGElement>for DOM ref access...rest) for native SVG attributes (data-*,style,aria-*)aria-hidden="true"for decorative icons,role="img"+aria-labelfor labelled iconsGitLabIconPropstype extendingOmit<SVGProps<SVGSVGElement>, "children">focusable="false"always set (prevents IE/Edge focus ring on SVG)provider-icons.stories.tsx— New Accessibility storyGithubBadge.tsx— DeletedWhat was NOT changed (deliberate)
IconComponenttype stays narrow ({size?, className?}) — correct minimal interface for the dispatch layer<title>element —role="img"+aria-labelis sufficient per WAI-ARIA;<title>has inconsistent screen reader supportonemphasis/oninversetoken naming — currently unused, separate cleanupHow to Test
cd webapp && npm run storybook→ "Provider/Icons" → verify all 6 stories render correctly, including new "Accessibility" storyconst ref = createRef(); <GitLabMergeRequestIcon ref={ref} /><GitLabMergeRequestIcon aria-label="Open MR" />renders withrole="img", noaria-hiddenCloses #724