[WEB-2613] chore: open parent and sibling issue in new tab from peek-overview/ issue detail page. #5819
Conversation
…overview/ issue detail page.
WalkthroughThe changes involve updates to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
web/core/components/issues/issue-detail/parent/sibling-item.tsx (1)
36-36: Approve with suggestions: Add security attribute and consider UX improvementThe addition of
target="_blank"successfully implements the desired functionality of opening parent and sibling issues in a new tab, aligning with the PR objectives. However, I have two suggestions for improvement:
For security reasons, when using
target="_blank", it's recommended to also includerel="noopener noreferrer". This prevents potential vulnerabilities related to tabnabbing.Consider adding a visual indicator (e.g., an icon) to show users that the link will open in a new tab, improving the user experience.
Here's a suggested improvement:
<Link href={`/${workspaceSlug}/projects/${issueDetail?.project_id as string}/issues/${issueDetail.id}`} - target="_blank" + target="_blank" rel="noopener noreferrer" className="flex items-center gap-2 py-0.5" > {issueDetail.project_id && projectDetails?.identifier && ( <> <IssueIdentifier projectId={issueDetail.project_id} issueTypeId={issueDetail.type_id} projectIdentifier={projectDetails?.identifier} issueSequenceId={issueDetail.sequence_id} textContainerClassName="text-xs" /> + <span className="text-xs">↗️</span> {/* or use an appropriate icon from your UI library */} </> )} </Link>This change adds the security attribute and a simple visual indicator for the new tab behavior.
web/core/components/issues/issue-detail/parent/root.tsx (2)
45-45: Approved with a security suggestion.The addition of
target="_blank"successfully implements the desired functionality of opening parent issues in a new tab. This aligns with the PR objectives and can improve user experience.However, for security reasons, it's recommended to add
rel="noopener noreferrer"when usingtarget="_blank". This prevents potential vulnerabilities where the new page could access the original page's JavaScript context.Consider updating the Link component as follows:
- <Link href={`/${workspaceSlug}/projects/${parentIssue?.project_id}/issues/${parentIssue.id}`} target="_blank"> + <Link href={`/${workspaceSlug}/projects/${parentIssue?.project_id}/issues/${parentIssue.id}`} target="_blank" rel="noopener noreferrer">This change maintains the desired functionality while enhancing security.
Action Required: Update
<Link>Components to Includetarget="_blank"The following
<Link>components are missing thetarget="_blank"attribute. Updating them will ensure consistency and enhance security by properly handling external links.
space/helpers/authentication.helper.tsx
<Link className="underline underline-offset-4 font-medium hover:font-bold transition-all" href={`/admin`}> Sign In </Link>space/core/lib/instance-provider.tsx
<Link href={`${SPACE_BASE_PATH}/`} className="h-[30px] w-[133px]"> <Image src={logo} alt="Plane logo" /> </Link>space/core/components/views/auth.tsx
<Link href={`${SPACE_BASE_PATH}/`} className="h-[30px] w-[133px]"> <Image src={logo} alt="Plane logo" /> </Link>web/app/sign-up/page.tsx
<Link href={`/`} className="h-[30px] w-[133px]"> <Image src={logo} alt="Plane logo" /> </Link>web/app/profile/sidebar.tsx
<Link href={`/${redirectWorkspaceSlug}`} onClick={handleItemClick}> <div className={`flex flex-shrink-0 items-center gap-2 truncate px-4 pt-4 ${sidebarCollapsed ? "justify-center" : ""}`}> {/* ... */} </div> </Link>web/app/page.tsx
<Link href={`/`} className="h-[30px] w-[133px]"> <Image src={logo} alt="Plane logo" /> </Link>web/app/not-found.tsx
<Link href="/"> <span className="flex justify-center"> <Button variant="neutral-primary" size="md"> Go to Home </Button> </span> </Link>web/app/invitations/page.tsx
<Link href={`/${redirectWorkspaceSlug}`}> <span> <Button variant="neutral-primary" size="md"> Go Home </Button> </span> </Link>web/core/components/workspace/sidebar/user-menu.tsx
<Link key={link.key} href={getHref(link)} onClick={() => handleLinkClick(link.key)}> <SidebarNavItem className={`${sidebarCollapsed ? "p-0 size-8 aspect-square justify-center mx-auto" : ""}`} isActive={link.highlight(pathname, `/${workspaceSlug}`, { userId: currentUser?.id })} /> </Link>web/core/layouts/auth-layout/workspace-wrapper.tsx
<Link href="/"> <Button>Go Home</Button> </Link>Please review and update these components to include
target="_blank"where appropriate to maintain consistency and improve user experience.🔗 Analysis chain
Line range hint
1-78: Overall assessment: Functionality implemented with a minor security enhancement suggested.The change successfully implements the PR objective of opening parent issues in a new tab. The modification is minimal and focused, which is good for maintainability.
To ensure consistency across the codebase, it would be beneficial to check if similar changes are needed in other components, particularly in the
IssueParentSiblingscomponent mentioned in this file. Run the following script to find other potential locations where this change might be applicable:This will help identify other Link components that might benefit from the same update for consistency across the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find other potential locations where target="_blank" might be needed for consistency. # Test: Search for Link components that might need the target="_blank" attribute. rg --type typescript --type typescriptreact 'import.*Link.*from.*next/link' -A 10 | rg '<Link' -A 3Length of output: 136
Script:
#!/bin/bash # Description: Find other potential locations where target="_blank" might be needed for consistency. # Test: Search for Link components that might need the target="_blank" attribute. rg --type ts --type tsx 'import.*Link.*from.*next/link' -A 10 | rg '<Link' -A 3Length of output: 109
Script:
#!/bin/bash # Description: Find other potential locations where target="_blank" might be needed for consistency. # Test: Search for Link components that might need the target="_blank" attribute in .ts and .tsx files. rg 'import.*Link.*from.*next/link' --glob '*.ts' --glob '*.tsx' -A 10 | rg '<Link' -A 3Length of output: 347
Script:
#!/bin/bash # Description: Identify <Link> components missing the target="_blank" attribute for consistency. # Search for Link components in .ts and .tsx files # and list those that do not have target="_blank" specified. rg '<Link\b(?![^>]*\btarget=["'\'']_blank["'\''])[^>]*>' --glob '*.ts' --glob '*.tsx' --context 3Length of output: 355
Script:
#!/bin/bash # Description: Identify <Link> components missing the target="_blank" attribute for consistency. # Search for Link components in .ts and .tsx files # and list those that do not have target="_blank" specified. rg --pcre2 '<Link\b(?![^>]*\btarget=["'\'']_blank["'\''])[^>]*>' --glob '*.ts' --glob '*.tsx' --context 3Length of output: 59426
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- web/core/components/issues/issue-detail/parent/root.tsx (1 hunks)
- web/core/components/issues/issue-detail/parent/sibling-item.tsx (1 hunks)
🧰 Additional context used
Issue link: WEB-2613
Summary by CodeRabbit
New Features
Bug Fixes
Documentation