mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(panel): active tab/route highlight (pickTab + sidebar footer exact match) (#520)
* feat(panel): add pickTab helper for validated URL tab params * fix(panel): validate kanban view + KB tab params via pickTab The bare `as T || default` cast only guarded null — a typo/invalid value (?view=deev, ?tab=foo) passed through as an out-of-set TabValue, blanking the active tab highlight and the content pane. pickTab validates against the known set and falls back to the default on null/empty/invalid. * fix(panel): highlight active sidebar footer link (exact match) SidebarFooter had no isActive branch (SidebarNav does), so footer links never highlighted. Added with EXACT match (pathname === href) — not startsWith — so /settings does not also highlight on /settings/ai-providers. Main nav keeps startsWith (longer hrefs need prefix matching); the two intentionally differ. --------- Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -10,16 +10,18 @@ import {
|
|||||||
} from "@/components/kanban";
|
} from "@/components/kanban";
|
||||||
import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs";
|
import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs";
|
||||||
import { Skeleton } from "@/components/ui/skeleton";
|
import { Skeleton } from "@/components/ui/skeleton";
|
||||||
|
import { pickTab } from "@/lib/tabs";
|
||||||
import { Code, TestTube, GitPullRequest, ClipboardList } from "lucide-react";
|
import { Code, TestTube, GitPullRequest, ClipboardList } from "lucide-react";
|
||||||
|
|
||||||
type KanbanView = "dev" | "qa" | "pr-review" | "pm";
|
type KanbanView = "dev" | "qa" | "pr-review" | "pm";
|
||||||
|
const KANBAN_VIEWS = ["dev", "qa", "pr-review", "pm"] as const satisfies readonly KanbanView[];
|
||||||
|
|
||||||
function KanbanPageContent() {
|
function KanbanPageContent() {
|
||||||
const router = useRouter();
|
const router = useRouter();
|
||||||
const searchParams = useSearchParams();
|
const searchParams = useSearchParams();
|
||||||
|
|
||||||
// Read view from URL params, default to "dev"
|
// Read view from URL params; fall back to "dev" on null/empty/invalid.
|
||||||
const view = (searchParams.get("view") as KanbanView) || "dev";
|
const view: KanbanView = pickTab(searchParams.get("view"), KANBAN_VIEWS, "dev");
|
||||||
|
|
||||||
const handleViewChange = (newView: string) => {
|
const handleViewChange = (newView: string) => {
|
||||||
if (newView === "dev") {
|
if (newView === "dev") {
|
||||||
|
|||||||
@@ -49,6 +49,7 @@ import { Skeleton } from "@/components/ui/skeleton";
|
|||||||
import { toast } from "sonner";
|
import { toast } from "sonner";
|
||||||
import { formatDistanceToNow } from "date-fns";
|
import { formatDistanceToNow } from "date-fns";
|
||||||
import { getErrorMessage } from "@/lib/api/client";
|
import { getErrorMessage } from "@/lib/api/client";
|
||||||
|
import { pickTab } from "@/lib/tabs";
|
||||||
import { usePageRefresh } from "@/hooks";
|
import { usePageRefresh } from "@/hooks";
|
||||||
|
|
||||||
// Components
|
// Components
|
||||||
@@ -62,7 +63,8 @@ import { MentorChat } from "./mentor-chat";
|
|||||||
import { KBCategoryNav } from "./kb-category-nav";
|
import { KBCategoryNav } from "./kb-category-nav";
|
||||||
import { KBCategoryView } from "./kb-category-view";
|
import { KBCategoryView } from "./kb-category-view";
|
||||||
|
|
||||||
type TabValue = "search" | "ask" | "mentor" | "browse" | "admin";
|
const TAB_VALUES = ["search", "ask", "mentor", "browse", "admin"] as const;
|
||||||
|
type TabValue = (typeof TAB_VALUES)[number];
|
||||||
|
|
||||||
const INDEX_LABELS: Record<KBIndexType, string> = {
|
const INDEX_LABELS: Record<KBIndexType, string> = {
|
||||||
[KBIndexType.DOCUMENTATION]: "Documentation",
|
[KBIndexType.DOCUMENTATION]: "Documentation",
|
||||||
@@ -95,8 +97,8 @@ function KnowledgeBaseBrowserContent() {
|
|||||||
const router = useRouter();
|
const router = useRouter();
|
||||||
const searchParams = useSearchParams();
|
const searchParams = useSearchParams();
|
||||||
|
|
||||||
// Read state from URL params
|
// Read state from URL params; fall back to "search" on null/empty/invalid.
|
||||||
const activeTab = (searchParams.get("tab") as TabValue) || "search";
|
const activeTab: TabValue = pickTab(searchParams.get("tab"), TAB_VALUES, "search");
|
||||||
const searchQuery = searchParams.get("q") || "";
|
const searchQuery = searchParams.get("q") || "";
|
||||||
const filtersParam = searchParams.get("filters");
|
const filtersParam = searchParams.get("filters");
|
||||||
const searchFilters: KBIndexType[] = filtersParam
|
const searchFilters: KBIndexType[] = filtersParam
|
||||||
|
|||||||
@@ -0,0 +1,35 @@
|
|||||||
|
import { describe, expect, it, vi, beforeEach } from "vitest";
|
||||||
|
import { render } from "@testing-library/react";
|
||||||
|
|
||||||
|
// Mutable pathname shared with the hoisted mock factory.
|
||||||
|
const { pathnameMock } = vi.hoisted(() => ({ pathnameMock: vi.fn() }));
|
||||||
|
vi.mock("next/navigation", () => ({ usePathname: () => pathnameMock() }));
|
||||||
|
|
||||||
|
import { SidebarFooter } from "../sidebar";
|
||||||
|
|
||||||
|
function linkFor(container: HTMLElement, href: string) {
|
||||||
|
return container.querySelector<HTMLAnchorElement>(`a[href="${href}"]`);
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("SidebarFooter active state (exact match)", () => {
|
||||||
|
beforeEach(() => pathnameMock.mockReset());
|
||||||
|
|
||||||
|
it("highlights only the matching footer link on /settings", () => {
|
||||||
|
pathnameMock.mockReturnValue("/settings");
|
||||||
|
const { container } = render(<SidebarFooter />);
|
||||||
|
expect(linkFor(container, "/settings")?.className).toMatch(/bg-primary/);
|
||||||
|
expect(linkFor(container, "/settings/ai-providers")?.className).not.toMatch(
|
||||||
|
/bg-primary/,
|
||||||
|
);
|
||||||
|
expect(linkFor(container, "/business")?.className).not.toMatch(/bg-primary/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not double-highlight /settings when on /settings/ai-providers", () => {
|
||||||
|
pathnameMock.mockReturnValue("/settings/ai-providers");
|
||||||
|
const { container } = render(<SidebarFooter />);
|
||||||
|
expect(linkFor(container, "/settings/ai-providers")?.className).toMatch(
|
||||||
|
/bg-primary/,
|
||||||
|
);
|
||||||
|
expect(linkFor(container, "/settings")?.className).not.toMatch(/bg-primary/);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -119,18 +119,24 @@ export function SidebarFooter({
|
|||||||
collapsed?: boolean;
|
collapsed?: boolean;
|
||||||
onNavigate?: () => void;
|
onNavigate?: () => void;
|
||||||
}) {
|
}) {
|
||||||
|
const pathname = usePathname();
|
||||||
// No Separator here: both wrappers (desktop aside + mobile Sheet) already
|
// No Separator here: both wrappers (desktop aside + mobile Sheet) already
|
||||||
// draw a border-t, and a second line reads as a rendering glitch.
|
// draw a border-t, and a second line reads as a rendering glitch.
|
||||||
return (
|
return (
|
||||||
<div className="space-y-1">
|
<div className="space-y-1">
|
||||||
{footerItems.map((item) => {
|
{footerItems.map((item) => {
|
||||||
|
// Exact match: /settings must not also highlight on /settings/ai-providers.
|
||||||
|
const isActive = pathname === item.href;
|
||||||
const link = (
|
const link = (
|
||||||
<Link
|
<Link
|
||||||
prefetch={false}
|
prefetch={false}
|
||||||
href={item.href}
|
href={item.href}
|
||||||
onClick={onNavigate}
|
onClick={onNavigate}
|
||||||
className={cn(
|
className={cn(
|
||||||
"flex items-center gap-3 rounded-lg px-3 py-2 text-sm font-medium text-muted-foreground hover:bg-muted hover:text-foreground transition-colors",
|
"flex items-center gap-3 rounded-lg px-3 py-2 text-sm font-medium transition-colors",
|
||||||
|
isActive
|
||||||
|
? "bg-primary text-primary-foreground"
|
||||||
|
: "text-muted-foreground hover:bg-muted hover:text-foreground",
|
||||||
collapsed && "justify-center px-2",
|
collapsed && "justify-center px-2",
|
||||||
)}
|
)}
|
||||||
>
|
>
|
||||||
|
|||||||
@@ -0,0 +1,23 @@
|
|||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import { pickTab } from "@/lib/tabs";
|
||||||
|
|
||||||
|
const VIEWS = ["dev", "qa", "pr-review", "pm"] as const;
|
||||||
|
|
||||||
|
describe("pickTab", () => {
|
||||||
|
it("returns the raw value when it is in the valid set", () => {
|
||||||
|
expect(pickTab("qa", VIEWS, "dev")).toBe("qa");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("falls back when raw is null", () => {
|
||||||
|
expect(pickTab(null, VIEWS, "dev")).toBe("dev");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("falls back when raw is an invalid/typo value", () => {
|
||||||
|
expect(pickTab("deev", VIEWS, "dev")).toBe("dev");
|
||||||
|
expect(pickTab("foo", VIEWS, "dev")).toBe("dev");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("falls back when raw is the empty string", () => {
|
||||||
|
expect(pickTab("", VIEWS, "dev")).toBe("dev");
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,13 @@
|
|||||||
|
/**
|
||||||
|
* Validate a URL search-param tab/view value against a known set, falling
|
||||||
|
* back to a default when it is null, empty, or not in the set. Replaces the
|
||||||
|
* bare `as T || default` cast that only guards null and lets a typo value
|
||||||
|
* blank the active tab + content pane.
|
||||||
|
*/
|
||||||
|
export function pickTab<T extends string>(
|
||||||
|
raw: string | null,
|
||||||
|
valid: readonly T[],
|
||||||
|
fallback: T,
|
||||||
|
): T {
|
||||||
|
return valid.includes(raw as T) ? (raw as T) : fallback;
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user