Skip to content

Commit 3904fba

Browse files
kofkof
committed
feat(redirects): improve UI layout and require non-empty target path
- Move search and action buttons to separate row below Redirects label - Add onAbort handler to clear search field - Fix ProjectNewRedirectPath schema to reject empty strings - Fix delete button by adding index parameter to map callback - Add test coverage for empty path validation
1 parent 56a2641 commit 3904fba

File tree

4 files changed

+126
-90
lines changed

4 files changed

+126
-90
lines changed

‎apps/builder/app/shared/project-settings/section-redirects.test.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,13 +229,12 @@ describe("validateFromPath", () => {
229229
});
230230

231231
describe("validateToPath", () => {
232-
// Note: ProjectNewRedirectPath uses `new URL(data, baseURL)` which accepts
233-
// most strings including empty strings when a base URL is provided.
234-
// Only truly invalid URL characters will fail.
232+
// Note: ProjectNewRedirectPath uses `new URL(data, baseURL)` which is permissive
233+
// for most strings. However, empty strings are explicitly rejected.
235234

236-
test("accepts empty path (becomes base URL)", () => {
235+
test("rejects empty path", () => {
237236
const errors = validateToPath("");
238-
expect(errors).toEqual([]);
237+
expect(errors).toContain("Path is required");
239238
});
240239

241240
test("accepts relative path with spaces (URL-encoded)", () => {

‎apps/builder/app/shared/project-settings/section-redirects.tsx‎

Lines changed: 105 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import {
77
DialogActions,
88
DialogClose,
99
DialogContent,
10-
DialogDescription,
1110
DialogTitle,
1211
Flex,
1312
Grid,
@@ -16,6 +15,8 @@ import {
1615
List,
1716
ListItem,
1817
rawTheme,
18+
ScrollArea,
19+
SearchField,
1920
Select,
2021
SmallIconButton,
2122
Text,
@@ -115,13 +116,23 @@ export const SectionRedirects = () => {
115116
const [toPathErrors, setToPathErrors] = useState<string[]>([]);
116117
const [isImportDialogOpen, setIsImportDialogOpen] = useState(false);
117118
const [isDeleteAllDialogOpen, setIsDeleteAllDialogOpen] = useState(false);
119+
const [searchQuery, setSearchQuery] = useState("");
118120
const pages = useStore($pages);
119121
const existingPaths = getExistingRoutePaths(pages);
120122
const fromPathRef = useRef<HTMLInputElement>(null);
121123

122124
const isValidRedirects =
123125
fromPathErrors.length === 0 && toPathErrors.length === 0;
124126

127+
// Filter redirects based on search query
128+
const filteredRedirects = searchQuery
129+
? redirects.filter(
130+
(redirect) =>
131+
redirect.old.toLowerCase().includes(searchQuery.toLowerCase()) ||
132+
redirect.new.toLowerCase().includes(searchQuery.toLowerCase())
133+
)
134+
: redirects;
135+
125136
// Get all page paths for combobox suggestions
126137
const pagePaths = Array.from(existingPaths).sort();
127138

@@ -220,44 +231,51 @@ export const SectionRedirects = () => {
220231
onImport={handleImportRedirects}
221232
/>
222233

223-
<Grid gap={2} css={sectionSpacing}>
224-
<Flex gap={1} align="center" justify="between">
225-
<Flex gap={1} align="center">
226-
<Text variant="titles">Redirects</Text>
227-
<Tooltip
228-
variant="wrapped"
229-
content={
230-
<Flex direction="column" gap="2">
231-
<Text color="subtle">
232-
Redirect old URLs to new ones so you don't lose traffic or
233-
search engine rankings.
234-
</Text>
235-
<Flex direction="column" gap="1">
236-
<Text variant="regularBold">Supported patterns:</Text>
237-
<Text>/path → Exact match</Text>
238-
<Text>/blog/* → All paths under /blog/</Text>
239-
<Text>/:slug → Dynamic segment</Text>
240-
<Text>/:id? → Optional segment</Text>
241-
</Flex>
234+
<Grid gap={3} css={sectionSpacing}>
235+
<Flex gap={1} align="center">
236+
<Text variant="titles">Redirects</Text>
237+
<Tooltip
238+
variant="wrapped"
239+
content={
240+
<Flex direction="column" gap="2">
241+
<Text>
242+
Redirect old URLs to new ones so you don't lose traffic or
243+
search engine rankings.
244+
</Text>
245+
<Flex direction="column" gap="1">
246+
<Text>Supported patterns:</Text>
247+
<Text>/path → Exact match</Text>
248+
<Text>/blog/* → All paths under /blog/</Text>
249+
<Text>/:slug → Dynamic segment</Text>
250+
<Text>/:id? → Optional segment</Text>
242251
</Flex>
243-
}
252+
</Flex>
253+
}
254+
>
255+
<InfoCircleIcon
256+
color={rawTheme.colors.foregroundSubtle}
257+
tabIndex={0}
258+
/>
259+
</Tooltip>
260+
</Flex>
261+
262+
<Flex gap="2" justify="between">
263+
<SearchField
264+
placeholder="Search"
265+
value={searchQuery}
266+
onChange={(event) => setSearchQuery(event.target.value)}
267+
onAbort={() => setSearchQuery("")}
268+
disabled={redirects.length === 0}
269+
/>
270+
<Flex gap="2">
271+
<Button
272+
color="ghost"
273+
prefix={<TrashIcon />}
274+
onClick={() => setIsDeleteAllDialogOpen(true)}
275+
disabled={redirects.length === 0}
244276
>
245-
<InfoCircleIcon
246-
color={rawTheme.colors.foregroundSubtle}
247-
tabIndex={0}
248-
/>
249-
</Tooltip>
250-
</Flex>
251-
<Flex gap="1">
252-
{redirects.length > 0 && (
253-
<Button
254-
color="ghost"
255-
prefix={<TrashIcon />}
256-
onClick={() => setIsDeleteAllDialogOpen(true)}
257-
>
258-
Delete all
259-
</Button>
260-
)}
277+
Delete all
278+
</Button>
261279
<Button
262280
color="ghost"
263281
prefix={<UploadIcon />}
@@ -274,10 +292,13 @@ export const SectionRedirects = () => {
274292
>
275293
<DialogContent>
276294
<DialogTitle>Delete all redirects</DialogTitle>
277-
<DialogDescription>
278-
Are you sure you want to delete all {redirects.length} redirect
279-
{redirects.length !== 1 ? "s" : ""}? This action cannot be undone.
280-
</DialogDescription>
295+
<Flex css={{ padding: theme.panel.padding }}>
296+
<Text>
297+
Are you sure you want to delete all {redirects.length} redirect
298+
{redirects.length !== 1 ? "s" : ""}? This action cannot be
299+
undone.
300+
</Text>
301+
</Flex>
281302
<DialogActions>
282303
<Button color="destructive" onClick={handleDeleteAll}>
283304
Delete all
@@ -323,8 +344,6 @@ export const SectionRedirects = () => {
323344
}}
324345
/>
325346

326-
<ArrowRightIcon size={16} style={{ flexShrink: 0 }} />
327-
328347
<InputErrorsTooltip
329348
errors={toPathErrors.length > 0 ? toPathErrors : undefined}
330349
side="top"
@@ -358,24 +377,37 @@ export const SectionRedirects = () => {
358377
</Flex>
359378
)}
360379
</Grid>
361-
362380
{redirects.length > 0 ? (
363-
<Grid css={sectionSpacing}>
364-
<List asChild>
365-
<Flex direction="column" gap="1" align="stretch">
366-
{redirects.map((redirect, index) => {
367-
return (
368-
<ListItem asChild key={redirect.old} index={index}>
369-
<Flex
370-
justify="between"
371-
align="center"
372-
gap="2"
373-
css={{
374-
p: theme.spacing[3],
375-
overflow: "hidden",
376-
}}
377-
>
378-
<Flex gap="2" align="center">
381+
<ScrollArea>
382+
<Grid css={sectionSpacing}>
383+
<List asChild>
384+
<Flex direction="column" gap="1" align="stretch">
385+
{filteredRedirects.map((redirect, index) => {
386+
return (
387+
<ListItem asChild key={redirect.old}>
388+
<Grid
389+
align="center"
390+
gap="2"
391+
css={{
392+
p: theme.spacing[3],
393+
overflow: "hidden",
394+
gridTemplateColumns: `1fr auto auto 1fr`,
395+
position: "relative",
396+
"& > button": {
397+
opacity: 0,
398+
position: "absolute",
399+
right: 0,
400+
top: 0,
401+
bottom: 0,
402+
height: "auto",
403+
borderRadius: 0,
404+
background: theme.colors.backgroundPanel,
405+
},
406+
"&:hover > button, &:focus-within > button": {
407+
opacity: 1,
408+
},
409+
}}
410+
>
379411
<Tooltip content={redirect.old}>
380412
<Link
381413
underline="hover"
@@ -384,18 +416,17 @@ export const SectionRedirects = () => {
384416
publishedOrigin
385417
).toString()}
386418
css={{
387-
width: theme.spacing[18],
388419
wordBreak: "break-all",
420+
overflow: "hidden",
421+
textOverflow: "ellipsis",
389422
}}
390423
target="_blank"
391424
>
392425
{redirect.old}
393426
</Link>
394427
</Tooltip>
395428
<Text>{redirect.status ?? "301"}</Text>
396-
<Flex shrink={false} align="center">
397-
<ArrowRightIcon size={16} aria-disabled />
398-
</Flex>
429+
<ArrowRightIcon size={16} />
399430
<Tooltip content={redirect.new}>
400431
<Link
401432
underline="hover"
@@ -405,29 +436,28 @@ export const SectionRedirects = () => {
405436
).toString()}
406437
css={{
407438
wordBreak: "break-all",
408-
maxWidth: theme.spacing[30],
439+
overflow: "hidden",
440+
textOverflow: "ellipsis",
409441
}}
410442
target="_blank"
411443
>
412444
{redirect.new}
413445
</Link>
414446
</Tooltip>
415-
</Flex>
416-
<Flex gap="2">
417447
<SmallIconButton
418448
variant="destructive"
419449
icon={<TrashIcon />}
420450
aria-label={`Delete redirect from ${redirect.old}`}
421451
onClick={() => handleDeleteRedirect(index)}
422452
/>
423-
</Flex>
424-
</Flex>
425-
</ListItem>
426-
);
427-
})}
428-
</Flex>
429-
</List>
430-
</Grid>
453+
</Grid>
454+
</ListItem>
455+
);
456+
})}
457+
</Flex>
458+
</List>
459+
</Grid>
460+
</ScrollArea>
431461
) : null}
432462
</>
433463
);

‎packages/sdk/src/schema/pages.test.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,10 @@ describe("ProjectNewRedirectPath", () => {
210210
expect(ProjectNewRedirectPath.safeParse("/한국어").success).toBe(true);
211211
});
212212

213+
test("rejects empty string", () => {
214+
expect(ProjectNewRedirectPath.safeParse("").success).toBe(false);
215+
});
216+
213217
test("rejects truly invalid URLs", () => {
214218
// Note: ProjectNewRedirectPath uses new URL(data, baseURL) which is very permissive
215219
// It treats most strings as valid relative paths. The only truly invalid inputs

‎packages/sdk/src/schema/pages.ts‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -160,16 +160,19 @@ const ProjectMeta = z.object({
160160
});
161161
export type ProjectMeta = z.infer<typeof ProjectMeta>;
162162

163-
export const ProjectNewRedirectPath = z.string().refine((data) => {
164-
// Users should be able to redirect from any old-path to the home page in the new project.
165-
try {
166-
// can be relative and absolute paths
167-
new URL(data, "http://url.com");
168-
return true;
169-
} catch {
170-
return false;
171-
}
172-
}, "Must be a valid URL");
163+
export const ProjectNewRedirectPath = z
164+
.string()
165+
.min(1, "Path is required")
166+
.refine((data) => {
167+
// Users should be able to redirect from any old-path to the home page in the new project.
168+
try {
169+
// can be relative and absolute paths
170+
new URL(data, "http://url.com");
171+
return true;
172+
} catch {
173+
return false;
174+
}
175+
}, "Must be a valid URL");
173176

174177
export const PageRedirect = z.object({
175178
old: OldPagePath,

0 commit comments

Comments
 (0)