Skip to content
Draft
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
60 changes: 36 additions & 24 deletions apps/desktop/src/features/workspace/RoleSwitcher.test.tsx
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@
import { readFileSync } from "node:fs";
import { resolve } from "node:path";
import { fireEvent, render, screen } from "@testing-library/react";
import { describe, expect, it, vi } from "vitest";
import { RoleSwitcher, tabValueToRoleId } from "./RoleSwitcher";

vi.mock("../../i18n", () => ({
createTranslator: () => (key: string) =>
createTranslator: () => (translationKey: string) =>
({
allRoles: "All Roles",
roleSwitcherTitle: "Role-specific View"
})[key] ?? key,
})[translationKey] ?? translationKey,
detectPreferredLocale: () => "en"
}));

describe("RoleSwitcher", () => {

it("renders the title and role options", () => {
const roles = [
{ id: "bass-guitar", name: "Bass Guitar" },
{ id: "lead-vocal", name: "Lead Vocal" }
const roleOptions = [
{ roleId: "bass-guitar", roleName: "Bass Guitar" },
{ roleId: "lead-vocal", roleName: "Lead Vocal" }
];

render(<RoleSwitcher roles={roles} activeRole={null} onRoleChange={vi.fn()} />);
render(<RoleSwitcher roleOptions={roleOptions} activeRole={null} onRoleChange={vi.fn()} />);

expect(screen.getByText("Role-specific View")).toBeInTheDocument();
expect(screen.getByRole("tab", { name: "All Roles" })).toBeInTheDocument();
Expand All @@ -28,48 +29,59 @@ describe("RoleSwitcher", () => {
});

it("keeps the all-roles control distinct from a real role whose id is all", () => {
const onRoleChange = vi.fn();
const roleChangeHandler = vi.fn();

render(
<RoleSwitcher
roles={[
{ id: "all", name: "Alloy Synth" },
{ id: "bass-guitar", name: "Bass Guitar" }
roleOptions={[
{ roleId: "all", roleName: "Alloy Synth" },
{ roleId: "bass-guitar", roleName: "Bass Guitar" }
]}
activeRole="bass-guitar"
onRoleChange={onRoleChange}
onRoleChange={roleChangeHandler}
/>
);

fireEvent.click(screen.getByRole("tab", { name: "Alloy Synth" }));
expect(onRoleChange).toHaveBeenLastCalledWith("all");
expect(roleChangeHandler).toHaveBeenLastCalledWith("all");

fireEvent.click(screen.getByRole("tab", { name: "All Roles" }));
expect(onRoleChange).toHaveBeenLastCalledWith(null);
expect(roleChangeHandler).toHaveBeenLastCalledWith(null);
});

it("uses the project-standard active tab data selector", () => {
render(
<RoleSwitcher
roles={[{ id: "bass-guitar", name: "Bass Guitar" }]}
roleOptions={[{ roleId: "bass-guitar", roleName: "Bass Guitar" }]}
activeRole={null}
onRoleChange={vi.fn()}
/>
);

const allRolesTrigger = screen.getByRole("tab", { name: "All Roles" });
expect(allRolesTrigger.className).toContain("data-active:bg-cyan-300");
expect(allRolesTrigger.className).not.toContain("data-[state=active]:");
const allRolesTab = screen.getByRole("tab", { name: "All Roles" });
expect(allRolesTab.className).toContain("data-active:bg-cyan-300");
expect(allRolesTab.className).not.toContain("data-[state=active]:");
});

it("ignores tab values that are not in the rendered role allowlist", () => {
const roles = [
{ id: "bass-guitar", name: "Bass Guitar" },
{ id: "lead-vocal", name: "Lead Vocal" }
const roleOptions = [
{ roleId: "bass-guitar", roleName: "Bass Guitar" },
{ roleId: "lead-vocal", roleName: "Lead Vocal" }
];

expect(tabValueToRoleId("role:bass-guitar", roles)).toBe("bass-guitar");
expect(tabValueToRoleId("role:unknown-role", roles)).toBeNull();
expect(tabValueToRoleId("raw-unknown-role", roles)).toBeNull();
expect(tabValueToRoleId("role:bass-guitar", roleOptions)).toBe("bass-guitar");
expect(tabValueToRoleId("role:unknown-role", roleOptions)).toBeNull();
expect(tabValueToRoleId("raw-unknown-role", roleOptions)).toBeNull();
});

it("does not retain the deprecated id/name compatibility layer", () => {
const roleSwitcherSource = readFileSync(
resolve(process.cwd(), "src/features/workspace/RoleSwitcher.tsx"),
"utf8"
);

expect(roleSwitcherSource).not.toContain("LegacyRehearsalRoleOption");
expect(roleSwitcherSource).not.toContain("normalizeLegacyRoleOptions");
expect(roleSwitcherSource).not.toContain("roles: LegacyRehearsalRoleOption[]");
});
});
41 changes: 23 additions & 18 deletions apps/desktop/src/features/workspace/RoleSwitcher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@ import { createTranslator, detectPreferredLocale } from "../../i18n";
import { Tabs, TabsList, TabsTrigger } from "@/components/ui/tabs";
import { Users } from "lucide-react";

/** Renderable role option accepted by the role tab allowlist. */
/** A selectable rehearsal-role projection owned by the workspace role switcher. */
export interface RehearsalRoleOption {
id: string;
name: string;
/** Stable role identity used by selection and tab-value mapping. */
roleId: string;
/** Human-readable role name rendered in the role switcher. */
roleName: string;
}

interface RoleSwitcherProps {
roles: RehearsalRoleOption[];
roleOptions: RehearsalRoleOption[];
activeRole: string | null;
onRoleChange: (roleId: string | null) => void;
}
Expand All @@ -23,48 +25,51 @@ function roleTabValue(roleId: string): string {
}

/** Documented. */
export function tabValueToRoleId(value: string, roles: RehearsalRoleOption[]): string | null {
if (value === ALL_ROLES_VALUE) {
export function tabValueToRoleId(
tabValue: string,
roleOptions: RehearsalRoleOption[]
): string | null {
if (tabValue === ALL_ROLES_VALUE) {
return null;
}

if (!value.startsWith(ROLE_VALUE_PREFIX)) {
if (!tabValue.startsWith(ROLE_VALUE_PREFIX)) {
return null;
}

const roleId = value.slice(ROLE_VALUE_PREFIX.length);
return roles.some((role) => role.id === roleId) ? roleId : null;
const roleId = tabValue.slice(ROLE_VALUE_PREFIX.length);
return roleOptions.some((roleOption) => roleOption.roleId === roleId) ? roleId : null;
}

/** Documented. */
export function RoleSwitcher({ roles, activeRole, onRoleChange }: RoleSwitcherProps) {
const t = createTranslator(detectPreferredLocale());
export function RoleSwitcher({ roleOptions, activeRole, onRoleChange }: RoleSwitcherProps) {
const translatedText = createTranslator(detectPreferredLocale());

return (
<div className="flex flex-col gap-4 py-2 sm:flex-row sm:items-center">
<div className="flex whitespace-nowrap text-sm font-semibold text-slate-200">
<Users className="mr-2 size-4 text-cyan-300" aria-hidden="true" />
{t("roleSwitcherTitle")}
{translatedText("roleSwitcherTitle")}
</div>
<Tabs
value={activeRole === null ? ALL_ROLES_VALUE : roleTabValue(activeRole)}
onValueChange={(val) => onRoleChange(tabValueToRoleId(val, roles))}
onValueChange={(tabValue) => onRoleChange(tabValueToRoleId(tabValue, roleOptions))}
className="w-full sm:w-auto"
>
<TabsList className="h-auto w-full flex-wrap justify-start border border-white/10 bg-white/[0.05] p-1 sm:h-10 sm:w-auto">
<TabsTrigger
value={ALL_ROLES_VALUE}
className="rounded-md px-4 text-slate-300 data-active:bg-cyan-300 data-active:text-slate-950 data-active:shadow-[0_8px_24px_rgba(34,211,238,0.24)]"
>
{t("allRoles")}
{translatedText("allRoles")}
</TabsTrigger>
{roles.map((role) => (
{roleOptions.map((roleOption) => (
<TabsTrigger
key={role.id}
value={roleTabValue(role.id)}
key={roleOption.roleId}
value={roleTabValue(roleOption.roleId)}
className="rounded-md px-4 text-slate-300 data-active:bg-cyan-300 data-active:text-slate-950 data-active:shadow-[0_8px_24px_rgba(34,211,238,0.24)]"
>
{role.name}
{roleOption.roleName}
</TabsTrigger>
))}
</TabsList>
Expand Down
10 changes: 5 additions & 5 deletions apps/desktop/src/features/workspace/Workspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,13 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp
return map;
}, [song]);

const allRoles = useMemo(() => {
const roleOptions = useMemo(() => {
// Performance: Avoid O(N) allocation of intermediate array from Array.from() before mapping
const roles: { id: string; name: string }[] = [];
const options: { roleId: string; roleName: string }[] = [];
for (const role of roleMap.values()) {
roles.push({ id: role.id, name: role.name });
options.push({ roleId: role.id, roleName: role.name });
}
return roles;
return options;
}, [roleMap]);

// Performance: use the cached roleMap so activeRoleDetails does not rescan sections and roles on every render.
Expand Down Expand Up @@ -362,7 +362,7 @@ export function Workspace({ song, sourceBootstrap = null, onSongUpdate }: Worksp
<p className="mt-1 text-sm text-slate-400">Filter the board by player or vocal role without losing the full form context.</p>
</div>
<RoleSwitcher
roles={allRoles}
roleOptions={roleOptions}
activeRole={activeRole}
onRoleChange={setActiveRole}
/>
Expand Down
Loading