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
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ export const PERSISTED_KEY_REGISTRY: ReadonlyArray<
"src/renderer/routes/_authenticated/_dashboard/tasks/stores/tasks-filter-state.ts",
["tasks-filter-state"],
],
[
"src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsFilterStore/pullRequestsFilterStore.ts",
["pull-requests-filter-state"],
],
[
"src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx",
["active_organization_id"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip";
import { cn } from "@superset/ui/utils";
import { useMatchRoute, useNavigate } from "@tanstack/react-router";
import { useRef } from "react";
import { GoGitPullRequest } from "react-icons/go";
import { HiOutlineClipboardDocumentList } from "react-icons/hi2";
import { LuClock, LuLayers, LuPlus, LuSearch } from "react-icons/lu";
import {
Expand All @@ -28,6 +29,10 @@ import { useFolderFirstImport } from "renderer/routes/_authenticated/_dashboard/
import { NavigationControls } from "renderer/routes/_authenticated/_dashboard/components/NavigationControls";
import { SidebarToggle } from "renderer/routes/_authenticated/_dashboard/components/SidebarToggle";
import { useFailedAutomations } from "renderer/routes/_authenticated/_dashboard/hooks/useFailedAutomations";
import {
pullRequestsSearchFromFilters,
usePullRequestsFilterStore,
} from "renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsFilterStore";
import {
tasksSearchFromFilters,
useTasksFilterStore,
Expand Down Expand Up @@ -100,6 +105,10 @@ export function DashboardSidebarHeader({
fuzzy: true,
});
const isTasksOpen = !!matchRoute({ to: "/tasks", fuzzy: true });
const isPullRequestsOpen = !!matchRoute({
to: "/pull-requests",
fuzzy: true,
});
const isAutomationsOpen = !!matchRoute({ to: "/automations", fuzzy: true });
const { myFailedCount } = useFailedAutomations();

Expand All @@ -110,7 +119,13 @@ export function DashboardSidebarHeader({
typeTab: lastTypeTab,
projectFilter: lastProjectFilter,
linearProjectFilter: lastLinearProjectFilter,
includeClosedIssues: lastIncludeClosedIssues,
} = useTasksFilterStore();
const {
search: lastPullRequestsSearch,
projectFilter: lastPullRequestsProjectFilter,
includeClosed: lastPullRequestsIncludeClosed,
} = usePullRequestsFilterStore();

const handleWorkspacesClick = () => {
navigate({ to: "/v2-workspaces" });
Expand All @@ -131,6 +146,20 @@ export function DashboardSidebarHeader({
typeTab: lastTypeTab,
projectFilter: lastProjectFilter,
linearProjectFilter: lastLinearProjectFilter,
includeClosedIssues: lastIncludeClosedIssues,
}),
});
});
};

const handlePullRequestsClick = () => {

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.

P2: Both DashboardSidebarHeader and WorkspaceSidebarHeader now contain near-identical handlePullRequestsClick functions, identical usePullRequestsFilterStore subscriptions, and identical includeClosedIssues additions to handleTasksClick. This duplicated business logic means any future change to PR navigation behavior (adding a filter param, changing the gate feature, or altering the navigate call) must be applied in two separate files. The pattern matches prior team feedback about extracting shared navigation logic into a hook or helper to keep behavior consistent and reduce cognitive load when changes are made.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarHeader/DashboardSidebarHeader.tsx, line 156:

<comment>Both `DashboardSidebarHeader` and `WorkspaceSidebarHeader` now contain near-identical `handlePullRequestsClick` functions, identical `usePullRequestsFilterStore` subscriptions, and identical `includeClosedIssues` additions to `handleTasksClick`. This duplicated business logic means any future change to PR navigation behavior (adding a filter param, changing the gate feature, or altering the navigate call) must be applied in two separate files. The pattern matches prior team feedback about extracting shared navigation logic into a hook or helper to keep behavior consistent and reduce cognitive load when changes are made.</comment>

<file context>
@@ -132,6 +147,20 @@ export function DashboardSidebarHeader({
+		});
+	};
+
+	const handlePullRequestsClick = () => {
+		gateFeature(GATED_FEATURES.TASKS, () => {
+			navigate({
</file context>

gateFeature(GATED_FEATURES.TASKS, () => {
navigate({
to: "/pull-requests",
search: pullRequestsSearchFromFilters({
search: lastPullRequestsSearch,
projectFilter: lastPullRequestsProjectFilter,
includeClosed: lastPullRequestsIncludeClosed,
}),
});
});
Expand Down Expand Up @@ -246,6 +275,8 @@ export function DashboardSidebarHeader({
<button
type="button"
onClick={handleTasksClick}
aria-label="Tasks"
aria-current={isTasksOpen ? "page" : undefined}
className={cn(
"flex size-7 items-center justify-center rounded-md transition-colors",
isTasksOpen
Expand All @@ -256,7 +287,27 @@ export function DashboardSidebarHeader({
<HiOutlineClipboardDocumentList className="size-3.5" />
</button>
</TooltipTrigger>
<TooltipContent side="right">Tasks & PRs</TooltipContent>
<TooltipContent side="right">Tasks</TooltipContent>
</Tooltip>

<Tooltip delayDuration={300}>
<TooltipTrigger asChild>
<button
type="button"
onClick={handlePullRequestsClick}
aria-label="Pull requests"
aria-current={isPullRequestsOpen ? "page" : undefined}
className={cn(
"flex size-7 items-center justify-center rounded-md transition-colors",
isPullRequestsOpen
? "bg-fill-selected text-muted-foreground"
: "text-muted-foreground hover:bg-fill-hover",
)}
>
<GoGitPullRequest className="size-3.5" />
</button>
</TooltipTrigger>
<TooltipContent side="right">Pull requests</TooltipContent>
</Tooltip>

<DropdownMenu>
Expand Down Expand Up @@ -410,6 +461,8 @@ export function DashboardSidebarHeader({
<button
type="button"
onClick={handleTasksClick}
aria-label="Tasks"
aria-current={isTasksOpen ? "page" : undefined}
className={cn(
"flex w-full items-center gap-2 rounded-md px-2 py-1 text-[13px] font-medium transition-colors",
isTasksOpen
Expand All @@ -418,7 +471,23 @@ export function DashboardSidebarHeader({
)}
>
<HiOutlineClipboardDocumentList className="size-3.5 shrink-0 text-muted-foreground" />
<span className="flex-1 text-left">Tasks & PRs</span>
<span className="flex-1 text-left">Tasks</span>
</button>

<button
type="button"
onClick={handlePullRequestsClick}
aria-label="Pull requests"
aria-current={isPullRequestsOpen ? "page" : undefined}
className={cn(
"flex w-full items-center gap-2 rounded-md px-2 py-1 text-[13px] font-medium transition-colors",
isPullRequestsOpen
? "bg-fill-selected text-foreground"
: "text-muted-foreground hover:bg-fill-hover hover:text-foreground",
)}
>
<GoGitPullRequest className="size-3.5 shrink-0 text-muted-foreground" />
<span className="flex-1 text-left">Pull requests</span>
</button>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { ToggleGroup, ToggleGroupItem } from "@superset/ui/toggle-group";

interface OpenClosedFilterProps {
includeClosed: boolean;
onChange: (includeClosed: boolean) => void;
}

export function OpenClosedFilter({
includeClosed,
onChange,
}: OpenClosedFilterProps) {
return (
<div className="flex items-center gap-2">
<span className="text-xs text-muted-foreground">State</span>
<ToggleGroup
type="single"
value={includeClosed ? "all" : "open"}
onValueChange={(value) => {
if (value) onChange(value === "all");
}}
variant="outline"
size="sm"
aria-label="Filter by state"
className="h-8 rounded-md border-0 bg-muted/50 p-0.5"
>
<ToggleGroupItem
value="open"
aria-label="Show open items"
className="h-7 border-0 px-2 text-xs data-[state=on]:bg-background data-[state=on]:text-foreground"
>
Open
</ToggleGroupItem>
<ToggleGroupItem
value="all"
aria-label="Show items in all states"
className="h-7 border-0 px-2 text-xs data-[state=on]:bg-background data-[state=on]:text-foreground"
>
All
</ToggleGroupItem>
</ToggleGroup>
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { OpenClosedFilter } from "./OpenClosedFilter";
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export function ProjectFilter({ value, onChange }: ProjectFilterProps) {
const [search, setSearch] = useState("");

// Projects are fully local — identity comes from the host fan-out.
const { projects: hostProjects } = useHostProjects();
const { projects: hostProjects, isReady } = useHostProjects();
const projects = useMemo(
() =>
hostProjects.map((project) => ({
Expand All @@ -51,6 +51,7 @@ export function ProjectFilter({ value, onChange }: ProjectFilterProps) {
setOpen(false);
setSearch("");
};
const fallbackLabel = isReady ? "Select project" : "Loading projects";

return (
<Popover
Expand All @@ -64,8 +65,8 @@ export function ProjectFilter({ value, onChange }: ProjectFilterProps) {
<Button
variant="ghost"
size="sm"
title={selected ? selected.name : "Project"}
aria-label={selected ? selected.name : "Project"}
title={selected ? selected.name : fallbackLabel}
aria-label={selected ? `Project: ${selected.name}` : fallbackLabel}
className="h-8 gap-1.5 px-2 text-muted-foreground hover:text-foreground"
>
{selected ? (
Expand All @@ -77,8 +78,8 @@ export function ProjectFilter({ value, onChange }: ProjectFilterProps) {
) : (
<HiOutlineFolder className="size-4" />
)}
<span className="text-sm hidden @4xl:inline">
{selected ? selected.name : "Project"}
<span className="hidden max-w-32 truncate text-sm @4xl:inline @6xl:max-w-48">
{selected ? selected.name : fallbackLabel}
</span>
<HiChevronDown className="size-3" />
</Button>
Expand All @@ -91,8 +92,14 @@ export function ProjectFilter({ value, onChange }: ProjectFilterProps) {
onValueChange={setSearch}
/>
<CommandList className="max-h-80">
{filtered.length === 0 && search && (
<CommandEmpty>No projects found.</CommandEmpty>
{filtered.length === 0 && (
<CommandEmpty>
{isReady
? search
? "No projects found."
: "No projects available."
: "Loading projects…"}
</CommandEmpty>
)}
{filtered.length > 0 && (
<CommandGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { Button } from "@superset/ui/button";
import type { ReactNode } from "react";
import { HiArrowLeft } from "react-icons/hi2";
import { LuExternalLink, LuPlus } from "react-icons/lu";

interface WorkItemDetailHeaderProps {
itemNumber: number | null;
icon: ReactNode;
backLabel: string;
externalLabel: string;
url: string | null;
onBack: () => void;
onAddToWorkspace: (() => void) | null;
}

export function WorkItemDetailHeader({
itemNumber,
icon,
backLabel,
externalLabel,
url,
onBack,
onAddToWorkspace,
}: WorkItemDetailHeaderProps) {
return (
<div className="@container flex shrink-0 items-center gap-2 border-b border-border px-4 py-3 @md:gap-3 @md:px-6 @md:py-4">
<Button
variant="ghost"
size="icon"
className="size-8 shrink-0"
onClick={onBack}
aria-label={backLabel}
>
<HiArrowLeft className="size-4" />
</Button>
{icon}
<span className="min-w-0 truncate font-mono text-sm tabular-nums text-muted-foreground">
{itemNumber === null ? "#—" : `#${itemNumber}`}
</span>
{/* Window-drag leaf standing in for the hidden TopBar. */}
<div className="drag -my-3 min-w-0 flex-1 self-stretch @md:-my-4" />
<div className="ml-auto flex shrink-0 items-center gap-1">
{url && (
<Button variant="ghost" size="icon" className="size-8" asChild>
<a
href={url}
target="_blank"
rel="noopener noreferrer"
aria-label={externalLabel}
title={externalLabel}
>
<LuExternalLink className="size-4" />
</a>
</Button>
)}
{onAddToWorkspace && (
<Button
variant="outline"
size="sm"
className="h-8 gap-1.5 px-2 @md:px-3"
onClick={onAddToWorkspace}
aria-label="Add to workspace"
title="Add to workspace"
>
<LuPlus className="size-4" />
<span className="hidden @md:inline">Add to workspace</span>
</Button>
)}
</div>
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { WorkItemDetailHeader } from "./WorkItemDetailHeader";
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { Button } from "@superset/ui/button";
import { cn } from "@superset/ui/utils";
import { LuRefreshCw } from "react-icons/lu";

interface WorkItemDetailStateProps {
message: string;
isLoading?: boolean;
isError?: boolean;
onRetry?: () => void;
}

export function WorkItemDetailState({
message,
isLoading = false,
isError = false,
onRetry,
}: WorkItemDetailStateProps) {
return (
<div

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.

P3: When isError is true, the role="alert" (implicit aria-live="assertive") conflicts with the static aria-live="polite". Explicit aria-live takes precedence over implicit values from the role, so error announcements may be treated as polite instead of assertive, reducing responsiveness for time-sensitive errors. Other role="alert" usages in the codebase omit aria-live entirely. Consider removing aria-live from the container or making it conditional so it's omitted when role="alert" is active.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/WorkItemDetailState/WorkItemDetailState.tsx, line 19:

<comment>When `isError` is true, the `role="alert"` (implicit `aria-live="assertive"`) conflicts with the static `aria-live="polite"`. Explicit `aria-live` takes precedence over implicit values from the role, so error announcements may be treated as polite instead of assertive, reducing responsiveness for time-sensitive errors. Other `role="alert"` usages in the codebase omit `aria-live` entirely. Consider removing `aria-live` from the container or making it conditional so it's omitted when `role="alert"` is active.</comment>

<file context>
@@ -0,0 +1,39 @@
+	onRetry,
+}: WorkItemDetailStateProps) {
+	return (
+		<div
+			className={cn(
+				"flex flex-1 flex-col items-center justify-center gap-3 px-6 py-10 text-center text-sm",
</file context>

className={cn(
"flex flex-1 flex-col items-center justify-center gap-3 px-6 py-10 text-center text-sm",
isError ? "text-destructive" : "text-muted-foreground",
(isError || onRetry) && "cursor-text select-text",
)}
role={isError ? "alert" : "status"}
aria-live="polite"
>
{isLoading && (
<LuRefreshCw className="size-4 animate-spin motion-reduce:animate-none" />
)}
<span className="max-w-prose text-wrap-pretty">{message}</span>
{onRetry && (
<Button variant="outline" size="sm" onClick={onRetry}>
Try again
</Button>
)}
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { WorkItemDetailState } from "./WorkItemDetailState";
Loading
Loading