Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions apps/code/src/renderer/components/ui/NestedButton.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { describe, expect, it, vi } from "vitest";
import { NestedButton } from "./NestedButton";

describe("NestedButton", () => {
it("renders an accessible button", () => {
render(<NestedButton onActivate={() => {}}>x</NestedButton>);
expect(screen.getByRole("button")).toBeTruthy();
});

it("calls onActivate on click without bubbling to the parent", async () => {
const onActivate = vi.fn();
const onParentClick = vi.fn();
render(
// biome-ignore lint/a11y/useKeyWithClickEvents: test-only wrapper
// biome-ignore lint/a11y/noStaticElementInteractions: test-only wrapper
<div onClick={onParentClick}>
<NestedButton onActivate={onActivate}>x</NestedButton>
</div>,
);
await userEvent.click(screen.getByRole("button"));
expect(onActivate).toHaveBeenCalledTimes(1);
expect(onParentClick).not.toHaveBeenCalled();
});

it.each(["{Enter}", " "])("activates with the %s key", async (key) => {
const onActivate = vi.fn();
render(<NestedButton onActivate={onActivate}>x</NestedButton>);
screen.getByRole("button").focus();
await userEvent.keyboard(key);
expect(onActivate).toHaveBeenCalledTimes(1);
});
});
50 changes: 50 additions & 0 deletions apps/code/src/renderer/components/ui/NestedButton.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import type React from "react";
import { forwardRef } from "react";

interface NestedButtonProps extends React.HTMLAttributes<HTMLSpanElement> {
onActivate: () => void;
}

/**
* A button nested inside another button. Rows like SidebarItem render as a real
* `<button>`, and HTML forbids nesting a `<button>` inside one, so this is a
* `<span role="button">` with full keyboard support instead. Click, double
* click and Enter/Space stop propagation so the parent button does not also
* fire. Forwards its ref and composes any injected handlers so it works as a
* Radix `asChild` trigger (e.g. wrapped in a Tooltip).
*/
export const NestedButton = forwardRef<HTMLSpanElement, NestedButtonProps>(
function NestedButton(
{ onActivate, onClick, onDoubleClick, onKeyDown, children, ...rest },
ref,
) {
return (
// biome-ignore lint/a11y/useSemanticElements: nested clickable inside a parent <button> (e.g. SidebarItem)
<span
{...rest}
ref={ref}
role="button"
tabIndex={0}
onClick={(e) => {
e.stopPropagation();
onClick?.(e);
onActivate();
}}
onDoubleClick={(e) => {
e.stopPropagation();
onDoubleClick?.(e);
}}
onKeyDown={(e) => {
onKeyDown?.(e);
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
e.stopPropagation();
onActivate();
}
}}
>
{children}
</span>
);
},
);
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ function TaskRow({
slackThreadUrl={task.slackThreadUrl}
prState={prState}
hasDiff={hasDiff}
prUrl={task.cloudPrUrl}
timestamp={timestamp}
onClick={onClick}
onDoubleClick={onDoubleClick}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { DotsCircleSpinner } from "@components/DotsCircleSpinner";
import { NestedButton } from "@components/ui/NestedButton";
import { Tooltip } from "@components/ui/Tooltip";
import type { SidebarPrState } from "@features/sidebar/hooks/useTaskPrStatus";
import type { WorkspaceMode } from "@main/services/workspace/schemas";
Expand All @@ -14,8 +15,8 @@ import {
PushPin,
SlackLogo,
} from "@phosphor-icons/react";
import { trpcClient } from "@renderer/trpc/client";
import { isTerminalStatus, type TaskRunStatus } from "@shared/types";
import { openUrlInBrowser } from "@utils/browser";

export const ICON_SIZE = 12;

Expand All @@ -39,14 +40,10 @@ function getOriginProductMeta(
return originProduct ? ORIGIN_PRODUCT_META[originProduct] : undefined;
}

// Renders the icon inside a span. When `link` is set the span becomes
// clickable and opens the originating thread externally. SidebarItem renders
// the row as a `<button>`, so a real `<a>` here would be invalid HTML — match
// the inline role="button" pattern used by TaskHoverToolbar.
//
// Returned as a plain React element (not a component) so the span is the
// direct child of Tooltip — Radix's `asChild` Slot needs a host element to
// attach hover handlers to.
// Renders the icon inside a span. When `link` is set the icon becomes a
// clickable NestedButton that opens the originating thread externally.
// SidebarItem renders the row as a `<button>`, so a real `<a>` or a nested
// `<button>` here would be invalid HTML.
function renderIconSpan({
icon,
link,
Expand All @@ -59,31 +56,16 @@ function renderIconSpan({
if (!link) {
return <span className="flex items-center justify-center">{icon}</span>;
}
const open = () => {
void trpcClient.os.openExternal.mutate({ url: link });
};
return (
// biome-ignore lint/a11y/useSemanticElements: nested clickable inside SidebarItem button
<span
role="button"
tabIndex={0}
<NestedButton
aria-label={ariaLabel}
className="flex cursor-pointer items-center justify-center rounded transition-opacity hover:opacity-70"
onClick={(e) => {
e.stopPropagation();
open();
}}
onDoubleClick={(e) => e.stopPropagation()}
onKeyDown={(e) => {
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
e.stopPropagation();
open();
}
onActivate={() => {
void openUrlInBrowser(link);
}}
>
{icon}
</span>
</NestedButton>
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,33 @@
import { NestedButton } from "@components/ui/NestedButton";
import { Tooltip } from "@components/ui/Tooltip";
import type { SidebarPrState } from "@features/sidebar/hooks/useTaskPrStatus";
import type { WorkspaceMode } from "@main/services/workspace/schemas";
import { Archive, PushPin } from "@phosphor-icons/react";
import { Archive, GitPullRequest, PushPin } from "@phosphor-icons/react";
import { parseGithubUrl } from "@posthog/git/utils";
import type { TaskRunStatus } from "@shared/types";
import { openUrlInBrowser } from "@utils/browser";
import { formatRelativeTimeShort } from "@utils/time";
import { useCallback, useEffect, useRef, useState } from "react";
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
import { SidebarItem } from "../SidebarItem";
import { TaskIcon } from "./TaskIcon";

function PrBadge({ url, number }: { url: string; number: number }) {
return (
<Tooltip content="Open pull request" side="top">
<NestedButton
aria-label={`Open pull request #${number}`}
className="flex h-4 shrink-0 cursor-pointer items-center gap-0.5 rounded bg-gray-3 px-1 text-[11px] text-gray-11 transition-colors hover:bg-gray-4 hover:text-gray-12"
onActivate={() => {
void openUrlInBrowser(url);
}}
>
<GitPullRequest size={10} weight="bold" />
{`#${number}`}
</NestedButton>
</Tooltip>
);
}

interface TaskItemProps {
depth?: number;
taskId: string;
Expand All @@ -27,6 +47,7 @@ interface TaskItemProps {
slackThreadUrl?: string;
prState?: SidebarPrState;
hasDiff?: boolean;
prUrl?: string | null;
timestamp?: number;
isEditing?: boolean;
onClick: (e: React.MouseEvent) => void;
Expand All @@ -53,50 +74,24 @@ function TaskHoverToolbar({
<span className="hidden shrink-0 items-center gap-0.5 group-hover:flex">
{onTogglePin && (
<Tooltip content={isPinned ? "Unpin task" : "Pin task"} side="top">
{/* biome-ignore lint/a11y/useSemanticElements: Cannot use button inside parent button (SidebarItem) */}
<span
role="button"
tabIndex={0}
<NestedButton
aria-label={isPinned ? "Unpin task" : "Pin task"}
className="flex h-5 w-5 cursor-pointer items-center justify-center rounded text-gray-10 transition-colors hover:bg-gray-4 hover:text-gray-12"
onClick={(e) => {
e.stopPropagation();
onTogglePin();
}}
onDoubleClick={(e) => e.stopPropagation()}
onKeyDown={(e) => {
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
e.stopPropagation();
onTogglePin();
}
}}
onActivate={onTogglePin}
>
<PushPin size={12} weight={isPinned ? "fill" : "regular"} />
</span>
</NestedButton>
</Tooltip>
)}
{onArchive && (
<Tooltip content="Archive task" side="top">
{/* biome-ignore lint/a11y/useSemanticElements: Cannot use button inside parent button (SidebarItem) */}
<span
role="button"
tabIndex={0}
<NestedButton
aria-label="Archive task"
className="flex h-5 w-5 cursor-pointer items-center justify-center rounded text-gray-10 transition-colors hover:bg-gray-4 hover:text-gray-12"
onClick={(e) => {
e.stopPropagation();
onArchive();
}}
onDoubleClick={(e) => e.stopPropagation()}
onKeyDown={(e) => {
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
e.stopPropagation();
onArchive();
}
}}
onActivate={onArchive}
>
<Archive size={12} />
</span>
</NestedButton>
</Tooltip>
)}
</span>
Expand All @@ -123,6 +118,7 @@ export function TaskItem({
slackThreadUrl,
prState,
hasDiff,
prUrl,
timestamp,
isEditing = false,
onClick,
Expand All @@ -149,11 +145,19 @@ export function TaskItem({
/>
);

const timestampNode = timestamp ? (
<span className="shrink-0 text-[11px] text-gray-11 group-hover:hidden">
{formatRelativeTimeShort(timestamp)}
</span>
) : null;
const prRef = useMemo(() => (prUrl ? parseGithubUrl(prUrl) : null), [prUrl]);
const prBadge =
prUrl && prRef?.kind === "pr" ? (
<PrBadge url={prUrl} number={prRef.number} />
) : null;

// The PR badge takes the timestamp's slot, so hide the timestamp when shown.
const timestampNode =
timestamp && !prBadge ? (
<span className="shrink-0 text-[11px] text-gray-11 group-hover:hidden">
{formatRelativeTimeShort(timestamp)}
</span>
) : null;

const toolbar =
!hideHoverActions && (onArchive || onTogglePin) ? (
Expand All @@ -165,8 +169,9 @@ export function TaskItem({
) : null;

const endContent =
timestampNode || toolbar ? (
prBadge || timestampNode || toolbar ? (
<>
{prBadge}
{timestampNode}
{toolbar}
</>
Expand Down
Loading