Skip to content

fix(view): locked-repo reorder skips invisible repos #73

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wgordon17 wants to merge 4 commits into from
Closed

fix(view): locked-repo reorder skips invisible repos #73

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1f7c2aa
fix(view): moveLockedRepo skips invisible repos during reorder
wgordon17 Apr 19, 2026
0918532
fix(ui): RepoLockControls uses visible locked repos for reorder
wgordon17 Apr 19, 2026
fc27cd0
refactor(ui): simplifies insertIdx, memoizes Set
wgordon17 Apr 19, 2026
711f7c3
test(view): adds skip-multiple-invisible down direction test
wgordon17 Apr 19, 2026
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
6 changes: 5 additions & 1 deletion src/app/components/dashboard/ActionsTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,10 @@ export default function ActionsTab(props: ActionsTabProps) {
const repoGroups = createMemo(() =>
orderRepoGroups(groupRuns(filteredRuns()), viewState.lockedRepos)
);
const visibleLockedRepos = createMemo(() => {
const rendered = new Set(repoGroups().map(g => g.repoFullName));
return viewState.lockedRepos.filter(name => rendered.has(name));
});

createEffect(() => {
const names = activeRepoNames();
Expand Down Expand Up @@ -302,7 +306,7 @@ export default function ActionsTab(props: ActionsTabProps) {
trailing={
<>
<RepoGitHubLink repoFullName={repoGroup.repoFullName} section="actions" />
<RepoLockControls repoFullName={repoGroup.repoFullName} />
<RepoLockControls repoFullName={repoGroup.repoFullName} visibleLockedRepos={visibleLockedRepos()} />
</>
}
collapsedSummary={
Expand Down
6 changes: 5 additions & 1 deletion src/app/components/dashboard/IssuesTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ export default function IssuesTab(props: IssuesTabProps) {
const repoGroups = createMemo(() =>
orderRepoGroups(groupByRepo(filteredSorted()), viewState.lockedRepos)
);
const visibleLockedRepos = createMemo(() => {
const rendered = new Set(repoGroups().map(g => g.repoFullName));
return viewState.lockedRepos.filter(name => rendered.has(name));
});
const pageLayout = createMemo(() => computePageLayout(repoGroups(), config.itemsPerPage));
const pageCount = createMemo(() => pageLayout().pageCount);
const pageGroups = createMemo(() =>
Expand Down Expand Up @@ -367,7 +371,7 @@ export default function IssuesTab(props: IssuesTabProps) {
trailing={
<>
<RepoGitHubLink repoFullName={repoGroup.repoFullName} section="issues" />
<RepoLockControls repoFullName={repoGroup.repoFullName} />
<RepoLockControls repoFullName={repoGroup.repoFullName} visibleLockedRepos={visibleLockedRepos()} />
</>
}
collapsedSummary={
Expand Down
6 changes: 5 additions & 1 deletion src/app/components/dashboard/PullRequestsTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,10 @@ export default function PullRequestsTab(props: PullRequestsTabProps) {
const repoGroups = createMemo(() =>
orderRepoGroups(groupByRepo(filteredSorted()), viewState.lockedRepos)
);
const visibleLockedRepos = createMemo(() => {
const rendered = new Set(repoGroups().map(g => g.repoFullName));
return viewState.lockedRepos.filter(name => rendered.has(name));
});
const pageLayout = createMemo(() => computePageLayout(repoGroups(), config.itemsPerPage));
const pageCount = createMemo(() => pageLayout().pageCount);
const pageGroups = createMemo(() =>
Expand Down Expand Up @@ -474,7 +478,7 @@ export default function PullRequestsTab(props: PullRequestsTabProps) {
trailing={
<>
<RepoGitHubLink repoFullName={repoGroup.repoFullName} section="pulls" />
<RepoLockControls repoFullName={repoGroup.repoFullName} />
<RepoLockControls repoFullName={repoGroup.repoFullName} visibleLockedRepos={visibleLockedRepos()} />
</>
}
collapsedSummary={
Expand Down
17 changes: 10 additions & 7 deletions src/app/components/shared/RepoLockControls.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,19 @@ import { withFlipAnimation } from "../../lib/scroll";

interface RepoLockControlsProps {
repoFullName: string;
visibleLockedRepos: string[];
}

export default function RepoLockControls(props: RepoLockControlsProps) {
const visibleSet = createMemo(() => new Set(props.visibleLockedRepos));

const lockInfo = createMemo(() => {
const list = viewState.lockedRepos;
const idx = list.indexOf(props.repoFullName);
const isLocked = viewState.lockedRepos.indexOf(props.repoFullName) !== -1;
const visIdx = props.visibleLockedRepos.indexOf(props.repoFullName);
return {
isLocked: idx !== -1,
isFirst: idx === 0,
isLast: idx !== -1 && idx === list.length - 1,
isLocked,
isFirst: visIdx === 0,
isLast: visIdx !== -1 && visIdx === props.visibleLockedRepos.length - 1,
};
});

Expand Down Expand Up @@ -52,7 +55,7 @@ export default function RepoLockControls(props: RepoLockControlsProps) {
<Tooltip content={lockInfo().isFirst ? "Already at top of pinned list" : "Move up"}>
<button
class="btn btn-ghost btn-xs"
onClick={() => withFlipAnimation(() => moveLockedRepo(props.repoFullName, "up"))}
onClick={() => withFlipAnimation(() => moveLockedRepo(props.repoFullName, "up", visibleSet()))}
disabled={lockInfo().isFirst}
aria-label={`Move ${props.repoFullName} up`}
>
Expand All @@ -65,7 +68,7 @@ export default function RepoLockControls(props: RepoLockControlsProps) {
<Tooltip content={lockInfo().isLast ? "Already at bottom of pinned list" : "Move down"}>
<button
class="btn btn-ghost btn-xs"
onClick={() => withFlipAnimation(() => moveLockedRepo(props.repoFullName, "down"))}
onClick={() => withFlipAnimation(() => moveLockedRepo(props.repoFullName, "down", visibleSet()))}
disabled={lockInfo().isLast}
aria-label={`Move ${props.repoFullName} down`}
>
Expand Down
36 changes: 30 additions & 6 deletions src/app/stores/view.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,17 +341,41 @@ export function unlockRepo(repoFullName: string): void {

export function moveLockedRepo(
repoFullName: string,
direction: "up" | "down"
direction: "up" | "down",
visibleRepoNames?: ReadonlySet<string>
): void {
setViewState(produce((draft) => {
const arr = draft.lockedRepos;
const idx = arr.indexOf(repoFullName);
if (idx === -1) return;
const targetIdx = direction === "up" ? idx - 1 : idx + 1;
if (targetIdx < 0 || targetIdx >= arr.length) return;
const tmp = arr[idx];
arr[idx] = arr[targetIdx];
arr[targetIdx] = tmp;

if (!visibleRepoNames) {
// Backward-compatible adjacent swap
const targetIdx = direction === "up" ? idx - 1 : idx + 1;
if (targetIdx < 0 || targetIdx >= arr.length) return;
const tmp = arr[idx];
arr[idx] = arr[targetIdx];
arr[targetIdx] = tmp;
return;
}

// Find the next visible repo in the given direction
const step = direction === "up" ? -1 : 1;
let targetIdx = -1;
for (let i = idx + step; i >= 0 && i < arr.length; i += step) {
if (visibleRepoNames.has(arr[i])) {
targetIdx = i;
break;
}
}
if (targetIdx === -1) return;

// Remove from old position and insert adjacent to target
arr.splice(idx, 1);
// After removal, target index shifts down by 1 if source preceded it
const adjustedTarget = targetIdx - (idx < targetIdx ? 1 : 0);
const insertIdx = direction === "up" ? adjustedTarget : adjustedTarget + 1;
arr.splice(insertIdx, 0, repoFullName);
}));
}

Expand Down
63 changes: 49 additions & 14 deletions tests/components/shared/RepoLockControls.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ beforeEach(() => {
describe("RepoLockControls", () => {
it("renders unlock (pin) icon when repo is not locked", () => {
render(() => (
<RepoLockControls repoFullName="owner/repo" />
<RepoLockControls repoFullName="owner/repo" visibleLockedRepos={[]} />
));
expect(screen.getByLabelText("Pin owner/repo to top of list")).toBeTruthy();
});

it("renders lock icon + chevrons when repo IS locked", () => {
lockRepo("owner/repo");
render(() => (
<RepoLockControls repoFullName="owner/repo" />
<RepoLockControls repoFullName="owner/repo" visibleLockedRepos={["owner/repo"]} />
));
expect(screen.getByLabelText("Unpin owner/repo")).toBeTruthy();
expect(screen.getByLabelText("Move owner/repo up")).toBeTruthy();
Expand All @@ -27,7 +27,7 @@ describe("RepoLockControls", () => {

it("click pin icon → locks the repo", () => {
render(() => (
<RepoLockControls repoFullName="owner/repo" />
<RepoLockControls repoFullName="owner/repo" visibleLockedRepos={[]} />
));
fireEvent.click(screen.getByLabelText("Pin owner/repo to top of list"));
expect(viewState.lockedRepos).toContain("owner/repo");
Expand All @@ -36,7 +36,7 @@ describe("RepoLockControls", () => {
it("click lock icon → unlocks the repo", () => {
lockRepo("owner/repo");
render(() => (
<RepoLockControls repoFullName="owner/repo" />
<RepoLockControls repoFullName="owner/repo" visibleLockedRepos={["owner/repo"]} />
));
fireEvent.click(screen.getByLabelText("Unpin owner/repo"));
expect(viewState.lockedRepos).not.toContain("owner/repo");
Expand All @@ -46,7 +46,7 @@ describe("RepoLockControls", () => {
lockRepo("owner/a");
lockRepo("owner/b");
render(() => (
<RepoLockControls repoFullName="owner/b" />
<RepoLockControls repoFullName="owner/b" visibleLockedRepos={["owner/a", "owner/b"]} />
));
fireEvent.click(screen.getByLabelText("Move owner/b up"));
expect(viewState.lockedRepos[0]).toBe("owner/b");
Expand All @@ -57,7 +57,7 @@ describe("RepoLockControls", () => {
lockRepo("owner/a");
lockRepo("owner/b");
render(() => (
<RepoLockControls repoFullName="owner/a" />
<RepoLockControls repoFullName="owner/a" visibleLockedRepos={["owner/a", "owner/b"]} />
));
fireEvent.click(screen.getByLabelText("Move owner/a down"));
expect(viewState.lockedRepos[0]).toBe("owner/b");
Expand All @@ -67,7 +67,7 @@ describe("RepoLockControls", () => {
it("up button is disabled when repo is first in locked list", () => {
lockRepo("owner/repo");
render(() => (
<RepoLockControls repoFullName="owner/repo" />
<RepoLockControls repoFullName="owner/repo" visibleLockedRepos={["owner/repo"]} />
));
const upBtn = screen.getByLabelText("Move owner/repo up") as HTMLButtonElement;
expect(upBtn.disabled).toBe(true);
Expand All @@ -76,18 +76,53 @@ describe("RepoLockControls", () => {
it("down button is disabled when repo is last in locked list", () => {
lockRepo("owner/repo");
render(() => (
<RepoLockControls repoFullName="owner/repo" />
<RepoLockControls repoFullName="owner/repo" visibleLockedRepos={["owner/repo"]} />
));
const downBtn = screen.getByLabelText("Move owner/repo down") as HTMLButtonElement;
expect(downBtn.disabled).toBe(true);
});

it("up button disabled when first in visibleLockedRepos even if hidden repos precede it", () => {
lockRepo("owner/hidden");
lockRepo("owner/a");
lockRepo("owner/b");
render(() => (
<RepoLockControls repoFullName="owner/a" visibleLockedRepos={["owner/a", "owner/b"]} />
));
const upBtn = screen.getByLabelText("Move owner/a up") as HTMLButtonElement;
expect(upBtn.disabled).toBe(true);
});

it("down button disabled when last in visibleLockedRepos even if hidden repos follow it", () => {
lockRepo("owner/a");
lockRepo("owner/b");
lockRepo("owner/hidden");
render(() => (
<RepoLockControls repoFullName="owner/b" visibleLockedRepos={["owner/a", "owner/b"]} />
));
const downBtn = screen.getByLabelText("Move owner/b down") as HTMLButtonElement;
expect(downBtn.disabled).toBe(true);
});

it("up/down buttons enabled when repo has visible neighbors", () => {
lockRepo("owner/a");
lockRepo("owner/b");
lockRepo("owner/c");
render(() => (
<RepoLockControls repoFullName="owner/b" visibleLockedRepos={["owner/a", "owner/b", "owner/c"]} />
));
const upBtn = screen.getByLabelText("Move owner/b up") as HTMLButtonElement;
const downBtn = screen.getByLabelText("Move owner/b down") as HTMLButtonElement;
expect(upBtn.disabled).toBe(false);
expect(downBtn.disabled).toBe(false);
});

it("stopPropagation — parent click NOT triggered on locked button click", () => {
lockRepo("owner/repo");
const parentClick = vi.fn();
render(() => (
<div onClick={parentClick}>
<RepoLockControls repoFullName="owner/repo" />
<RepoLockControls repoFullName="owner/repo" visibleLockedRepos={["owner/repo"]} />
</div>
));
fireEvent.click(screen.getByLabelText("Unpin owner/repo"));
Expand All @@ -98,7 +133,7 @@ describe("RepoLockControls", () => {
const parentClick = vi.fn();
render(() => (
<div onClick={parentClick}>
<RepoLockControls repoFullName="owner/repo" />
<RepoLockControls repoFullName="owner/repo" visibleLockedRepos={[]} />
</div>
));
fireEvent.click(screen.getByLabelText("Pin owner/repo to top of list"));
Expand All @@ -123,7 +158,7 @@ describe("RepoLockControls — scroll preservation", () => {

it("preserves scroll position when locking a repo", () => {
render(() => (
<RepoLockControls repoFullName="owner/repo" />
<RepoLockControls repoFullName="owner/repo" visibleLockedRepos={[]} />
));
fireEvent.click(screen.getByLabelText("Pin owner/repo to top of list"));
expect(window.scrollTo).toHaveBeenCalledWith(0, 500);
Expand All @@ -132,7 +167,7 @@ describe("RepoLockControls — scroll preservation", () => {
it("preserves scroll position when unlocking a repo", () => {
lockRepo("owner/repo");
render(() => (
<RepoLockControls repoFullName="owner/repo" />
<RepoLockControls repoFullName="owner/repo" visibleLockedRepos={["owner/repo"]} />
));
fireEvent.click(screen.getByLabelText("Unpin owner/repo"));
expect(window.scrollTo).toHaveBeenCalledWith(0, 500);
Expand All @@ -142,7 +177,7 @@ describe("RepoLockControls — scroll preservation", () => {
lockRepo("owner/a");
lockRepo("owner/b");
render(() => (
<RepoLockControls repoFullName="owner/b" />
<RepoLockControls repoFullName="owner/b" visibleLockedRepos={["owner/a", "owner/b"]} />
));
fireEvent.click(screen.getByLabelText("Move owner/b up"));
expect(window.scrollTo).toHaveBeenCalledWith(0, 500);
Expand All @@ -152,7 +187,7 @@ describe("RepoLockControls — scroll preservation", () => {
lockRepo("owner/a");
lockRepo("owner/b");
render(() => (
<RepoLockControls repoFullName="owner/a" />
<RepoLockControls repoFullName="owner/a" visibleLockedRepos={["owner/a", "owner/b"]} />
));
fireEvent.click(screen.getByLabelText("Move owner/a down"));
expect(window.scrollTo).toHaveBeenCalledWith(0, 500);
Expand Down
67 changes: 67 additions & 0 deletions tests/stores/view-lock.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,73 @@ describe("view lock store", () => {
moveLockedRepo("org/repo-z", "up");
expect(viewState.lockedRepos).toEqual(["org/repo-a"]);
});

describe("with visibleRepoNames", () => {
it("skips one invisible repo when moving up", () => {
lockRepo("org/a");
lockRepo("org/hidden");
lockRepo("org/c");
moveLockedRepo("org/c", "up", new Set(["org/a", "org/c"]));
expect(viewState.lockedRepos).toEqual(["org/c", "org/a", "org/hidden"]);
});

it("skips one invisible repo when moving down", () => {
lockRepo("org/a");
lockRepo("org/hidden");
lockRepo("org/c");
moveLockedRepo("org/a", "down", new Set(["org/a", "org/c"]));
expect(viewState.lockedRepos).toEqual(["org/hidden", "org/c", "org/a"]);
});

it("skips multiple invisible repos when moving up", () => {
lockRepo("org/a");
lockRepo("org/h1");
lockRepo("org/h2");
lockRepo("org/d");
moveLockedRepo("org/d", "up", new Set(["org/a", "org/d"]));
expect(viewState.lockedRepos).toEqual(["org/d", "org/a", "org/h1", "org/h2"]);
});

it("skips multiple invisible repos when moving down", () => {
lockRepo("org/a");
lockRepo("org/h1");
lockRepo("org/h2");
lockRepo("org/d");
moveLockedRepo("org/a", "down", new Set(["org/a", "org/d"]));
expect(viewState.lockedRepos).toEqual(["org/h1", "org/h2", "org/d", "org/a"]);
});

it("no-op when already first visible and moving up", () => {
lockRepo("org/h1");
lockRepo("org/a");
lockRepo("org/b");
moveLockedRepo("org/a", "up", new Set(["org/a", "org/b"]));
expect(viewState.lockedRepos).toEqual(["org/h1", "org/a", "org/b"]);
});

it("no-op when already last visible and moving down", () => {
lockRepo("org/a");
lockRepo("org/b");
lockRepo("org/h1");
moveLockedRepo("org/b", "down", new Set(["org/a", "org/b"]));
expect(viewState.lockedRepos).toEqual(["org/a", "org/b", "org/h1"]);
});

it("falls back to adjacent swap when visibleRepoNames is undefined", () => {
lockRepo("org/a");
lockRepo("org/b");
lockRepo("org/c");
moveLockedRepo("org/c", "up");
expect(viewState.lockedRepos).toEqual(["org/a", "org/c", "org/b"]);
});

it("no-op when visibleRepoNames is empty set", () => {
lockRepo("org/a");
lockRepo("org/b");
moveLockedRepo("org/a", "down", new Set());
expect(viewState.lockedRepos).toEqual(["org/a", "org/b"]);
});
});
});

describe("pruneLockedRepos", () => {
Expand Down