Auto-evict the oldest search tab at capacity, raise the limit to 20 (#442)
This commit is contained in:
parent
32b1b1d38c
commit
acd28749c8
@ -193,10 +193,6 @@ export function BacklinksPage({
|
||||
<BacklinksSearchCard
|
||||
errorMessage={overviewErrorMessage}
|
||||
initialValues={searchCardInitialValues}
|
||||
canOpenSearch={(values) =>
|
||||
searchTabs.canOpenTab(toBacklinksTabInput(values))
|
||||
}
|
||||
tabLimit={searchTabs.limit}
|
||||
onSubmit={(values) => {
|
||||
searchTabs.openTab(toBacklinksTabInput(values));
|
||||
navigateToBacklinksSearch(navigate, values);
|
||||
|
||||
@ -18,8 +18,6 @@ type SearchDraft = Pick<BacklinksSearchState, "target" | "scope">;
|
||||
function getBacklinksValidationErrors(
|
||||
value: SearchDraft,
|
||||
shouldValidateUntouchedField: boolean,
|
||||
canOpenSearch?: (value: SearchDraft) => boolean,
|
||||
tabLimit?: number,
|
||||
) {
|
||||
if (!value.target.trim()) {
|
||||
if (!shouldValidateUntouchedField) {
|
||||
@ -33,34 +31,17 @@ function getBacklinksValidationErrors(
|
||||
});
|
||||
}
|
||||
|
||||
const normalizedValue = {
|
||||
...value,
|
||||
target: value.target.trim(),
|
||||
};
|
||||
|
||||
if (canOpenSearch && !canOpenSearch(normalizedValue)) {
|
||||
return createFormValidationErrors({
|
||||
fields: {
|
||||
target: `Close a tab to open more searches (max ${tabLimit ?? 8}).`,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
export function BacklinksSearchCard({
|
||||
canOpenSearch,
|
||||
errorMessage,
|
||||
initialValues,
|
||||
onSubmit,
|
||||
tabLimit,
|
||||
}: {
|
||||
canOpenSearch?: (values: SearchDraft) => boolean;
|
||||
errorMessage: string | null;
|
||||
initialValues: SearchDraft;
|
||||
onSubmit: (values: SearchDraft) => void;
|
||||
tabLimit?: number;
|
||||
}) {
|
||||
const [userSelectedScope, setUserSelectedScope] = useState(false);
|
||||
const form = useForm({
|
||||
@ -70,11 +51,8 @@ export function BacklinksSearchCard({
|
||||
getBacklinksValidationErrors(
|
||||
value,
|
||||
shouldValidateFieldOnChange(formApi, "target"),
|
||||
canOpenSearch,
|
||||
tabLimit,
|
||||
),
|
||||
onSubmit: ({ value }) =>
|
||||
getBacklinksValidationErrors(value, true, canOpenSearch, tabLimit),
|
||||
onSubmit: ({ value }) => getBacklinksValidationErrors(value, true),
|
||||
},
|
||||
onSubmit: ({ value }) => {
|
||||
const target = value.target.trim();
|
||||
|
||||
@ -482,39 +482,6 @@ export function DomainOverviewPage({
|
||||
navigateToInput: navigateToSearchTab,
|
||||
});
|
||||
|
||||
const handleSearchSubmit = useCallback(
|
||||
(event: FormEvent) => {
|
||||
const values = state.controlsForm.state.values;
|
||||
const target = normalizeDomainTarget(values.domain);
|
||||
if (!target) {
|
||||
state.handleSearchSubmit(event);
|
||||
return;
|
||||
}
|
||||
|
||||
const nextTabInput: SearchTabInput = {
|
||||
type: "domain",
|
||||
domain: target,
|
||||
subdomains: values.subdomains,
|
||||
locationCode: values.locationCode,
|
||||
};
|
||||
|
||||
if (!searchTabs.canOpenTab(nextTabInput)) {
|
||||
event.preventDefault();
|
||||
state.controlsForm.setErrorMap({
|
||||
onSubmit: createFormValidationErrors({
|
||||
fields: {
|
||||
domain: `Close a tab to open more searches (max ${searchTabs.limit}).`,
|
||||
},
|
||||
}),
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
state.handleSearchSubmit(event);
|
||||
},
|
||||
[searchTabs, state],
|
||||
);
|
||||
|
||||
const tabControls = routeState.domain ? (
|
||||
<div className="flex flex-col gap-2">
|
||||
<div>
|
||||
@ -555,7 +522,7 @@ export function DomainOverviewPage({
|
||||
<DomainSearchCard
|
||||
controlsForm={state.controlsForm}
|
||||
isLoading={state.isLoading}
|
||||
onSubmit={handleSearchSubmit}
|
||||
onSubmit={state.handleSearchSubmit}
|
||||
onSortChange={(sort) =>
|
||||
state.applySort(sort, getDefaultSortOrder(sort))
|
||||
}
|
||||
|
||||
@ -11,22 +11,12 @@ import {
|
||||
} from "@/client/features/keywords/keywordResearchTypes";
|
||||
import { parseKeywordInput } from "@/client/features/keywords/state/keywordControllerActions";
|
||||
|
||||
type KeywordTabValidationInput = {
|
||||
keyword: string;
|
||||
locationCode: number | undefined;
|
||||
resultLimit: ResultLimit;
|
||||
mode: KeywordMode;
|
||||
clickstream: boolean;
|
||||
};
|
||||
|
||||
type UseKeywordControlsFormInput = {
|
||||
keywordInput: string;
|
||||
locationCode: number;
|
||||
resultLimit: ResultLimit;
|
||||
keywordMode: KeywordMode;
|
||||
clickstream: boolean;
|
||||
getOpenKeywordTabs?: () => readonly KeywordTabValidationInput[];
|
||||
keywordTabsLimit?: number;
|
||||
};
|
||||
|
||||
export type KeywordControlsValues = {
|
||||
@ -66,62 +56,6 @@ function getKeywordSearchValidationErrors(
|
||||
return null;
|
||||
}
|
||||
|
||||
function getKeywordTabCapacityError(
|
||||
value: KeywordControlsValues,
|
||||
openKeywordTabs: readonly KeywordTabValidationInput[] | undefined,
|
||||
keywordTabsLimit: number | undefined,
|
||||
) {
|
||||
if (!openKeywordTabs || keywordTabsLimit == null) return null;
|
||||
|
||||
const keywords = parseKeywordInput(value.keyword);
|
||||
if (keywords.length === 0) return null;
|
||||
|
||||
let simulatedOpenTabs = [...openKeywordTabs];
|
||||
let skippedCount = 0;
|
||||
|
||||
for (const keyword of keywords) {
|
||||
const input = {
|
||||
keyword,
|
||||
locationCode: value.locationCode,
|
||||
resultLimit: value.resultLimit,
|
||||
mode: value.mode,
|
||||
clickstream: value.clickstream,
|
||||
};
|
||||
const alreadyOpen = simulatedOpenTabs.some((tab) =>
|
||||
keywordTabMatches(tab, input),
|
||||
);
|
||||
if (alreadyOpen) continue;
|
||||
|
||||
if (simulatedOpenTabs.length >= keywordTabsLimit) {
|
||||
skippedCount += 1;
|
||||
continue;
|
||||
}
|
||||
|
||||
simulatedOpenTabs = [...simulatedOpenTabs, input];
|
||||
}
|
||||
|
||||
if (skippedCount === 0) return null;
|
||||
|
||||
return createFormValidationErrors({
|
||||
fields: {
|
||||
keyword: `${skippedCount} keyword${skippedCount === 1 ? "" : "s"} skipped - close a tab to open more (max ${keywordTabsLimit}).`,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
function keywordTabMatches(
|
||||
tab: KeywordTabValidationInput,
|
||||
input: KeywordTabValidationInput,
|
||||
) {
|
||||
return (
|
||||
tab.keyword === input.keyword &&
|
||||
tab.locationCode === input.locationCode &&
|
||||
tab.resultLimit === input.resultLimit &&
|
||||
tab.mode === input.mode &&
|
||||
tab.clickstream === input.clickstream
|
||||
);
|
||||
}
|
||||
|
||||
export function useKeywordControlsForm(
|
||||
input: UseKeywordControlsFormInput,
|
||||
onSubmit: (value: KeywordControlsValues) => void,
|
||||
@ -142,12 +76,7 @@ export function useKeywordControlsForm(
|
||||
false,
|
||||
),
|
||||
onSubmit: ({ value }) =>
|
||||
getKeywordSearchValidationErrors(value, true, true) ??
|
||||
getKeywordTabCapacityError(
|
||||
value,
|
||||
input.getOpenKeywordTabs?.(),
|
||||
input.keywordTabsLimit,
|
||||
),
|
||||
getKeywordSearchValidationErrors(value, true, true),
|
||||
},
|
||||
onSubmit: ({ value }) => {
|
||||
onSubmit(value);
|
||||
|
||||
@ -1,5 +1,5 @@
|
||||
import { Link } from "@tanstack/react-router";
|
||||
import { useCallback, useEffect, useMemo } from "react";
|
||||
import { useCallback, useMemo } from "react";
|
||||
import { AlertCircle, ArrowLeft } from "lucide-react";
|
||||
import { getErrorCode } from "@/client/lib/error-messages";
|
||||
import { BILLING_ROUTE } from "@/shared/billing";
|
||||
@ -132,14 +132,10 @@ export function KeywordResearchPage(input: Props) {
|
||||
clickstream: value.clickstream,
|
||||
}));
|
||||
|
||||
let activeInput: KeywordSearchTabInput | null = null;
|
||||
for (const tabInput of inputs) {
|
||||
const result = searchTabs.openTab(tabInput);
|
||||
if (result.tab?.input.type === "keyword") {
|
||||
activeInput = result.tab.input;
|
||||
}
|
||||
searchTabs.openTab(tabInput);
|
||||
}
|
||||
if (activeInput) navigateToKeywordInput(activeInput);
|
||||
navigateToKeywordInput(inputs.at(-1) ?? null);
|
||||
},
|
||||
[navigateToKeywordInput, searchTabs],
|
||||
);
|
||||
@ -147,24 +143,6 @@ export function KeywordResearchPage(input: Props) {
|
||||
searchTabs.setActiveTab(null);
|
||||
navigateToKeywordInput(null);
|
||||
}, [navigateToKeywordInput, searchTabs]);
|
||||
const getOpenKeywordTabs = useCallback(
|
||||
() =>
|
||||
searchTabs.tabs.flatMap((tab) =>
|
||||
tab.input.type === "keyword"
|
||||
? [
|
||||
{
|
||||
keyword: tab.input.keyword,
|
||||
locationCode: tab.input.locationCode,
|
||||
resultLimit: tab.input.resultLimit,
|
||||
mode: tab.input.mode,
|
||||
clickstream: tab.input.clickstream,
|
||||
},
|
||||
]
|
||||
: [],
|
||||
),
|
||||
[searchTabs.tabs],
|
||||
);
|
||||
|
||||
const controllerInput = useMemo<ControllerProps>(
|
||||
() =>
|
||||
activeTab
|
||||
@ -178,24 +156,18 @@ export function KeywordResearchPage(input: Props) {
|
||||
resultLimit: activeTab.input.resultLimit,
|
||||
keywordMode: activeTab.input.mode,
|
||||
clickstream: activeTab.input.clickstream,
|
||||
getOpenKeywordTabs,
|
||||
keywordTabsLimit: searchTabs.limit,
|
||||
}
|
||||
: {
|
||||
...input,
|
||||
locationCode,
|
||||
displayedLocationCode,
|
||||
setPreferredLocationCode,
|
||||
getOpenKeywordTabs,
|
||||
keywordTabsLimit: searchTabs.limit,
|
||||
},
|
||||
[
|
||||
activeTab,
|
||||
getOpenKeywordTabs,
|
||||
input,
|
||||
displayedLocationCode,
|
||||
locationCode,
|
||||
searchTabs.limit,
|
||||
setPreferredLocationCode,
|
||||
],
|
||||
);
|
||||
@ -203,20 +175,6 @@ export function KeywordResearchPage(input: Props) {
|
||||
...controllerInput,
|
||||
onFormSubmit,
|
||||
});
|
||||
useEffect(() => {
|
||||
controller.controlsForm.setErrorMap({ onSubmit: undefined });
|
||||
controller.controlsForm.setFieldMeta("keyword", (meta) => ({
|
||||
...meta,
|
||||
errorMap: {
|
||||
...meta.errorMap,
|
||||
onSubmit: undefined,
|
||||
},
|
||||
errorSourceMap: {
|
||||
...meta.errorSourceMap,
|
||||
onSubmit: undefined,
|
||||
},
|
||||
}));
|
||||
}, [controller.controlsForm, searchTabs.tabs]);
|
||||
|
||||
return (
|
||||
<div className="px-4 py-4 md:px-6 md:py-6 pb-24 md:pb-8 overflow-auto">
|
||||
|
||||
@ -28,14 +28,6 @@ import {
|
||||
} from "./keywordControllerInternals";
|
||||
import { useKeywordOverviewState } from "./useKeywordOverviewState";
|
||||
|
||||
type OpenKeywordTabInput = {
|
||||
keyword: string;
|
||||
locationCode: number | undefined;
|
||||
resultLimit: ResultLimit;
|
||||
mode: KeywordMode;
|
||||
clickstream: boolean;
|
||||
};
|
||||
|
||||
export type KeywordResearchControllerInput = {
|
||||
projectId: string;
|
||||
keywordInput: string;
|
||||
@ -47,8 +39,6 @@ export type KeywordResearchControllerInput = {
|
||||
clickstream: boolean;
|
||||
sortField: SortField;
|
||||
sortDir: SortDir;
|
||||
getOpenKeywordTabs?: () => readonly OpenKeywordTabInput[];
|
||||
keywordTabsLimit?: number;
|
||||
/**
|
||||
* Called when the user submits the search form. Lets the caller decide
|
||||
* whether the submission opens tabs or just rewrites the URL — the
|
||||
@ -150,8 +140,6 @@ export function useKeywordResearchController(
|
||||
{
|
||||
...input,
|
||||
locationCode: displayedLocationCode,
|
||||
getOpenKeywordTabs: input.getOpenKeywordTabs,
|
||||
keywordTabsLimit: input.keywordTabsLimit,
|
||||
},
|
||||
(value) => {
|
||||
setPreferredLocationCode(value.locationCode);
|
||||
|
||||
@ -50,13 +50,10 @@ export function useSearchTabNavigation({
|
||||
return;
|
||||
}
|
||||
|
||||
const result = openTab({
|
||||
openTab({
|
||||
label: getLabel(urlInput),
|
||||
input: urlInput,
|
||||
});
|
||||
if (result.dropped) {
|
||||
setActiveTab(null);
|
||||
}
|
||||
}, [activeTabId, findMatchingTab, getLabel, openTab, setActiveTab, urlInput]);
|
||||
|
||||
const selectTab = useCallback(
|
||||
@ -85,7 +82,7 @@ export function useSearchTabNavigation({
|
||||
const openSearchTab = useCallback(
|
||||
(input: SearchTabInput) => {
|
||||
closedInputKeysRef.current.delete(tabInputKey(input));
|
||||
return openTab({
|
||||
openTab({
|
||||
label: getLabel(input),
|
||||
input,
|
||||
});
|
||||
@ -104,9 +101,7 @@ export function useSearchTabNavigation({
|
||||
return {
|
||||
activeTabId: tabs.activeTabId,
|
||||
tabs: visibleTabs,
|
||||
canOpenTab: tabs.canOpenTab,
|
||||
closeTab: closeSearchTab,
|
||||
limit: tabs.limit,
|
||||
markTabViewed,
|
||||
openTab: openSearchTab,
|
||||
selectTab,
|
||||
|
||||
@ -1,5 +1,6 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { parseStoredState } from "./useSearchTabs";
|
||||
import { appendTabWithEviction, parseStoredState } from "./useSearchTabs";
|
||||
import type { SearchTab } from "./types";
|
||||
|
||||
function persistedTab(input: unknown) {
|
||||
return {
|
||||
@ -11,6 +12,46 @@ function persistedTab(input: unknown) {
|
||||
};
|
||||
}
|
||||
|
||||
function searchTab(index: number): SearchTab {
|
||||
return {
|
||||
id: `tab-${index}`,
|
||||
label: `example-${index}.com`,
|
||||
createdAt: index,
|
||||
viewedAt: null,
|
||||
input: {
|
||||
type: "backlinks",
|
||||
target: `example-${index}.com`,
|
||||
scope: "domain",
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("appendTabWithEviction", () => {
|
||||
it("appends without evicting below the limit", () => {
|
||||
const tabs = Array.from({ length: 19 }, (_, index) => searchTab(index));
|
||||
|
||||
const next = appendTabWithEviction(tabs, searchTab(19));
|
||||
|
||||
expect(next).toHaveLength(20);
|
||||
expect(next[0].id).toBe("tab-0");
|
||||
expect(next[19].id).toBe("tab-19");
|
||||
});
|
||||
|
||||
it("evicts the oldest tab at capacity", () => {
|
||||
const tabs = Array.from({ length: 20 }, (_, index) => searchTab(index));
|
||||
|
||||
const next = appendTabWithEviction(tabs, searchTab(20));
|
||||
|
||||
expect(next).toHaveLength(20);
|
||||
expect(next[0].id).toBe("tab-1");
|
||||
expect(next[19].id).toBe("tab-20");
|
||||
});
|
||||
|
||||
it("appends to an empty list", () => {
|
||||
expect(appendTabWithEviction([], searchTab(0))).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("parseStoredState", () => {
|
||||
it("keeps domain tabs persisted without a locationCode (default location)", () => {
|
||||
const state = parseStoredState({
|
||||
@ -77,6 +118,24 @@ describe("parseStoredState", () => {
|
||||
expect(state.tabs[0].input).toMatchObject({ locationCode: 2840 });
|
||||
});
|
||||
|
||||
it("keeps the newest tabs when stored state exceeds the limit", () => {
|
||||
const state = parseStoredState({
|
||||
activeTabId: null,
|
||||
tabs: Array.from({ length: 25 }, (_, index) => ({
|
||||
...persistedTab({
|
||||
type: "backlinks",
|
||||
target: `example-${index}.com`,
|
||||
scope: "domain",
|
||||
}),
|
||||
id: `tab-${index}`,
|
||||
})),
|
||||
});
|
||||
|
||||
expect(state.tabs).toHaveLength(20);
|
||||
expect(state.tabs[0].id).toBe("tab-5");
|
||||
expect(state.tabs[19].id).toBe("tab-24");
|
||||
});
|
||||
|
||||
it("still rejects malformed tab inputs", () => {
|
||||
const state = parseStoredState({
|
||||
activeTabId: null,
|
||||
|
||||
@ -11,11 +11,6 @@ type OpenTabInput = {
|
||||
input: SearchTabInput;
|
||||
};
|
||||
|
||||
type OpenTabResult = {
|
||||
tab: SearchTab | null;
|
||||
dropped: boolean;
|
||||
};
|
||||
|
||||
const EMPTY_STATE: TabsState = {
|
||||
tabs: [],
|
||||
activeTabId: null,
|
||||
@ -23,7 +18,7 @@ const EMPTY_STATE: TabsState = {
|
||||
|
||||
const CHANGE_EVENT = "search-tabs-change";
|
||||
const stateCache = new Map<string, TabsState>();
|
||||
const SEARCH_TABS_LIMIT = 8;
|
||||
const SEARCH_TABS_LIMIT = 20;
|
||||
|
||||
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||
return typeof value === "object" && value !== null;
|
||||
@ -136,7 +131,8 @@ export function parseStoredState(value: unknown): TabsState {
|
||||
},
|
||||
];
|
||||
})
|
||||
.slice(0, SEARCH_TABS_LIMIT);
|
||||
// Keep the newest tabs when over the limit, matching openTab's eviction.
|
||||
.slice(-SEARCH_TABS_LIMIT);
|
||||
const activeTabId =
|
||||
typeof value.activeTabId === "string" &&
|
||||
tabs.some((tab) => tab.id === value.activeTabId)
|
||||
@ -194,6 +190,16 @@ function subscribe(onChange: () => void) {
|
||||
return () => window.removeEventListener(CHANGE_EVENT, onChange);
|
||||
}
|
||||
|
||||
// At capacity, evict the oldest tabs instead of refusing the new one.
|
||||
// Exported for unit tests.
|
||||
export function appendTabWithEviction(
|
||||
tabs: SearchTab[],
|
||||
next: SearchTab,
|
||||
): SearchTab[] {
|
||||
const kept = tabs.slice(Math.max(0, tabs.length - SEARCH_TABS_LIMIT + 1));
|
||||
return [...kept, next];
|
||||
}
|
||||
|
||||
function generateTabId(): string {
|
||||
if (typeof crypto !== "undefined" && "randomUUID" in crypto) {
|
||||
return crypto.randomUUID();
|
||||
@ -209,22 +215,15 @@ export function useSearchTabs(key: string) {
|
||||
);
|
||||
|
||||
const openTab = useCallback(
|
||||
({ label, input }: OpenTabInput): OpenTabResult => {
|
||||
let result: SearchTab | null = null;
|
||||
let dropped = false;
|
||||
({ label, input }: OpenTabInput) => {
|
||||
update(key, (current) => {
|
||||
const inputKey = tabInputKey(input);
|
||||
const existing = current.tabs.find(
|
||||
(tab) => tabInputKey(tab.input) === inputKey,
|
||||
);
|
||||
if (existing) {
|
||||
result = existing;
|
||||
return { ...current, activeTabId: existing.id };
|
||||
}
|
||||
if (current.tabs.length >= SEARCH_TABS_LIMIT) {
|
||||
dropped = true;
|
||||
return current;
|
||||
}
|
||||
const next: SearchTab = {
|
||||
id: generateTabId(),
|
||||
label,
|
||||
@ -232,13 +231,11 @@ export function useSearchTabs(key: string) {
|
||||
createdAt: Date.now(),
|
||||
viewedAt: null,
|
||||
};
|
||||
result = next;
|
||||
return {
|
||||
tabs: [...current.tabs, next],
|
||||
tabs: appendTabWithEviction(current.tabs, next),
|
||||
activeTabId: next.id,
|
||||
};
|
||||
});
|
||||
return { tab: result, dropped };
|
||||
},
|
||||
[key],
|
||||
);
|
||||
@ -314,12 +311,6 @@ export function useSearchTabs(key: string) {
|
||||
[state.tabs],
|
||||
);
|
||||
|
||||
const canOpenTab = useCallback(
|
||||
(input: SearchTabInput) =>
|
||||
Boolean(findMatchingTab(input)) || state.tabs.length < SEARCH_TABS_LIMIT,
|
||||
[findMatchingTab, state.tabs.length],
|
||||
);
|
||||
|
||||
const activeTab = useMemo(
|
||||
() => state.tabs.find((tab) => tab.id === state.activeTabId) ?? null,
|
||||
[state.activeTabId, state.tabs],
|
||||
@ -329,10 +320,8 @@ export function useSearchTabs(key: string) {
|
||||
activeTab,
|
||||
activeTabId: state.activeTabId,
|
||||
tabs: state.tabs,
|
||||
canOpenTab,
|
||||
closeTab,
|
||||
findMatchingTab,
|
||||
limit: SEARCH_TABS_LIMIT,
|
||||
markTabViewed,
|
||||
openTab,
|
||||
setActiveTab,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user