Skip to content

feat(nav): open starred issue views in a new tab on modifier-click#117879

Merged
cvxluo merged 1 commit into
masterfrom
cvxluo/open-starred-issue-views-in-a-new-tab-on-modifier-
Jun 22, 2026
Merged

feat(nav): open starred issue views in a new tab on modifier-click#117879
cvxluo merged 1 commit into
masterfrom
cvxluo/open-starred-issue-views-in-a-new-tab-on-modifier-

Conversation

@cvxluo

@cvxluo cvxluo commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Support ctrl/cmd click to open a starred view in a new tab.

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jun 16, 2026
@cvxluo cvxluo force-pushed the cvxluo/open-starred-issue-views-in-a-new-tab-on-modifier- branch from db6ce43 to 0415a03 Compare June 16, 2026 22:50
// The browser fires a click after a drag's pointerup that would navigate the
// dragged link. react-router's navigation can't be cancelled from React's
// synthetic handlers here, so swallow that click natively in the capture phase.
function suppressNextClick() {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there was a consistent bug where dragging a starred view to a new location, then back, caused a reload. i tried a number of approaches here to prevent it and this was the only one that worked, but i couldn't figure out a way to make this less hacky

@cvxluo cvxluo changed the title feat(nav): Open starred issue views in a new tab on modifier-click feat(nav): open starred issue views in a new tab on modifier-click Jun 22, 2026
@cvxluo cvxluo marked this pull request as ready for review June 22, 2026 16:57
@cvxluo cvxluo requested a review from a team as a code owner June 22, 2026 16:57
@cvxluo cvxluo requested a review from a team June 22, 2026 16:57
Comment thread static/app/views/navigation/secondary/components.tsx Outdated
Comment thread static/app/views/navigation/secondary/components.tsx
@scttcper scttcper requested a review from malwilley June 22, 2026 17:17
@scttcper

Copy link
Copy Markdown
Member

Codex AI comment:

I think we can avoid the global click suppression by changing the structure slightly.

The hack exists because the drag handle is nested inside a real <Link>. After dnd-kit activates a drag, it stops propagation of the synthesized click, but it does not call preventDefault(), so the anchor default navigation can still happen. That forces us to install a native document capture listener after onDragEnd just to cancel the next click.

Instead, we could render the link and drag handle as siblings inside the sortable <li>:

  • keep the main nav item as a real <Link> so cmd/ctrl/middle-click works naturally
  • render the dnd-kit activator handle outside the link
  • absolutely position the handle over the icon slot on hover/focus
  • keep setNodeRef on the sortable list item, and setActivatorNodeRef/listeners on the handle

Rough shape:

<Container as="li" ref={setNodeRef} ...>
  <StyledReorderableLink to={to} ...>
    <IconSlot>{icon}</IconSlot>
    {children}
    {trailingItems}
  </StyledReorderableLink>
  <GrabHandle />
</Container>

That way a drag starts from a non-anchor element, so there is no anchor default navigation to suppress after pointerup. The global suppressNextClick() helper can go away, and the link remains a normal browser link for modifier clicks.

The tradeoff is a little more positioning/CSS around the handle and icon slot, but the event model becomes much cleaner: links navigate, handles drag.

@cvxluo cvxluo force-pushed the cvxluo/open-starred-issue-views-in-a-new-tab-on-modifier- branch from 0415a03 to f9f9f8f Compare June 22, 2026 22:11

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f9f9f8f. Configure here.

data-drag-icon
ref={setActivatorNodeRef}
style={{cursor: isDragging ? 'grabbing' : 'grab'}}
onClick={e => e.stopPropagation()}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle wrapper blocks icon clicks

Medium Severity

Moving GrabHandle outside the anchor leaves an absolutely positioned Flex over the project icon. Only the inner GrabHandleAnimation uses pointer-events: none; the wrapper still receives clicks, so taps on the icon no longer reach the Link and the starred view does not open.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f9f9f8f. Configure here.

transform ${p => p.theme.motion.smooth.moderate};
/* Center over the 18px project icon, which sits at the link's left padding. */
left: calc(${p => p.theme.space.lg} + 9px);
transition: opacity ${p => p.theme.motion.smooth.moderate};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle offset without positioning

Low Severity

GrabHandleAnimation sets left: calc(...) and transform: translate(-50%, -50%) to center over the project icon, but the styled div is not positioned (position: absolute with a vertical anchor). The left offset is ignored, so the drag activator may not sit on the icon the comment describes.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f9f9f8f. Configure here.

These reorderable nav items (the starred issue views) were fake div
links navigated in JS, so cmd/ctrl/middle-click could not open them in
a new tab the way real links do. Render them as anchor links instead.

Because the items are also drag-to-reorder, the grab handle is rendered
as a sibling of the link rather than a child of it, so a drag (which
always starts on the handle) can never produce a click whose event path
includes the anchor — and therefore never triggers navigation.

Co-authored-by: Claude <noreply@anthropic.com>
@cvxluo cvxluo force-pushed the cvxluo/open-starred-issue-views-in-a-new-tab-on-modifier- branch from f9f9f8f to 9e98be6 Compare June 22, 2026 22:40

@scttcper scttcper left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feels pretty good

@cvxluo cvxluo merged commit 04316e1 into master Jun 22, 2026
76 of 77 checks passed
@cvxluo cvxluo deleted the cvxluo/open-starred-issue-views-in-a-new-tab-on-modifier- branch June 22, 2026 23:10
sehr-m pushed a commit that referenced this pull request Jun 23, 2026
…117879)

Support ctrl/cmd click to open a starred view in a new tab.

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants