-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WEB-2912]fix: modified link behaviour to improve accessibility #6282
Conversation
WalkthroughThe pull request introduces modifications to two components: Changes
Sequence DiagramsequenceDiagram
participant User
participant LinkComponent
participant Clipboard
participant Toast
User->>LinkComponent: Click Copy Icon
LinkComponent->>Clipboard: Copy Link
Clipboard-->>LinkComponent: Link Copied
LinkComponent->>Toast: Show Confirmation
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
web/core/components/modules/links/list-item.tsx (3)
48-50
: Ensure accessible link text.
While the link is opened in a new tab, consider providing a clear label or anaria-label
to announce this for screen readers, e.g.,"Open link in a new tab"
. This ensures better accessibility compliance.
68-70
: Provide user feedback for copying when text differs from URL.
The code copies either the title or the URL, but the toast message always says “URL.” Consider tailoring the toast message to clarify what was actually copied (either the title or the URL), to avoid user confusion.
72-73
: Addaria-label
ortitle
for the copy icon.
Assistive technologies may need descriptive text about this icon’s action (e.g., “Copy link to clipboard”).web/core/components/issues/issue-detail/links/link-item.tsx (2)
51-54
: Adopt anaria-label
for the anchor.
Since the link opens in a new tab, consider adding something likearia-label="Open link in a new tab"
or a visually hidden indicator to improve screen reader clarity.Also applies to: 58-58
65-73
: Ensure consistent user experience across devices.
The copy icon triggers a toast notification, which is great feedback. Consider consistency with the label shown in the notification text, and also ensure it is easily dismissible on mobile devices if it covers interactive elements.Also applies to: 76-77
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/core/components/issues/issue-detail/links/link-item.tsx
(2 hunks)web/core/components/modules/links/list-item.tsx
(3 hunks)
🔇 Additional comments (2)
web/core/components/modules/links/list-item.tsx (1)
2-2
: Good introduction of the Copy icon.
Importing the Copy
icon from lucide-react
aligns with the new requirement to copy URLs more easily.
web/core/components/issues/issue-detail/links/link-item.tsx (1)
5-5
: New icon usage is consistent with the updated design.
Replacing ExternalLink
with Copy
matches the plan to emphasize link copying.
Description
Updated the Link component to allow clicking anywhere on the link to open it in a new tab. Replaced the external link icon with a copy link icon for improved functionality.
Type of Change
References
WEB-2912
Summary by CodeRabbit
New Features
Copy
icon for copying links in theIssueLinkItem
andModulesLinksListItem
components.span
element, improving accessibility and usability.Bug Fixes