From b1a795fbbd75f160f64e0251dfcfa78d9507f477 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 23:59:34 +0000 Subject: [PATCH 01/40] Show the quick insert tip on every fast default insert The tip was capped at once ever, so the one time it fired was often a slow, deliberate insert. Time it from the menu opening instead: an unchanged insert within 1.5 seconds is one a right-click would have done, and is worth saying every time it happens. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- .../insert/components/insert-menu.tsx | 28 +++++++++++++++---- .../features/insert/open-insert-menu.tsx | 4 +++ .../features/insert/quick-insert-tip.ts | 13 +++++---- src/frontend/lib/ui-state.ts | 4 +-- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/frontend/features/insert/components/insert-menu.tsx b/src/frontend/features/insert/components/insert-menu.tsx index 843e6f1f..9a7a7e58 100644 --- a/src/frontend/features/insert/components/insert-menu.tsx +++ b/src/frontend/features/insert/components/insert-menu.tsx @@ -35,11 +35,13 @@ interface InsertMenuContentProps { /** The modal this renders in, so the header can track the selection. */ modalId: string; defaultConfiguration?: ParameterValues; + /** When the menu opened, for the quick insert tip. */ + openedAt: number; onInsert: () => void; } export function InsertMenuContent(props: InsertMenuContentProps): ReactNode { - const { insertable, modalId, onInsert } = props; + const { insertable, modalId, openedAt, onInsert } = props; const favorites = useFavoritesQuery().data?.favorites; const isSignedIn = useIsSignedIn(); @@ -155,6 +157,7 @@ export function InsertMenuContent(props: InsertMenuContentProps): ReactNode { ) } isFavorite={favorite !== undefined} + openedAt={openedAt} onInsert={onInsert} /> @@ -171,6 +174,8 @@ interface InsertButtonsProps { insertable: InsertableOut; configuration?: ParameterValues; isFavorite: boolean; + /** When the menu opened, for the quick insert tip. */ + openedAt: number; onInsert: () => void; } @@ -178,8 +183,14 @@ interface InsertButtonsProps { * The derive/insert button plus the insert and fasten checkbox. */ function InsertButtons(props: InsertButtonsProps): ReactNode { - const { insertable, configuration, isUnchanged, isFavorite, onInsert } = - props; + const { + insertable, + configuration, + isUnchanged, + isFavorite, + openedAt, + onInsert + } = props; const search = useSearch({ from: "/app" }); // Inserting targets the current Onshape document; there's nothing to insert @@ -202,10 +213,17 @@ function InsertButtons(props: InsertButtonsProps): ReactNode { const handleClick = useCallback(() => { insertMutation.mutate(canFasten && uiState.fasten); if (isUnchanged) { - showQuickInsertTip(); + showQuickInsertTip(openedAt); } onInsert(); - }, [insertMutation, onInsert, canFasten, uiState.fasten, isUnchanged]); + }, [ + insertMutation, + onInsert, + canFasten, + uiState.fasten, + isUnchanged, + openedAt + ]); if (!isConnected) { return null; diff --git a/src/frontend/features/insert/open-insert-menu.tsx b/src/frontend/features/insert/open-insert-menu.tsx index dde5b50e..a62336fe 100644 --- a/src/frontend/features/insert/open-insert-menu.tsx +++ b/src/frontend/features/insert/open-insert-menu.tsx @@ -20,6 +20,9 @@ interface OpenInsertMenuProps { export function openInsertMenu(props: OpenInsertMenuProps) { const { insertable, defaultConfiguration } = props; let didInsert = false; + // How quickly the insert follows is what says whether the menu was worth + // opening, so the quick insert tip is timed from here. + const openedAt = Date.now(); // Minted here so the content can address the modal it lives in, which is // what lets the header follow the selected configuration. const id = crypto.randomUUID(); @@ -37,6 +40,7 @@ export function openInsertMenu(props: OpenInsertMenuProps) { insertable={insertable} modalId={id} defaultConfiguration={defaultConfiguration} + openedAt={openedAt} onInsert={() => { didInsert = true; modals.close(id); diff --git a/src/frontend/features/insert/quick-insert-tip.ts b/src/frontend/features/insert/quick-insert-tip.ts index 07da8a58..10b6588f 100644 --- a/src/frontend/features/insert/quick-insert-tip.ts +++ b/src/frontend/features/insert/quick-insert-tip.ts @@ -1,15 +1,18 @@ import { showInfoToast } from "../../lib/notifications"; -import { getUiState, updateUiState } from "../../lib/ui-state"; + +/** An insert this soon after opening didn't need anything from the menu. */ +const QUICK_INSERT_WINDOW_MS = 1500; /** * After an insert that changed nothing in the menu, points out that a - * right-click would have done it. Once only: the menu is a fine way to work. + * right-click would have done it. Only when the menu was dismissed as fast as + * a right-click: a slower one was spent looking at the part, which the tip + * has no better answer for. */ -export function showQuickInsertTip(): void { - if (getUiState().hasSeenQuickInsertTip) { +export function showQuickInsertTip(openedAt: number): void { + if (Date.now() - openedAt >= QUICK_INSERT_WINDOW_MS) { return; } - updateUiState({ hasSeenQuickInsertTip: true }); showInfoToast( "Tip: right-click a part to insert it without opening the insert menu.", { id: "quick-insert-tip", autoClose: 8000 } diff --git a/src/frontend/lib/ui-state.ts b/src/frontend/lib/ui-state.ts index 147de9e2..9e67fca1 100644 --- a/src/frontend/lib/ui-state.ts +++ b/src/frontend/lib/ui-state.ts @@ -18,9 +18,7 @@ const UiStateSchema = z.object({ openGroupId: z.string().optional(), fasten: z.boolean().default(true), /** The access level to view the app as; absent means the granted default. */ - accessLevel: AccessLevelType.optional(), - /** Whether the quick-insert tip has been shown; it is only worth saying once. */ - hasSeenQuickInsertTip: z.boolean().default(false) + accessLevel: AccessLevelType.optional() }); type UiState = z.infer; From 38d964f44fb8a4fc4264638650f62f5e5182faf2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 00:39:13 +0000 Subject: [PATCH 02/40] Match an inch mark as part of its number, and un-deprecate the icons `1"` tokenized to a bare `1`, which prefix-matches every 1.5", 10-32 and 16T in the library, so the sizes a user asked for were buried. The mark now stays on the number and ends its token, leaving `1"` an exact size while a bare `1` still prefixes them all. Phosphor deprecated the unsuffixed icon names in favor of `*Icon`. The local heart wrappers become FavoriteIcon/UnfavoriteIcon, which is what they meant anyway and leaves HeartIcon to Phosphor. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- src/backend/features/search/search-index.ts | 11 ++- src/frontend/components/alerts.tsx | 8 ++- src/frontend/components/app-menu.tsx | 6 +- src/frontend/components/app-navbar.tsx | 6 +- src/frontend/components/app-title.tsx | 8 +-- src/frontend/components/app-zero-state.tsx | 6 +- src/frontend/components/change-order.tsx | 16 ++--- src/frontend/components/open-url-button.tsx | 4 +- src/frontend/components/root-error.tsx | 4 +- .../build-status/components/build-status.tsx | 32 ++++----- .../favorites/components/favorite-button.tsx | 26 +++---- .../favorites/components/favorite-card.tsx | 4 +- .../favorites/components/favorite-menu.tsx | 8 +-- .../favorites/components/favorites-list.tsx | 4 +- .../features/favorites/open-favorite-menu.tsx | 4 +- .../insert/components/insert-menu.tsx | 6 +- .../library/components/add-group-menu.tsx | 8 +-- .../library/components/card-components.tsx | 24 +++---- .../library/components/group-card.tsx | 15 ++-- .../components/reload-groups-button.tsx | 4 +- .../search/components/search-errors.tsx | 14 ++-- src/frontend/features/search/search.test.ts | 68 ++++++++++++++++++- .../settings/components/vendor-filters.tsx | 8 +-- .../thumbnails/components/thumbnail.tsx | 4 +- src/frontend/lib/notifications.tsx | 8 +-- src/frontend/lib/url.tsx | 4 +- .../library/$libraryId/groups/$groupId.tsx | 12 ++-- .../routes/app/library/$libraryId/index.tsx | 10 +-- 28 files changed, 212 insertions(+), 120 deletions(-) diff --git a/src/backend/features/search/search-index.ts b/src/backend/features/search/search-index.ts index 3d7593db..2fa6572b 100644 --- a/src/backend/features/search/search-index.ts +++ b/src/backend/features/search/search-index.ts @@ -98,9 +98,14 @@ export function normalizeForMatch(text: string): string { export function tokenize(text: string): string[] { // Canonicalize before splitting: fractions span `/` and `-`. Casing stays, // since processTerm splits on camelCase. - return canonicalizeNumbers(text) - .split(/[-()"'#&\s^/]+/) - .filter(Boolean); + return ( + canonicalizeNumbers(text) + // An inch mark stays on its number and ends the token, so `1"` is a + // size of its own rather than a prefix of `1.5`, `10`, and `16t`. + .replace(/(\d")/g, `$1${deliminator}`) + .split(/(? + } title={props.title} /> diff --git a/src/frontend/components/app-menu.tsx b/src/frontend/components/app-menu.tsx index 1c7b69d5..52934994 100644 --- a/src/frontend/components/app-menu.tsx +++ b/src/frontend/components/app-menu.tsx @@ -1,6 +1,6 @@ import { PropsWithChildren, ReactNode } from "react"; import { FloatingPosition, Menu, ActionIcon } from "@mantine/core"; -import { DotsThree } from "@phosphor-icons/react"; +import { DotsThreeIcon } from "@phosphor-icons/react"; import { IconSize } from "../lib/style-constants"; interface AppContextMenuProps { @@ -82,7 +82,9 @@ export function MenuButton(props: MenuButtonProps): ReactNode { title="View options" onClick={(e) => e.stopPropagation()} > - + ); diff --git a/src/frontend/components/app-navbar.tsx b/src/frontend/components/app-navbar.tsx index 56dc8013..70509936 100644 --- a/src/frontend/components/app-navbar.tsx +++ b/src/frontend/components/app-navbar.tsx @@ -10,7 +10,7 @@ import { TextInput, Tooltip } from "@mantine/core"; -import { Gear, MagnifyingGlass } from "@phosphor-icons/react"; +import { GearIcon, MagnifyingGlassIcon } from "@phosphor-icons/react"; import { BORDER, CHROME_BACKGROUND, @@ -213,7 +213,7 @@ export function SettingsButton() { size="lg" onClick={() => openSettingsMenu()} > - + ); } @@ -250,7 +250,7 @@ export function SearchBar() { // The panel opens to a library the caller is here to search. autoFocus flex={1} - leftSection={} + leftSection={} placeholder={`Search ${getLibraryName(libraryId)}...`} ref={ref} value={uiState.searchQuery ?? ""} diff --git a/src/frontend/components/app-title.tsx b/src/frontend/components/app-title.tsx index 337c7a70..aa5741ce 100644 --- a/src/frontend/components/app-title.tsx +++ b/src/frontend/components/app-title.tsx @@ -8,7 +8,7 @@ import { Text, Tooltip } from "@mantine/core"; -import { ArrowSquareOut, Check, Copy } from "@phosphor-icons/react"; +import { ArrowSquareOutIcon, CheckIcon, CopyIcon } from "@phosphor-icons/react"; import type { ReactNode } from "react"; import type { SearchRecord } from "@backend/features/configurations/models"; import { FontWeight, IconSize, TITLE_ICON_NUDGE } from "../lib/style-constants"; @@ -120,9 +120,9 @@ function PartNumber({ onClick={copy} > {copied ? ( - + ) : ( - + )} @@ -149,7 +149,7 @@ function PartNumber({ {partNumber} - + ); } diff --git a/src/frontend/components/app-zero-state.tsx b/src/frontend/components/app-zero-state.tsx index 10a7c441..5e1a2552 100644 --- a/src/frontend/components/app-zero-state.tsx +++ b/src/frontend/components/app-zero-state.tsx @@ -1,9 +1,11 @@ import { Box, Center, EmptyState, Loader } from "@mantine/core"; -import { X } from "@phosphor-icons/react"; +import { XIcon } from "@phosphor-icons/react"; import { IconSize } from "../lib/style-constants"; import { type JSX, ReactNode } from "react"; -const DEFAULT_ERROR_ICON = ; +const DEFAULT_ERROR_ICON = ( + +); interface ZeroStateProps { icon?: ReactNode; diff --git a/src/frontend/components/change-order.tsx b/src/frontend/components/change-order.tsx index 4baea732..ecda85cc 100644 --- a/src/frontend/components/change-order.tsx +++ b/src/frontend/components/change-order.tsx @@ -1,9 +1,9 @@ import { Menu } from "@mantine/core"; import { - CaretDoubleDown, - CaretDoubleUp, - CaretDown, - CaretUp + CaretDoubleDownIcon, + CaretDoubleUpIcon, + CaretDownIcon, + CaretUpIcon } from "@phosphor-icons/react"; import { IconSize } from "../lib/style-constants"; import { type ReactNode } from "react"; @@ -32,7 +32,7 @@ export function ChangeOrderItems(props: ChangeOrderMenuProps): ReactNode { <> {operations.includes(MoveOperation.MOVE_UP) && ( } + leftSection={} onClick={() => { onOrderChange( applyMoveOperation(id, order, MoveOperation.MOVE_UP) @@ -44,7 +44,7 @@ export function ChangeOrderItems(props: ChangeOrderMenuProps): ReactNode { )} {operations.includes(MoveOperation.MOVE_DOWN) && ( } + leftSection={} onClick={() => { onOrderChange( applyMoveOperation( @@ -60,7 +60,7 @@ export function ChangeOrderItems(props: ChangeOrderMenuProps): ReactNode { )} {operations.includes(MoveOperation.MOVE_TO_TOP) && ( } + leftSection={} onClick={() => { onOrderChange( applyMoveOperation( @@ -76,7 +76,7 @@ export function ChangeOrderItems(props: ChangeOrderMenuProps): ReactNode { )} {operations.includes(MoveOperation.MOVE_TO_BOTTOM) && ( } + leftSection={} onClick={() => { onOrderChange( applyMoveOperation( diff --git a/src/frontend/components/open-url-button.tsx b/src/frontend/components/open-url-button.tsx index bc31d142..c52613fc 100644 --- a/src/frontend/components/open-url-button.tsx +++ b/src/frontend/components/open-url-button.tsx @@ -1,5 +1,5 @@ import { Button } from "@mantine/core"; -import { ArrowSquareOut } from "@phosphor-icons/react"; +import { ArrowSquareOutIcon } from "@phosphor-icons/react"; import { IconSize } from "../lib/style-constants"; import { openUrlInNewTab } from "../lib/url"; @@ -11,7 +11,7 @@ interface UrlButtonProps { export function OpenUrlButton(props: UrlButtonProps) { return ( + + + )} ); } From c43d515bbdaded61d31ee2b4ffbbc752405ed876 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 02:17:16 +0000 Subject: [PATCH 08/40] Hold FTCDesignLib behind a coming soon page The tab stays, so the library is announced, but selecting it lands on a zero state instead of an empty library: its groups, favorites and search all render inside the route, so gating there covers them at once, and the loader stops fetching what nothing will show. The search row goes with them, having nothing left to search. Also says why the group-restore latch is there, which a reviewer had to ask: without it, leaving a group redirects straight back into it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- src/frontend/components/app-navbar.tsx | 19 +++++++--- src/frontend/components/app-zero-state.tsx | 12 +++++++ .../library/components/coming-soon.tsx | 24 +++++++++++++ src/frontend/features/library/library-path.ts | 7 +++- .../routes/app/library/$libraryId/route.tsx | 36 +++++++++++++++++-- 5 files changed, 89 insertions(+), 9 deletions(-) create mode 100644 src/frontend/features/library/components/coming-soon.tsx diff --git a/src/frontend/components/app-navbar.tsx b/src/frontend/components/app-navbar.tsx index 70509936..a43f0421 100644 --- a/src/frontend/components/app-navbar.tsx +++ b/src/frontend/components/app-navbar.tsx @@ -24,7 +24,11 @@ import frcDesignBook from "/frc-design-book.svg"; import { openSettingsMenu } from "../features/settings/open-settings-menu"; import { VendorMenu } from "../features/settings/components/vendor-filters"; import { useUiState } from "../lib/ui-state"; -import { getLibraryName, useLibraryId } from "../features/library/library-path"; +import { + getLibraryName, + isComingSoon, + useLibraryId +} from "../features/library/library-path"; import { RequireAccessLevel } from "../features/auth/access-level"; import { useSaveSettings } from "../features/settings/settings"; import { useAccessData } from "../features/auth/access-level"; @@ -39,6 +43,9 @@ import { getLibraryVersionQuery } from "../features/library/queries"; * brand and settings alongside, over a row holding search and its filters. */ export function AppNavbar(): ReactNode { + // Nothing to search until the library opens. + const showSearch = !isComingSoon(useLibraryId()); + return ( {/* Stretched so the tabs run the full height and their underline @@ -59,10 +66,12 @@ export function AppNavbar(): ReactNode { - - - - + {showSearch && ( + + + + + )} ); } diff --git a/src/frontend/components/app-zero-state.tsx b/src/frontend/components/app-zero-state.tsx index 5e1a2552..ca7155e1 100644 --- a/src/frontend/components/app-zero-state.tsx +++ b/src/frontend/components/app-zero-state.tsx @@ -93,6 +93,18 @@ export function SectionError(props: ErrorProps): ReactNode { ); } +interface PageMessageProps extends ZeroStateProps { + /** Pass true to keep the message closer to the top of the page. */ + justifyUp?: boolean; +} + +/** A page-level zero state that is not an error, so it carries no fallback. */ +export function PageMessage(props: PageMessageProps): ReactNode { + const { justifyUp, ...zeroState } = props; + const message = ; + return justifyUp ? message :
{message}
; +} + interface PageErrorProps extends ErrorProps { /** * Pass in true to keep the error closer to the top of the page. diff --git a/src/frontend/features/library/components/coming-soon.tsx b/src/frontend/features/library/components/coming-soon.tsx new file mode 100644 index 00000000..8c884fb2 --- /dev/null +++ b/src/frontend/features/library/components/coming-soon.tsx @@ -0,0 +1,24 @@ +import { Box } from "@mantine/core"; +import { HammerIcon } from "@phosphor-icons/react"; +import { ReactNode } from "react"; +import { IconSize, PrimaryColor } from "../../../lib/style-constants"; +import { PageMessage } from "../../../components/app-zero-state"; +import { getLibraryName, useLibraryId } from "../library-path"; + +/** Stands in for a library that is announced but has nothing to show yet. */ +export function ComingSoon(): ReactNode { + const libraryId = useLibraryId(); + return ( + + } + title={`${getLibraryName(libraryId)} is coming soon`} + description="It is still being put together. Check back soon!" + /> + ); +} diff --git a/src/frontend/features/library/library-path.ts b/src/frontend/features/library/library-path.ts index ee4082d1..6cfd80f5 100644 --- a/src/frontend/features/library/library-path.ts +++ b/src/frontend/features/library/library-path.ts @@ -45,11 +45,16 @@ export function getLibraryName(libraryId: string): string { throw new Error("Unknown library: " + libraryId); } +/** Announced, but with nothing to show yet. */ +export function isComingSoon(libraryId: string): boolean { + return libraryId === LibraryId.FTC_DESIGN_LIB; +} + /** Where a library is in its life; undefined once it is simply supported. */ export function getLibraryStatus(libraryId: string): string | undefined { switch (libraryId) { case LibraryId.FTC_DESIGN_LIB: - return "Beta"; + return "Coming soon"; case LibraryId.MKCAD: return "Deprecated"; } diff --git a/src/frontend/routes/app/library/$libraryId/route.tsx b/src/frontend/routes/app/library/$libraryId/route.tsx index 1dcbdca5..a355bcf4 100644 --- a/src/frontend/routes/app/library/$libraryId/route.tsx +++ b/src/frontend/routes/app/library/$libraryId/route.tsx @@ -1,4 +1,10 @@ -import { createFileRoute, notFound, redirect } from "@tanstack/react-router"; +import { + createFileRoute, + notFound, + Outlet, + redirect +} from "@tanstack/react-router"; +import { ReactNode } from "react"; import { queryClient } from "../../../../lib/query-client"; import { getFavoritesQuery } from "../../../../features/favorites/queries"; import { @@ -8,12 +14,22 @@ import { import { getSearchDbQuery } from "../../../../features/search/queries"; import { LibraryId } from "@backend/features/library/library-id"; import { getUiState } from "../../../../lib/ui-state"; -import { isLibraryId } from "../../../../features/library/library-path"; +import { + isComingSoon, + isLibraryId, + useLibraryId +} from "../../../../features/library/library-path"; +import { ComingSoon } from "../../../../features/library/components/coming-soon"; -/** Restoring the last group is an entry behavior, so it happens once per load. */ +/** + * Restoring the last group is an entry behavior, so it happens once per load. + * The latch is what ends it: leaving a group navigates here, which runs this + * again while `openGroupId` still names the group, and redirects straight back. + */ let restoredGroup = false; export const Route = createFileRoute("/app/library/$libraryId")({ + component: LibraryRoute, params: { // Narrowed by beforeLoad, which 404s an unknown library. parse: ({ libraryId }) => ({ libraryId: libraryId as LibraryId }), @@ -25,6 +41,11 @@ export const Route = createFileRoute("/app/library/$libraryId")({ if (!isLibraryId(params.libraryId)) { throw notFound(); } + // A coming-soon library has no group to land in. + if (isComingSoon(params.libraryId)) { + restoredGroup = true; + return; + } // Client state, so the entry redirect can't restore it. const { openGroupId } = getUiState(); if (openGroupId && !restoredGroup) { @@ -38,6 +59,10 @@ export const Route = createFileRoute("/app/library/$libraryId")({ }, loader: async ({ params }) => { const { libraryId } = params; + // Nothing below is rendered, so nothing below is worth fetching. + if (isComingSoon(libraryId)) { + return; + } // The only awaited fetch: everything below keys its url off the version. const cacheVersion = await queryClient.ensureQueryData( getLibraryVersionQuery(libraryId) @@ -51,3 +76,8 @@ export const Route = createFileRoute("/app/library/$libraryId")({ void queryClient.prefetchQuery(getFavoritesQuery(libraryId)); } }); + +/** One gate for the whole library: its groups and search render inside it. */ +function LibraryRoute(): ReactNode { + return isComingSoon(useLibraryId()) ? : ; +} From e00642e6e0d565a52cadd4f4934564d7ee4d0d8a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 03:25:29 +0000 Subject: [PATCH 09/40] Resume in the last group from user data, not a client latch Where a caller left off was client state, so the entry redirect could not see it and the app had to bounce itself into the group after loading, guarded by a module-level latch against redirecting straight back out. The group now sits in the user row beside the library it belongs to, and entry computes the whole landing url in one query: the join is also the check, so a deleted group, or one left behind by a library switch, lands in the library instead. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- drizzle/0007_resume_group.sql | 9 + drizzle/meta/0007_snapshot.json | 510 ++++++++++++++++++ drizzle/meta/_journal.json | 115 ++-- src/backend/db/schema.ts | 5 +- src/backend/features/entry/routes.test.ts | 46 ++ src/backend/features/entry/routes.ts | 23 +- src/backend/features/settings/routes.test.ts | 21 + src/backend/features/settings/routes.ts | 4 +- src/backend/features/settings/settings.ts | 5 +- .../features/settings/local-settings.ts | 10 +- src/frontend/features/settings/settings.ts | 38 +- src/frontend/lib/ui-state.ts | 1 - .../library/$libraryId/groups/$groupId.tsx | 5 +- .../routes/app/library/$libraryId/index.tsx | 6 +- .../routes/app/library/$libraryId/route.tsx | 30 +- src/frontend/routes/index.tsx | 9 +- 16 files changed, 723 insertions(+), 114 deletions(-) create mode 100644 drizzle/0007_resume_group.sql create mode 100644 drizzle/meta/0007_snapshot.json diff --git a/drizzle/0007_resume_group.sql b/drizzle/0007_resume_group.sql new file mode 100644 index 00000000..8c2dd9bf --- /dev/null +++ b/drizzle/0007_resume_group.sql @@ -0,0 +1,9 @@ +/* + Where a caller resumes moves from the browser to their user row, beside the + library it belongs to, so the entry redirect can compute the whole landing url + rather than the app bouncing itself into the group after it loads. + + Starts null, which is the library itself: nobody's last group is known until + they open one. +*/ +ALTER TABLE `users` ADD `group_id` text; diff --git a/drizzle/meta/0007_snapshot.json b/drizzle/meta/0007_snapshot.json new file mode 100644 index 00000000..6e6d47e7 --- /dev/null +++ b/drizzle/meta/0007_snapshot.json @@ -0,0 +1,510 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "7f0e79d4-3077-4a1f-aeab-4d98a104be46", + "prevId": "2416035c-6b28-4014-a7b3-bdc33f803962", + "tables": { + "configurations": { + "name": "configurations", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "parameters": { + "name": "parameters", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'[]'" + }, + "records": { + "name": "records", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'[]'" + }, + "build_issues": { + "name": "build_issues", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'[]'" + } + }, + "indexes": {}, + "foreignKeys": { + "configurations_id_insertables_id_fk": { + "name": "configurations_id_insertables_id_fk", + "tableFrom": "configurations", + "tableTo": "insertables", + "columnsFrom": ["id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "favorites": { + "name": "favorites", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "user_id": { + "name": "user_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "library_id": { + "name": "library_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "insertable_id": { + "name": "insertable_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "default_configuration": { + "name": "default_configuration", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "sort_order": { + "name": "sort_order", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + } + }, + "indexes": { + "favorites_user_id_library_id_insertable_id_unique": { + "name": "favorites_user_id_library_id_insertable_id_unique", + "columns": ["user_id", "library_id", "insertable_id"], + "isUnique": true + } + }, + "foreignKeys": { + "favorites_user_id_users_id_fk": { + "name": "favorites_user_id_users_id_fk", + "tableFrom": "favorites", + "tableTo": "users", + "columnsFrom": ["user_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "favorites_library_id_libraries_id_fk": { + "name": "favorites_library_id_libraries_id_fk", + "tableFrom": "favorites", + "tableTo": "libraries", + "columnsFrom": ["library_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "favorites_insertable_id_insertables_id_fk": { + "name": "favorites_insertable_id_insertables_id_fk", + "tableFrom": "favorites", + "tableTo": "insertables", + "columnsFrom": ["insertable_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "groups": { + "name": "groups", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "library_id": { + "name": "library_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "document_id": { + "name": "document_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "version_id": { + "name": "version_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "sort_alphabetically": { + "name": "sort_alphabetically", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "sort_order": { + "name": "sort_order", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "small_thumbnail_url": { + "name": "small_thumbnail_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "large_thumbnail_url": { + "name": "large_thumbnail_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "build_issues": { + "name": "build_issues", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'[]'" + }, + "last_loaded_at": { + "name": "last_loaded_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "groups_document_id_library_id_unique": { + "name": "groups_document_id_library_id_unique", + "columns": ["document_id", "library_id"], + "isUnique": true + } + }, + "foreignKeys": { + "groups_library_id_libraries_id_fk": { + "name": "groups_library_id_libraries_id_fk", + "tableFrom": "groups", + "tableTo": "libraries", + "columnsFrom": ["library_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "insertables": { + "name": "insertables", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "element_id": { + "name": "element_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "group_id": { + "name": "group_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "document_id": { + "name": "document_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "library_id": { + "name": "library_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "element_type": { + "name": "element_type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "microversion_id": { + "name": "microversion_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "is_visible": { + "name": "is_visible", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "is_open_composite": { + "name": "is_open_composite", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "supports_fasten": { + "name": "supports_fasten", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "index_configurations": { + "name": "index_configurations", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "version_id": { + "name": "version_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "sort_order": { + "name": "sort_order", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "vendors": { + "name": "vendors", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'[]'" + }, + "small_thumbnail_url": { + "name": "small_thumbnail_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "large_thumbnail_url": { + "name": "large_thumbnail_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "fasten_info": { + "name": "fasten_info", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "part_data": { + "name": "part_data", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "build_issues": { + "name": "build_issues", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'[]'" + }, + "last_loaded_at": { + "name": "last_loaded_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": { + "insertables_group_id_groups_id_fk": { + "name": "insertables_group_id_groups_id_fk", + "tableFrom": "insertables", + "tableTo": "groups", + "columnsFrom": ["group_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "insertables_library_id_libraries_id_fk": { + "name": "insertables_library_id_libraries_id_fk", + "tableFrom": "insertables", + "tableTo": "libraries", + "columnsFrom": ["library_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "libraries": { + "name": "libraries", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "cache_version": { + "name": "cache_version", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "users": { + "name": "users", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "theme": { + "name": "theme", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'system'" + }, + "library_id": { + "name": "library_id", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'frc-design-lib'" + }, + "group_id": { + "name": "group_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": {} + } +} diff --git a/drizzle/meta/_journal.json b/drizzle/meta/_journal.json index 391bafad..6164e539 100644 --- a/drizzle/meta/_journal.json +++ b/drizzle/meta/_journal.json @@ -1,55 +1,62 @@ { - "version": "7", - "dialect": "sqlite", - "entries": [ - { - "idx": 0, - "version": "6", - "when": 1785982272297, - "tag": "0000_init", - "breakpoints": true - }, - { - "idx": 1, - "version": "6", - "when": 1786201159430, - "tag": "0001_sad_arclight", - "breakpoints": true - }, - { - "idx": 2, - "version": "6", - "when": 1786201159431, - "tag": "0002_configuration_records", - "breakpoints": true - }, - { - "idx": 3, - "version": "6", - "when": 1786511719906, - "tag": "0003_drop_search_db", - "breakpoints": true - }, - { - "idx": 4, - "version": "6", - "when": 1786511719907, - "tag": "0004_explicit_thumbnail_urls", - "breakpoints": true - }, - { - "idx": 5, - "version": "6", - "when": 1786511719908, - "tag": "0005_index_configurations", - "breakpoints": true - }, - { - "idx": 6, - "version": "6", - "when": 1787265951428, - "tag": "0006_split_part_data", - "breakpoints": true - } - ] -} \ No newline at end of file + "version": "7", + "dialect": "sqlite", + "entries": [ + { + "idx": 0, + "version": "6", + "when": 1785982272297, + "tag": "0000_init", + "breakpoints": true + }, + { + "idx": 1, + "version": "6", + "when": 1786201159430, + "tag": "0001_sad_arclight", + "breakpoints": true + }, + { + "idx": 2, + "version": "6", + "when": 1786201159431, + "tag": "0002_configuration_records", + "breakpoints": true + }, + { + "idx": 3, + "version": "6", + "when": 1786511719906, + "tag": "0003_drop_search_db", + "breakpoints": true + }, + { + "idx": 4, + "version": "6", + "when": 1786511719907, + "tag": "0004_explicit_thumbnail_urls", + "breakpoints": true + }, + { + "idx": 5, + "version": "6", + "when": 1786511719908, + "tag": "0005_index_configurations", + "breakpoints": true + }, + { + "idx": 6, + "version": "6", + "when": 1787265951428, + "tag": "0006_split_part_data", + "breakpoints": true + }, + { + "idx": 7, + "version": "6", + "when": 1787265951429, + "tag": "0007_resume_group", + "breakpoints": true + } + ] +} diff --git a/src/backend/db/schema.ts b/src/backend/db/schema.ts index 46d6e0ee..851eb6e6 100644 --- a/src/backend/db/schema.ts +++ b/src/backend/db/schema.ts @@ -145,7 +145,10 @@ export const users = sqliteTable("users", { libraryId: text("library_id") .$type() .notNull() - .default(DEFAULT_SETTINGS.libraryId) + .default(DEFAULT_SETTINGS.libraryId), + // The group last opened in that library, which entry resumes in. Null for + // the library itself; a stale one resolves to that, so it is never cleaned. + groupId: text("group_id") }); export const favorites = sqliteTable( diff --git a/src/backend/features/entry/routes.test.ts b/src/backend/features/entry/routes.test.ts index 07a28549..162f12f8 100644 --- a/src/backend/features/entry/routes.test.ts +++ b/src/backend/features/entry/routes.test.ts @@ -5,10 +5,13 @@ import { users } from "../../db/schema"; import { LibraryId } from "../library/library-id"; import { Theme } from "../settings/settings"; import { + TEST_GROUP_ID, TEST_USER_ID, createTestApp, jsonRequest, + TEST_LIBRARY_ID, resetDb, + seedGroup, seedUser } from "../../../__test_utils__"; import { getDb } from "../../db/client"; @@ -84,6 +87,49 @@ describe("GET /init", () => { expect(location.searchParams.get("theme")).toBe(Theme.SYSTEM); }); + /** The library and group a user left off in, as their row records them. */ + async function seedResume(libraryId: LibraryId, groupId: string | null) { + await seedUser(db); + await db + .update(users) + .set({ libraryId, groupId }) + .where(eq(users.id, TEST_USER_ID)); + } + + async function entryPath(): Promise { + const res = await createTestApp().request( + "/init", + jsonRequest("GET"), + env + ); + return new URL(res.headers.get("Location")!, "http://x").pathname; + } + + it("resumes in the group they last opened", async () => { + await seedGroup(db); + await seedResume(TEST_LIBRARY_ID, TEST_GROUP_ID); + + expect(await entryPath()).toBe( + `/app/library/${TEST_LIBRARY_ID}/groups/${TEST_GROUP_ID}` + ); + }); + + // The group is gone, so the caller lands in the library rather than on a + // "group not found" page. + it("falls back to the library when the group has been deleted", async () => { + await seedResume(TEST_LIBRARY_ID, "deleted-group"); + + expect(await entryPath()).toBe(`/app/library/${TEST_LIBRARY_ID}`); + }); + + // Whichever library they switched to, they have not opened a group in it. + it("ignores a group belonging to another library", async () => { + await seedGroup(db); + await seedResume(LibraryId.MKCAD, TEST_GROUP_ID); + + expect(await entryPath()).toBe(`/app/library/${LibraryId.MKCAD}`); + }); + it("never caches the gate's verdict", async () => { const res = await createTestApp().request( "/init", diff --git a/src/backend/features/entry/routes.ts b/src/backend/features/entry/routes.ts index 00f08b83..c49cff17 100644 --- a/src/backend/features/entry/routes.ts +++ b/src/backend/features/entry/routes.ts @@ -2,9 +2,9 @@ * `/init` is where Onshape lands. It gates on auth, then resumes the caller in * the library and theme they last used. */ -import { eq } from "drizzle-orm"; +import { and, eq } from "drizzle-orm"; import { getDb } from "../../db/client"; -import { users } from "../../db/schema"; +import { group, users } from "../../db/schema"; import { cacheMiddleware } from "../../lib/cache"; import { getApp, type AppContext } from "../../lib/context"; import { getSessionCompanyId } from "../auth/session"; @@ -19,9 +19,22 @@ function getRelativeUrl(requestUrl: string) { /** Builds the url the caller resumes at, seeded with the library and theme they last used. */ async function getEntryUrl(c: AppContext): Promise { const db = getDb(c.env.DB); + // The join is the check on the stored group: one deleted, or left behind by + // a library switch, comes back null and lands the caller in the library. const user = await db - .select({ libraryId: users.libraryId, theme: users.theme }) + .select({ + libraryId: users.libraryId, + theme: users.theme, + groupId: group.id + }) .from(users) + .leftJoin( + group, + and( + eq(group.id, users.groupId), + eq(group.libraryId, users.libraryId) + ) + ) .where(eq(users.id, await c.var.getUserId())) .get(); @@ -33,7 +46,9 @@ async function getEntryUrl(c: AppContext): Promise { search.set("theme", user?.theme ?? DEFAULT_SETTINGS.theme); const libraryId = user?.libraryId ?? DEFAULT_SETTINGS.libraryId; - return `/app/library/${libraryId}?${search.toString()}`; + const path = `/app/library/${libraryId}`; + const groupPath = user?.groupId ? `${path}/groups/${user.groupId}` : path; + return `${groupPath}?${search.toString()}`; } export const entryRoutes = getApp(); diff --git a/src/backend/features/settings/routes.test.ts b/src/backend/features/settings/routes.test.ts index 4589bb98..e1bc59a5 100644 --- a/src/backend/features/settings/routes.test.ts +++ b/src/backend/features/settings/routes.test.ts @@ -36,4 +36,25 @@ describe("settings routes", () => { .get(); expect(row?.theme).toBe(Theme.DARK); }); + + it("POST /settings records and clears the open group", async () => { + const app = createTestApp(); + const post = (body: unknown) => + app.request("/api/settings", jsonRequest("POST", body), env); + const storedGroupId = async () => + ( + await db + .select() + .from(users) + .where(eq(users.id, TEST_USER_ID)) + .get() + )?.groupId; + + await post({ groupId: "group-1" }); + expect(await storedGroupId()).toBe("group-1"); + + // Null is leaving the group, which is not the same as saying nothing. + await post({ groupId: null }); + expect(await storedGroupId()).toBeNull(); + }); }); diff --git a/src/backend/features/settings/routes.ts b/src/backend/features/settings/routes.ts index c003eab4..19edb743 100644 --- a/src/backend/features/settings/routes.ts +++ b/src/backend/features/settings/routes.ts @@ -12,7 +12,9 @@ export const settingsRoutes = getApp(); const settingsBody = z.object({ theme: z.enum(Theme).optional(), - libraryId: z.enum(LibraryId).optional() + libraryId: z.enum(LibraryId).optional(), + // Null on leaving a group: the caller resumes in the library itself. + groupId: z.string().nullable().optional() }); /** POST /api/settings — update the caller's stored settings */ diff --git a/src/backend/features/settings/settings.ts b/src/backend/features/settings/settings.ts index c30a65ab..e67949ba 100644 --- a/src/backend/features/settings/settings.ts +++ b/src/backend/features/settings/settings.ts @@ -11,11 +11,14 @@ export interface Settings { theme: Theme; /** The library the caller last opened, and lands in next time. */ libraryId: LibraryId; + /** The group they last opened in it; null for the library itself. */ + groupId: string | null; } export type SettingsUpdate = Partial; export const DEFAULT_SETTINGS: Settings = { theme: Theme.SYSTEM, - libraryId: LibraryId.FRC_DESIGN_LIB + libraryId: LibraryId.FRC_DESIGN_LIB, + groupId: null }; diff --git a/src/frontend/features/settings/local-settings.ts b/src/frontend/features/settings/local-settings.ts index 2968c7bf..524ea756 100644 --- a/src/frontend/features/settings/local-settings.ts +++ b/src/frontend/features/settings/local-settings.ts @@ -1,8 +1,7 @@ -import { type LibraryId } from "@backend/features/library/library-id"; import { DEFAULT_SETTINGS, - type SettingsUpdate, - type Theme + type Settings, + type SettingsUpdate } from "@backend/features/settings/settings"; const SETTINGS_STORAGE_KEY = "frc-design-app-settings"; @@ -17,11 +16,12 @@ function readStored(): SettingsUpdate { } /** Locally-persisted settings (used when not signed in), with defaults filled. */ -export function readLocalSettings(): { theme: Theme; libraryId: LibraryId } { +export function readLocalSettings(): Settings { const stored = readStored(); return { theme: stored.theme ?? DEFAULT_SETTINGS.theme, - libraryId: stored.libraryId ?? DEFAULT_SETTINGS.libraryId + libraryId: stored.libraryId ?? DEFAULT_SETTINGS.libraryId, + groupId: stored.groupId ?? DEFAULT_SETTINGS.groupId }; } diff --git a/src/frontend/features/settings/settings.ts b/src/frontend/features/settings/settings.ts index f5e75733..a2217a67 100644 --- a/src/frontend/features/settings/settings.ts +++ b/src/frontend/features/settings/settings.ts @@ -6,22 +6,24 @@ import { getAccessDataQuery } from "../auth/access-level"; import { queryClient } from "../../lib/query-client"; import { writeLocalSettings } from "./local-settings"; +async function saveSettings(newSettings: SettingsUpdate): Promise { + // Resolved here rather than read off a render: a placeholder that says + // signed out would silently persist locally for a user who has a + // server-side row. + const { signedIn } = + await queryClient.ensureQueryData(getAccessDataQuery()); + // Not signed in: no server-side user row; persist locally instead. + if (!signedIn) { + writeLocalSettings(newSettings); + return; + } + await apiPost("/settings", { body: newSettings }); +} + export function useSaveSettings() { const { mutate } = useMutation({ mutationKey: ["settings"], - mutationFn: async (newSettings: SettingsUpdate) => { - // Resolved here rather than read off a render: a placeholder that - // says signed out would silently persist locally for a user who - // has a server-side row. - const { signedIn } = - await queryClient.ensureQueryData(getAccessDataQuery()); - // Not signed in: no server-side user row; persist locally instead. - if (!signedIn) { - writeLocalSettings(newSettings); - return; - } - return apiPost("/settings", { body: newSettings }); - }, + mutationFn: saveSettings, onError: () => { showErrorToast("Unexpectedly failed to update settings."); } @@ -29,3 +31,13 @@ export function useSaveSettings() { return mutate; } + +/** + * Records where the caller is, for the entry redirect to resume at. Called from + * a route rather than a component, so it cannot be the mutation above. + */ +export function rememberOpenGroup(groupId: string | null): void { + void saveSettings({ groupId }).catch(() => { + // Resuming in the library instead of the group is not worth a toast. + }); +} diff --git a/src/frontend/lib/ui-state.ts b/src/frontend/lib/ui-state.ts index 9e67fca1..3252b55d 100644 --- a/src/frontend/lib/ui-state.ts +++ b/src/frontend/lib/ui-state.ts @@ -15,7 +15,6 @@ const UiStateSchema = z.object({ isLibraryOpen: z.boolean().default(true), vendorFilters: z.array(VendorType).optional(), searchQuery: z.string().default(""), - openGroupId: z.string().optional(), fasten: z.boolean().default(true), /** The access level to view the app as; absent means the granted default. */ accessLevel: AccessLevelType.optional() diff --git a/src/frontend/routes/app/library/$libraryId/groups/$groupId.tsx b/src/frontend/routes/app/library/$libraryId/groups/$groupId.tsx index 648904e9..1929bb15 100644 --- a/src/frontend/routes/app/library/$libraryId/groups/$groupId.tsx +++ b/src/frontend/routes/app/library/$libraryId/groups/$groupId.tsx @@ -35,13 +35,14 @@ import { import { ClearFiltersButton } from "../../../../../features/settings/components/vendor-filters"; import { useLibraryQuery } from "../../../../../features/library/queries"; import { useLibraryId } from "../../../../../features/library/library-path"; -import { useUiState, updateUiState } from "../../../../../lib/ui-state"; +import { useUiState } from "../../../../../lib/ui-state"; +import { rememberOpenGroup } from "../../../../../features/settings/settings"; export const Route = createFileRoute("/app/library/$libraryId/groups/$groupId")( { component: GroupList, onEnter: (match) => { - updateUiState({ openGroupId: match.params.groupId }); + rememberOpenGroup(match.params.groupId); } } ); diff --git a/src/frontend/routes/app/library/$libraryId/index.tsx b/src/frontend/routes/app/library/$libraryId/index.tsx index 1a5ed140..d1e3c914 100644 --- a/src/frontend/routes/app/library/$libraryId/index.tsx +++ b/src/frontend/routes/app/library/$libraryId/index.tsx @@ -27,13 +27,15 @@ import { getLibraryStatus, useLibraryId } from "../../../../features/library/library-path"; -import { updateUiState, useUiState } from "../../../../lib/ui-state"; +import { useUiState } from "../../../../lib/ui-state"; +import { rememberOpenGroup } from "../../../../features/settings/settings"; import { useIsSignedIn } from "../../../../features/auth/access-level"; export const Route = createFileRoute("/app/library/$libraryId/")({ component: HomeList, + // Back in the library itself, which is where entry should resume. onEnter: () => { - updateUiState({ openGroupId: undefined }); + rememberOpenGroup(null); } }); diff --git a/src/frontend/routes/app/library/$libraryId/route.tsx b/src/frontend/routes/app/library/$libraryId/route.tsx index a355bcf4..cd34990c 100644 --- a/src/frontend/routes/app/library/$libraryId/route.tsx +++ b/src/frontend/routes/app/library/$libraryId/route.tsx @@ -1,9 +1,4 @@ -import { - createFileRoute, - notFound, - Outlet, - redirect -} from "@tanstack/react-router"; +import { createFileRoute, notFound, Outlet } from "@tanstack/react-router"; import { ReactNode } from "react"; import { queryClient } from "../../../../lib/query-client"; import { getFavoritesQuery } from "../../../../features/favorites/queries"; @@ -13,7 +8,6 @@ import { } from "../../../../features/library/queries"; import { getSearchDbQuery } from "../../../../features/search/queries"; import { LibraryId } from "@backend/features/library/library-id"; -import { getUiState } from "../../../../lib/ui-state"; import { isComingSoon, isLibraryId, @@ -21,13 +15,6 @@ import { } from "../../../../features/library/library-path"; import { ComingSoon } from "../../../../features/library/components/coming-soon"; -/** - * Restoring the last group is an entry behavior, so it happens once per load. - * The latch is what ends it: leaving a group navigates here, which runs this - * again while `openGroupId` still names the group, and redirects straight back. - */ -let restoredGroup = false; - export const Route = createFileRoute("/app/library/$libraryId")({ component: LibraryRoute, params: { @@ -41,21 +28,6 @@ export const Route = createFileRoute("/app/library/$libraryId")({ if (!isLibraryId(params.libraryId)) { throw notFound(); } - // A coming-soon library has no group to land in. - if (isComingSoon(params.libraryId)) { - restoredGroup = true; - return; - } - // Client state, so the entry redirect can't restore it. - const { openGroupId } = getUiState(); - if (openGroupId && !restoredGroup) { - restoredGroup = true; - throw redirect({ - to: "/app/library/$libraryId/groups/$groupId", - params: { libraryId: params.libraryId, groupId: openGroupId } - }); - } - restoredGroup = true; }, loader: async ({ params }) => { const { libraryId } = params; diff --git a/src/frontend/routes/index.tsx b/src/frontend/routes/index.tsx index bcf09fe5..92ca6712 100644 --- a/src/frontend/routes/index.tsx +++ b/src/frontend/routes/index.tsx @@ -6,7 +6,14 @@ import { RootAppError } from "../components/root-error"; // is handled server-side, so it never reaches this route. export const Route = createFileRoute("/")({ beforeLoad: () => { - const { libraryId, theme } = readLocalSettings(); + const { libraryId, theme, groupId } = readLocalSettings(); + if (groupId) { + throw redirect({ + to: "/app/library/$libraryId/groups/$groupId", + params: { libraryId, groupId }, + search: { theme } + }); + } throw redirect({ to: "/app/library/$libraryId", params: { libraryId }, From 035e3a9da67fed59419355d35e2bf110c7e6b547 Mon Sep 17 00:00:00 2001 From: Alex Kempen Date: Sun, 30 Aug 2026 22:44:52 -0500 Subject: [PATCH 10/40] Remove inch handling --- src/backend/features/search/search-index.ts | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/backend/features/search/search-index.ts b/src/backend/features/search/search-index.ts index 1ccbead4..f0167ead 100644 --- a/src/backend/features/search/search-index.ts +++ b/src/backend/features/search/search-index.ts @@ -87,31 +87,19 @@ function canonicalizeNumbers(text: string): string { ); } -/** - * A size's unit, in the spellings a part name uses. The mark is the form it - * keeps, so `1 in`, `1in` and `1"` are one size. A trailing `-` or letter means - * a word that merely starts with "in" (`in-line`, `insert`). - */ -const INCH_UNIT = /(\d)\s*(?:"|inches|inch|in)(?![\w-])/gi; - -/** Numbers and their units in the one spelling everything is stored as. */ -function canonicalize(text: string): string { - return canonicalizeNumbers(text).replace(INCH_UNIT, '$1"'); -} - /** * For direct, non-tokenized comparison: the index's canonicalization, * lowercased, so a `.5` query lines up with a stored `"1/2 Bearing"`. */ export function normalizeForMatch(text: string): string { - return canonicalize(text).toLowerCase(); + return canonicalizeNumbers(text).toLowerCase(); } export function tokenize(text: string): string[] { // Canonicalize before splitting: fractions span `/` and `-`. Casing stays, // since processTerm splits on camelCase. return ( - canonicalize(text) + canonicalizeNumbers(text) // An inch mark stays on its number and ends the token, so `1"` is a // size of its own rather than a prefix of `1.5`, `10`, and `16t`. .replace(/(\d")/g, `$1${deliminator}`) From 42e1f1b2a16bd42e187c8c2e64cb4a4e6dd12775 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 04:08:04 +0000 Subject: [PATCH 11/40] Make ui state the app's own store, and fold the selects in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six review notes, all pulling the same way — one local store the app reads from, instead of state spread across a second localStorage key, the url, and a hook returning a pair: - ui-state gains useGetUiState/useSetUiState, so a component reaches for the half it wants instead of indexing a tuple. - local-settings is gone: the caller's settings live in ui-state, which is now the source of truth for them. Saving still writes the row when they are signed in, which is what a browser running the app for the first time, and the Onshape launch, start from. - The theme comes off ui-state rather than the url, so a standalone visit with no parameters still renders in it. The entry redirect seeds the account's theme once; /app takes it and drops the parameter, and the retained set is now the launch parameters by name. - A sign-in leaves a flag in ui-state rather than a url marker, and returns to the entry point, which confirms it and resumes in one place. - AppSelect and its option helpers had one caller, so they collapse into a SettingSelect beside it: values in, capitalized labels out, no memos. - CHROME_BACKGROUND is jargon; it is the background of the bars framing the page, so FRAME_BACKGROUND. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- src/frontend/components/app-modal.tsx | 6 +- src/frontend/components/app-navbar.tsx | 9 +- src/frontend/components/app-select.tsx | 34 ---- src/frontend/components/open-app-modal.tsx | 6 +- src/frontend/components/select-utils.ts | 33 ---- src/frontend/features/auth/access-level.tsx | 4 +- src/frontend/features/auth/sign-in.ts | 33 +--- .../favorites/components/favorite-card.tsx | 4 +- .../favorites/components/favorites-list.tsx | 6 +- .../insert/components/insert-menu.tsx | 7 +- .../settings/components/settings-menu.tsx | 173 ++++++++---------- .../settings/components/vendor-filters.tsx | 8 +- .../features/settings/local-settings.ts | 36 ---- src/frontend/features/settings/settings.ts | 13 +- src/frontend/lib/onshape-params.ts | 7 +- src/frontend/lib/style-constants.ts | 5 +- src/frontend/lib/ui-state.ts | 31 +++- src/frontend/routes/__root.tsx | 12 +- .../library/$libraryId/groups/$groupId.tsx | 6 +- .../routes/app/library/$libraryId/index.tsx | 5 +- src/frontend/routes/app/route.tsx | 31 +++- src/frontend/routes/index.tsx | 23 ++- src/frontend/theme.ts | 2 +- 23 files changed, 205 insertions(+), 289 deletions(-) delete mode 100644 src/frontend/components/app-select.tsx delete mode 100644 src/frontend/components/select-utils.ts delete mode 100644 src/frontend/features/settings/local-settings.ts diff --git a/src/frontend/components/app-modal.tsx b/src/frontend/components/app-modal.tsx index f19a9a37..223bcc2e 100644 --- a/src/frontend/components/app-modal.tsx +++ b/src/frontend/components/app-modal.tsx @@ -1,13 +1,13 @@ import { Group, type MantineSpacing, Stack } from "@mantine/core"; import { PropsWithChildren, ReactNode } from "react"; -import { BORDER, CHROME_BACKGROUND } from "../lib/style-constants"; +import { BORDER, FRAME_BACKGROUND } from "../lib/style-constants"; interface AppModalBodyProps extends PropsWithChildren { /** Space between children; content that spaces itself should pass 0. */ gap?: MantineSpacing; } -/** A modal's content, padded away from the chrome framing it. */ +/** A modal's content, padded away from the header and footer framing it. */ export function AppModalBody(props: AppModalBodyProps): ReactNode { return ( @@ -23,7 +23,7 @@ export function AppModalFooter(props: PropsWithChildren): ReactNode { justify="space-between" wrap="nowrap" p="sm" - bg={CHROME_BACKGROUND} + bg={FRAME_BACKGROUND} style={{ borderTop: BORDER }} > {props.children} diff --git a/src/frontend/components/app-navbar.tsx b/src/frontend/components/app-navbar.tsx index a43f0421..b26dd1c9 100644 --- a/src/frontend/components/app-navbar.tsx +++ b/src/frontend/components/app-navbar.tsx @@ -13,7 +13,7 @@ import { import { GearIcon, MagnifyingGlassIcon } from "@phosphor-icons/react"; import { BORDER, - CHROME_BACKGROUND, + FRAME_BACKGROUND, IconSize, PrimaryColor } from "../lib/style-constants"; @@ -23,7 +23,7 @@ import { useNavigate } from "@tanstack/react-router"; import frcDesignBook from "/frc-design-book.svg"; import { openSettingsMenu } from "../features/settings/open-settings-menu"; import { VendorMenu } from "../features/settings/components/vendor-filters"; -import { useUiState } from "../lib/ui-state"; +import { useGetUiState, useSetUiState } from "../lib/ui-state"; import { getLibraryName, isComingSoon, @@ -55,7 +55,7 @@ export function AppNavbar(): ReactNode { px="sm" wrap="nowrap" align="stretch" - bg={CHROME_BACKGROUND} + bg={FRAME_BACKGROUND} style={{ borderBottom: BORDER }} > @@ -238,7 +238,8 @@ function selectAllInputText(ref: RefObject) { export function SearchBar() { const ref = useRef(null); - const [uiState, setUiState] = useUiState(); + const uiState = useGetUiState(); + const setUiState = useSetUiState(); const libraryId = useLibraryId(); const clearButton = uiState.searchQuery ? ( diff --git a/src/frontend/components/app-select.tsx b/src/frontend/components/app-select.tsx deleted file mode 100644 index 14e9d398..00000000 --- a/src/frontend/components/app-select.tsx +++ /dev/null @@ -1,34 +0,0 @@ -import { Select } from "@mantine/core"; -import { Dispatch, ReactNode } from "react"; -import { SelectOption } from "./select-utils"; - -interface AppSelectProps { - option: SelectOption; - /** - * A list of options to choose from. - * Should be wrapped in a useMemo to ensure stability. - */ - options: SelectOption[]; - label: string; - onSelect: Dispatch; -} - -export function AppSelect(props: AppSelectProps): ReactNode { - const { option, options, onSelect, label } = props; - - return ( - ({ + value: option, + label: capitalize(option) + }))} + value={value} + allowDeselect={false} + checkIconPosition="right" + comboboxProps={{ withinPortal: true }} + onChange={(selected) => { + if (selected !== null) { + onSelect(selected); + } + }} + /> + ); +} + +export function SettingsMenuContent(): ReactNode { + const { maxAccessLevel } = useAccessData(); return ( <> - {adminSettings} + {/* Unlike all other checks, this one uses maxAccessLevel so you can + still switch back up from user to admin. */} + {hasEditorAccess(maxAccessLevel) && ( + <> + + Admin Settings + + + + + )} ); } function UserSettings(): ReactNode { - // The modal renders at the root, outside the route matches, so navigate by - // exact path: a bare navigate would resolve to the route `from` names. - const location = useRouterState({ select: (state) => state.location }); - const navigate = useNavigate(); - const saveSettings = useSaveSettings(); const libraryId = useLibraryId(); const isConnected = useIsConnectedToOnshape(); - const theme = location.search.theme ?? DEFAULT_SETTINGS.theme; return ( <> - { - // The url renders it; the write-behind decides what the - // entry redirect seeds next time. - saveSettings({ theme }); - void navigate({ - to: location.pathname, - search: (prev) => ({ ...prev, theme }) - }); - }} - /> + {/* Only worth offering from inside Onshape's panel, which is what the standalone app is roomier than. */} {isConnected && ( )} @@ -131,34 +136,23 @@ function UserSettings(): ReactNode { /** * The app's own url for the current library, free of the params Onshape - * launches it with — carrying those over is what would keep it embedded. + * launches it with — carrying those over is what would keep it embedded. The + * settings come along on their own, being the same browser's. */ -function standaloneUrl(libraryId: LibraryId, theme: Theme): string { - const url = new URL(`/app/library/${libraryId}`, window.location.origin); - url.searchParams.set("theme", theme); - return url.toString(); +function standaloneUrl(libraryId: LibraryId): string { + return new URL(`/app/library/${libraryId}`, window.location.origin).href; } -interface ThemeSelectProps { - theme: Theme; - onThemeSelect: Dispatch; -} - -function ThemeSelect(props: ThemeSelectProps): ReactNode { - const { theme, onThemeSelect } = props; - - // Use a memo to stabilize access levels so Select's activeItem tracks properly between renders - const themes = useSelectOptions( - [Theme.SYSTEM, Theme.DARK, Theme.LIGHT], - capitalize - ); +function ThemeSelect(): ReactNode { + const theme = useGetUiState().theme; + const saveSettings = useSaveSettings(); return ( - onThemeSelect(value as Theme)} + value={theme ?? DEFAULT_SETTINGS.theme} + options={[Theme.SYSTEM, Theme.DARK, Theme.LIGHT]} + onSelect={(theme) => saveSettings({ theme })} /> ); } @@ -181,32 +175,19 @@ function AdminSettings(): ReactNode { } function AccessLevelSelect(): ReactNode { - const accessData = useAccessData(); - const setUiState = useUiState()[1]; - - const { maxAccessLevel, currentAccessLevel } = accessData; - // Use a memo to stabilize access levels so Select's activeItem tracks properly between renders - const accessLevels = useSelectOptions( - useMemo( - () => - [ - AccessLevel.ADMIN, - AccessLevel.EDITOR, - AccessLevel.USER - ].filter((level) => isWithinAccessLevel(level, maxAccessLevel)), - [maxAccessLevel] - ), - capitalize - ); + const { maxAccessLevel, currentAccessLevel } = useAccessData(); + const setUiState = useSetUiState(); return ( - { - setUiState({ accessLevel: value as AccessLevel }); - }} + value={currentAccessLevel} + options={[ + AccessLevel.ADMIN, + AccessLevel.EDITOR, + AccessLevel.USER + ].filter((level) => isWithinAccessLevel(level, maxAccessLevel))} + onSelect={(accessLevel) => setUiState({ accessLevel })} /> ); } diff --git a/src/frontend/features/settings/components/vendor-filters.tsx b/src/frontend/features/settings/components/vendor-filters.tsx index 2b77f3de..80dddca6 100644 --- a/src/frontend/features/settings/components/vendor-filters.tsx +++ b/src/frontend/features/settings/components/vendor-filters.tsx @@ -4,7 +4,7 @@ import { IconSize } from "../../../lib/style-constants"; import { ReactNode } from "react"; import { getVendorName } from "@backend/features/library/vendors"; import { Vendor } from "@backend/features/library/vendors"; -import { useUiState } from "../../../lib/ui-state"; +import { useGetUiState, useSetUiState } from "../../../lib/ui-state"; import { AppContextMenu } from "../../../components/app-menu"; interface ClearFiltersButtonProps { @@ -19,7 +19,8 @@ interface ClearFiltersButtonProps { } export function ClearFiltersButton(props: ClearFiltersButtonProps): ReactNode { - const [uiState, setUiState] = useUiState(); + const uiState = useGetUiState(); + const setUiState = useSetUiState(); const text = props.text ?? "Clear filters"; const small = props.small ?? false; @@ -46,7 +47,8 @@ export function ClearFiltersButton(props: ClearFiltersButtonProps): ReactNode { * vendor checkbox items. `undefined` filters mean "all vendors active". */ export function VendorMenu(): ReactNode { - const [uiState, setUiState] = useUiState(); + const uiState = useGetUiState(); + const setUiState = useSetUiState(); const hasFilters = uiState.vendorFilters !== undefined; const menuItems = ( diff --git a/src/frontend/features/settings/local-settings.ts b/src/frontend/features/settings/local-settings.ts deleted file mode 100644 index 524ea756..00000000 --- a/src/frontend/features/settings/local-settings.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { - DEFAULT_SETTINGS, - type Settings, - type SettingsUpdate -} from "@backend/features/settings/settings"; - -const SETTINGS_STORAGE_KEY = "frc-design-app-settings"; - -function readStored(): SettingsUpdate { - try { - const raw = localStorage.getItem(SETTINGS_STORAGE_KEY); - return raw ? (JSON.parse(raw) as SettingsUpdate) : {}; - } catch { - return {}; - } -} - -/** Locally-persisted settings (used when not signed in), with defaults filled. */ -export function readLocalSettings(): Settings { - const stored = readStored(); - return { - theme: stored.theme ?? DEFAULT_SETTINGS.theme, - libraryId: stored.libraryId ?? DEFAULT_SETTINGS.libraryId, - groupId: stored.groupId ?? DEFAULT_SETTINGS.groupId - }; -} - -/** Merges and persists settings locally, used when not signed in. */ -export function writeLocalSettings(newSettings: SettingsUpdate): void { - try { - const merged = { ...readStored(), ...newSettings }; - localStorage.setItem(SETTINGS_STORAGE_KEY, JSON.stringify(merged)); - } catch { - // Ignore storage failures (e.g. private browsing). - } -} diff --git a/src/frontend/features/settings/settings.ts b/src/frontend/features/settings/settings.ts index a2217a67..656fa777 100644 --- a/src/frontend/features/settings/settings.ts +++ b/src/frontend/features/settings/settings.ts @@ -4,17 +4,20 @@ import { showErrorToast } from "../../lib/notifications"; import { apiPost } from "../../lib/api-client"; import { getAccessDataQuery } from "../auth/access-level"; import { queryClient } from "../../lib/query-client"; -import { writeLocalSettings } from "./local-settings"; +import { updateUiState } from "../../lib/ui-state"; +/** + * Applies a setting locally, where the app reads it from, and saves it to the + * caller's row when they have one — which is what a browser running the app for + * the first time, and the Onshape launch, start from. + */ async function saveSettings(newSettings: SettingsUpdate): Promise { + updateUiState(newSettings); // Resolved here rather than read off a render: a placeholder that says - // signed out would silently persist locally for a user who has a - // server-side row. + // signed out would skip the save for a user who has a server-side row. const { signedIn } = await queryClient.ensureQueryData(getAccessDataQuery()); - // Not signed in: no server-side user row; persist locally instead. if (!signedIn) { - writeLocalSettings(newSettings); return; } await apiPost("/settings", { body: newSettings }); diff --git a/src/frontend/lib/onshape-params.ts b/src/frontend/lib/onshape-params.ts index ea986e35..a3b0297a 100644 --- a/src/frontend/lib/onshape-params.ts +++ b/src/frontend/lib/onshape-params.ts @@ -10,11 +10,10 @@ export interface OnshapeParams extends ElementPath { elementType: ElementType; /** The color scheme Onshape is using, forwarded by the entry redirect. */ systemTheme: ColorTheme; - /** The caller's saved theme, seeded by the entry redirect. */ - theme: Theme; + /** The account's saved theme, seeded by the entry redirect and then taken + * into ui-state, which is where the app reads it from. */ + theme?: Theme; server: string; - /** Set on the sign-in redirect so the app confirms success once. */ - justSignedIn?: string; } /** diff --git a/src/frontend/lib/style-constants.ts b/src/frontend/lib/style-constants.ts index 564eb5e0..c6103519 100644 --- a/src/frontend/lib/style-constants.ts +++ b/src/frontend/lib/style-constants.ts @@ -25,8 +25,9 @@ export enum FontWeight { export const BORDER = "1px solid var(--mantine-color-default-border)"; -/** A step off the page: the navbar's tab row, a modal's header and footer. */ -export const CHROME_BACKGROUND = +/** A step off the page, for the bars framing it: the navbar's tab row, a + * modal's header and footer. */ +export const FRAME_BACKGROUND = "light-dark(var(--mantine-color-gray-2), var(--mantine-color-dark-8))"; /** diff --git a/src/frontend/lib/ui-state.ts b/src/frontend/lib/ui-state.ts index 3252b55d..61da90b4 100644 --- a/src/frontend/lib/ui-state.ts +++ b/src/frontend/lib/ui-state.ts @@ -1,13 +1,17 @@ import { useSyncExternalStore } from "react"; import * as z from "zod"; import { AccessLevel } from "@backend/features/auth/access-level"; +import { LibraryId } from "@backend/features/library/library-id"; import { Vendor } from "@backend/features/library/vendors"; +import { DEFAULT_SETTINGS, Theme } from "@backend/features/settings/settings"; // Increment this when a breaking change is made to the schema const LATEST_VERSION = 3; const VendorType = z.enum(Object.values(Vendor)); const AccessLevelType = z.enum(Object.values(AccessLevel)); +const ThemeType = z.enum(Object.values(Theme)); +const LibraryIdType = z.enum(Object.values(LibraryId)); const UiStateSchema = z.object({ version: z.number().default(1), // We can't default the parsed version to LATEST_VERSION because of old versions floating around @@ -17,10 +21,19 @@ const UiStateSchema = z.object({ searchQuery: z.string().default(""), fasten: z.boolean().default(true), /** The access level to view the app as; absent means the granted default. */ - accessLevel: AccessLevelType.optional() + accessLevel: AccessLevelType.optional(), + /** Set on leaving for Onshape, so the app can confirm the sign-in on return. */ + justSignedIn: z.boolean().default(false), + // The caller's settings, which this is the source of truth for. A signed-in + // caller also has them server-side, which is what a browser that has never + // run the app starts from. + theme: ThemeType.default(DEFAULT_SETTINGS.theme), + libraryId: LibraryIdType.default(DEFAULT_SETTINGS.libraryId), + /** The group last opened in that library; null for the library itself. */ + groupId: z.string().nullable().default(DEFAULT_SETTINGS.groupId) }); -type UiState = z.infer; +export type UiState = z.infer; type Subscriber = () => void; @@ -108,9 +121,15 @@ export function updateUiState(partialState: Partial): UiState { export type SetUiState = (uiState: Partial) => void; -export function useUiState(): [UiState, SetUiState] { - // Create a react version of the state to trigger re-renders - const reactUiState = useSyncExternalStore(subscribeToUiState, getUiState); +/** The current state, re-rendering the caller whenever it changes. */ +export function useGetUiState(): UiState { + return useSyncExternalStore(subscribeToUiState, getUiState); +} - return [reactUiState, updateUiState]; +/** Merges into the state; every reader of it re-renders. */ +// The setter half of the pair above: a component reaches for one or the other, +// so both read as hooks even though setting needs no state of its own. +// eslint-disable-next-line react-x/no-unnecessary-use-prefix +export function useSetUiState(): SetUiState { + return updateUiState; } diff --git a/src/frontend/routes/__root.tsx b/src/frontend/routes/__root.tsx index 110c7d8d..b9b00e28 100644 --- a/src/frontend/routes/__root.tsx +++ b/src/frontend/routes/__root.tsx @@ -13,7 +13,7 @@ import { useColorScheme } from "@mantine/hooks"; import { queryClient } from "../lib/query-client"; import { createAppTheme } from "../theme"; import { getColorTheme } from "../lib/onshape-params"; -import { DEFAULT_SETTINGS } from "@backend/features/settings/settings"; +import { useGetUiState } from "../lib/ui-state"; import { NotFoundError, RootCrash } from "../components/root-error"; export const Route = createRootRoute({ @@ -26,21 +26,21 @@ export const Route = createRootRoute({ }); function RootComponent(): ReactNode { - // Both come off the url — the entry redirect seeds them and a switch - // rewrites them — so the first paint is already the right colors. + // The library comes off the url, so the first paint is already its color. const search = useSearch({ strict: false }); const params = useParams({ strict: false }); + const { theme: savedTheme, libraryId } = useGetUiState(); const theme = useMemo( - () => createAppTheme(params.libraryId ?? DEFAULT_SETTINGS.libraryId), - [params.libraryId] + () => createAppTheme(params.libraryId ?? libraryId), + [params.libraryId, libraryId] ); // Onshape puts its own scheme on the url when it launches us; standalone // there is none, and the OS is what "system" means. const osColorScheme = useColorScheme(); const colorTheme = getColorTheme( - search.theme ?? DEFAULT_SETTINGS.theme, + savedTheme, search.systemTheme ?? osColorScheme ); diff --git a/src/frontend/routes/app/library/$libraryId/groups/$groupId.tsx b/src/frontend/routes/app/library/$libraryId/groups/$groupId.tsx index 1929bb15..5409536b 100644 --- a/src/frontend/routes/app/library/$libraryId/groups/$groupId.tsx +++ b/src/frontend/routes/app/library/$libraryId/groups/$groupId.tsx @@ -35,7 +35,7 @@ import { import { ClearFiltersButton } from "../../../../../features/settings/components/vendor-filters"; import { useLibraryQuery } from "../../../../../features/library/queries"; import { useLibraryId } from "../../../../../features/library/library-path"; -import { useUiState } from "../../../../../lib/ui-state"; +import { useGetUiState } from "../../../../../lib/ui-state"; import { rememberOpenGroup } from "../../../../../features/settings/settings"; export const Route = createFileRoute("/app/library/$libraryId/groups/$groupId")( @@ -54,7 +54,7 @@ function GroupList(): ReactNode { from: "/app/library/$libraryId/groups/$groupId" }); - const uiState = useUiState()[0]; + const uiState = useGetUiState(); if (libraryQuery.isPending) { return ; @@ -157,7 +157,7 @@ export function GroupListContent(props: GroupListCardsProps): ReactNode { const { group, insertables } = props; const accessData = useAccessData(); - const uiState = useUiState()[0]; + const uiState = useGetUiState(); const groupInsertables = group.insertableOrder .map((insertableId) => insertables[insertableId]) diff --git a/src/frontend/routes/app/library/$libraryId/index.tsx b/src/frontend/routes/app/library/$libraryId/index.tsx index d1e3c914..841f4dc6 100644 --- a/src/frontend/routes/app/library/$libraryId/index.tsx +++ b/src/frontend/routes/app/library/$libraryId/index.tsx @@ -27,7 +27,7 @@ import { getLibraryStatus, useLibraryId } from "../../../../features/library/library-path"; -import { useUiState } from "../../../../lib/ui-state"; +import { useGetUiState, useSetUiState } from "../../../../lib/ui-state"; import { rememberOpenGroup } from "../../../../features/settings/settings"; import { useIsSignedIn } from "../../../../features/auth/access-level"; @@ -50,7 +50,8 @@ interface Section { } function HomeList(): ReactNode { - const [uiState, setUiState] = useUiState(); + const uiState = useGetUiState(); + const setUiState = useSetUiState(); // Not persisted: search results open on every visit, unlike the library. const [isSearchOpen, setIsSearchOpen] = useState(true); const libraryId = useLibraryId(); diff --git a/src/frontend/routes/app/route.tsx b/src/frontend/routes/app/route.tsx index 0967ab05..da339db8 100644 --- a/src/frontend/routes/app/route.tsx +++ b/src/frontend/routes/app/route.tsx @@ -1,6 +1,7 @@ import { createFileRoute, Outlet, + redirect, retainSearchParams, type SearchSchemaInput } from "@tanstack/react-router"; @@ -12,7 +13,7 @@ import { OnshapeParams } from "../../lib/onshape-params"; import { AppNavbar } from "../../components/app-navbar"; import { SectionLoading } from "../../components/app-zero-state"; import { useMessageListener } from "../../lib/messages"; -import { useSignInToast } from "../../features/auth/sign-in"; +import { updateUiState } from "../../lib/ui-state"; import { RootAppError } from "../../components/root-error"; export const Route = createFileRoute("/app")({ @@ -21,7 +22,32 @@ export const Route = createFileRoute("/app")({ return search as unknown as OnshapeParams; }, search: { - middlewares: [retainSearchParams(true)] + // What Onshape launched us with, which every navigation keeps. The + // theme rides along too, but only as far as beforeLoad below. + middlewares: [ + retainSearchParams([ + "documentId", + "instanceId", + "instanceType", + "elementId", + "elementType", + "systemTheme", + "server" + ]) + ] + }, + beforeLoad: ({ search, location }) => { + // The entry redirect seeds the theme the account last saved; ui-state + // is what the app reads from here on, so take it and drop the + // parameter rather than leave a second answer in the url. + if (search.theme) { + updateUiState({ theme: search.theme }); + throw redirect({ + to: location.pathname, + search: { ...search, theme: undefined }, + replace: true + }); + } }, errorComponent: RootAppError }); @@ -32,7 +58,6 @@ function App() { const { ref: headerRef, height: headerHeight } = useElementSize(); useMessageListener(); - useSignInToast(); return ( diff --git a/src/frontend/routes/index.tsx b/src/frontend/routes/index.tsx index 92ca6712..7a3d09f6 100644 --- a/src/frontend/routes/index.tsx +++ b/src/frontend/routes/index.tsx @@ -1,23 +1,32 @@ import { createFileRoute, redirect } from "@tanstack/react-router"; -import { readLocalSettings } from "../features/settings/local-settings"; +import { getUiState, updateUiState } from "../lib/ui-state"; +import { showSuccessToast } from "../lib/notifications"; import { RootAppError } from "../components/root-error"; -// Direct entry for a user opening the app outside Onshape. Onshape's own launch -// is handled server-side, so it never reaches this route. +// Direct entry for a user opening the app outside Onshape, and where signing in +// returns to. Onshape's own launch is handled server-side, so it never reaches +// this route. export const Route = createFileRoute("/")({ - beforeLoad: () => { - const { libraryId, theme, groupId } = readLocalSettings(); + beforeLoad: ({ search }) => { + const { libraryId, groupId, justSignedIn } = getUiState(); + if (justSignedIn) { + updateUiState({ justSignedIn: false }); + // Onshape only sends the caller back here on success, so arriving + // with the flag set is the confirmation. + showSuccessToast("Signed in to Onshape."); + } + // Whatever Onshape launched with rides along; only the path is ours. if (groupId) { throw redirect({ to: "/app/library/$libraryId/groups/$groupId", params: { libraryId, groupId }, - search: { theme } + search }); } throw redirect({ to: "/app/library/$libraryId", params: { libraryId }, - search: { theme } + search }); }, errorComponent: RootAppError diff --git a/src/frontend/theme.ts b/src/frontend/theme.ts index 33f7149e..12cab245 100644 --- a/src/frontend/theme.ts +++ b/src/frontend/theme.ts @@ -33,7 +33,7 @@ function getLibraryColor(libraryId: string): string { } } -/** The chrome stays neutral; a library's color is an accent on its controls. */ +/** The frame stays neutral; a library's color is an accent on its controls. */ export function createAppTheme(libraryId: string) { return createTheme({ colors: { frcGreen }, From c207934cf28c9d5a5811f6bef9ca45e201e087d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 04:10:50 +0000 Subject: [PATCH 12/40] Drop the tests for the unit folding that was removed The mark still ends its number, which is what the tests that remain cover; `1 in` still reaches the 1" configuration through the term scoring rather than through canonicalization. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- src/frontend/features/search/search.test.ts | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/frontend/features/search/search.test.ts b/src/frontend/features/search/search.test.ts index 6ecfb6c6..89066f67 100644 --- a/src/frontend/features/search/search.test.ts +++ b/src/frontend/features/search/search.test.ts @@ -75,23 +75,6 @@ describe("tokenize", () => { expect(tokenize("1/2 Bearing")).toEqual(["0.5", "Bearing"]); }); - it("spells a size's unit as the mark, however it was written", () => { - for (const size of ['1"', "1in", "1 in", "1 inch", "1 inches"]) { - expect(tokenize(size + " Standoff")).toEqual(['1"', "Standoff"]); - } - expect(tokenize("1/2 in Standoff")).toEqual(['0.5"', "Standoff"]); - }); - - // Only a unit is folded away; a word that merely starts with it is a word. - it("leaves a word beginning with the unit alone", () => { - expect(tokenize("8 in-line Insert")).toEqual([ - "8", - "in", - "line", - "Insert" - ]); - }); - // The mark is what makes `1"` a size rather than a prefix of 1.5 and 16T. it("keeps an inch mark on the number it measures", () => { expect(tokenize('1" Hex Shaft')).toEqual(['1"', "Hex", "Shaft"]); From 80ae0f42dc54125dcbdd40e46dc89fb4270fdd59 Mon Sep 17 00:00:00 2001 From: Alex Kempen Date: Sun, 30 Aug 2026 23:11:28 -0500 Subject: [PATCH 13/40] Refactor vendor parsing --- src/backend/features/configurations/utils.ts | 12 ++++++------ src/backend/features/library/vendors.test.ts | 4 ++-- src/backend/features/library/vendors.ts | 6 +++--- src/backend/features/load/parse-vendors.ts | 4 ++-- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/backend/features/configurations/utils.ts b/src/backend/features/configurations/utils.ts index 74a78814..d085bad1 100644 --- a/src/backend/features/configurations/utils.ts +++ b/src/backend/features/configurations/utils.ts @@ -16,8 +16,7 @@ import { import { Vendor, getVendorPartUrl, - parsePartNumberVendor, - toVendor + parseVendorFromPartNumber } from "../library/vendors"; import { LogicalOp, QuantityType, Unit } from "./enums"; import { type EvaluateOptions, valueWithUnits } from "./input-parser"; @@ -98,10 +97,11 @@ export function getPartUrl( if (record.description && /^https?:\/\//i.test(record.description)) { return record.description; } - const vendor = - parsePartNumberVendor(record.partNumber) ?? - toVendor(record.vendor) ?? - (vendors.length === 1 ? vendors[0] : undefined); + // WCP-123 -> WCP + let vendor = parseVendorFromPartNumber(record.partNumber); + if (!vendor && vendors.length === 1) { + vendor = vendors[0]; + } return getVendorPartUrl(vendor, record.partNumber); } diff --git a/src/backend/features/library/vendors.test.ts b/src/backend/features/library/vendors.test.ts index 6d07e7f4..60618f9e 100644 --- a/src/backend/features/library/vendors.test.ts +++ b/src/backend/features/library/vendors.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { Vendor, getVendorPartUrl, toVendor } from "./vendors"; +import { Vendor, getVendorPartUrl, parseVendor } from "./vendors"; describe("getVendorPartUrl", () => { // Each vendor writes its own casing, and only some have a per-part page. @@ -53,7 +53,7 @@ describe("toVendor", () => { ["", undefined], [undefined, undefined] ])("resolves %s", (text, expected) => { - expect(toVendor(text)).toBe(expected); + expect(parseVendor(text)).toBe(expected); }); }); diff --git a/src/backend/features/library/vendors.ts b/src/backend/features/library/vendors.ts index 29563eed..bc2c7f02 100644 --- a/src/backend/features/library/vendors.ts +++ b/src/backend/features/library/vendors.ts @@ -19,7 +19,7 @@ export enum Vendor { * Resolves the free text Onshape carries as a vendor to one we know, written * either as its code or as its full name. */ -export function toVendor(vendor: string | undefined): Vendor | undefined { +export function parseVendor(vendor: string | undefined): Vendor | undefined { const text = vendor?.trim().toUpperCase(); if (!text) { return undefined; @@ -35,10 +35,10 @@ export function toVendor(vendor: string | undefined): Vendor | undefined { * The vendor a part number names itself, e.g. `WCP-1025` — more precise than an * insertable's tagging, which is generic wherever one part spans vendors. */ -export function parsePartNumberVendor( +export function parseVendorFromPartNumber( partNumber: string | undefined ): Vendor | undefined { - return toVendor(/^([A-Za-z]+)-/.exec(partNumber?.trim() ?? "")?.[1]); + return parseVendor(/^([A-Za-z]+)-/.exec(partNumber?.trim() ?? "")?.[1]); } /** diff --git a/src/backend/features/load/parse-vendors.ts b/src/backend/features/load/parse-vendors.ts index 5c725dea..06f0c3dc 100644 --- a/src/backend/features/load/parse-vendors.ts +++ b/src/backend/features/load/parse-vendors.ts @@ -1,4 +1,4 @@ -import { Vendor, toVendor } from "../library/vendors"; +import { Vendor, parseVendor } from "../library/vendors"; import { ParameterType, type ConfigurationParameter, @@ -18,7 +18,7 @@ export function parseNameVendor(name: string): Vendor | undefined { /** A vendor an option names, as a token within its label or as the whole of it. */ function parseOptionVendor(optionName: string): Vendor | undefined { - return parseNameVendor(optionName) ?? toVendor(optionName); + return parseNameVendor(optionName) ?? parseVendor(optionName); } export function parseVendors( From 0b7810a9135675113addd370875784670e7563e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 12:41:20 +0000 Subject: [PATCH 14/40] Consolidate trimming, and give the part-number rule one home MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trimming and case-folding were sprinkled across the record parser, the vendor lookup, the index and the frontend's display rule, and the placeholder rule ran only at render — so `N/A` reached the index and searching it returned noise, while "repeats the name" was written twice. `clean` and `equalsIgnoreCase` are now a shared leaf, and one `meaningfulPartNumber` says whether a number identifies anything. The frontend's displayPartNumber delegates to it, so what is shown and what is indexed cannot drift apart. The escaped-slash literals read as strings while they are being touched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- .../configurations/part-number.test.ts | 24 ++++++++++++++ .../features/configurations/part-number.ts | 20 ++++++++++++ src/backend/features/configurations/utils.ts | 5 ++- src/backend/features/library/vendors.ts | 9 ++++-- .../load/parse-configuration-records.ts | 24 ++++++-------- src/backend/lib/text.test.ts | 31 +++++++++++++++++++ src/backend/lib/text.ts | 15 +++++++++ src/frontend/lib/part-number.test.ts | 20 ------------ src/frontend/lib/part-number.ts | 18 +---------- 9 files changed, 111 insertions(+), 55 deletions(-) create mode 100644 src/backend/features/configurations/part-number.test.ts create mode 100644 src/backend/features/configurations/part-number.ts create mode 100644 src/backend/lib/text.test.ts create mode 100644 src/backend/lib/text.ts delete mode 100644 src/frontend/lib/part-number.test.ts diff --git a/src/backend/features/configurations/part-number.test.ts b/src/backend/features/configurations/part-number.test.ts new file mode 100644 index 00000000..ce96049a --- /dev/null +++ b/src/backend/features/configurations/part-number.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from "vitest"; +import { meaningfulPartNumber } from "./part-number"; + +describe("meaningfulPartNumber", () => { + it.each(["N/A", "n/a", " N/a "])("hides the placeholder %s", (value) => { + expect(meaningfulPartNumber(value)).toBeUndefined(); + }); + + it.each([undefined, null, "", " "])("hides a blank %s", (value) => { + expect(meaningfulPartNumber(value)).toBeUndefined(); + }); + + it("hides a number that only repeats the name it sits under", () => { + expect(meaningfulPartNumber(" spacer ", "Spacer")).toBeUndefined(); + }); + + it("keeps a real number, trimmed", () => { + expect(meaningfulPartNumber(" WCP-1025 ", "Gearbox")).toBe("WCP-1025"); + }); + + it("keeps one that merely contains the placeholder", () => { + expect(meaningfulPartNumber("NA-1234")).toBe("NA-1234"); + }); +}); diff --git a/src/backend/features/configurations/part-number.ts b/src/backend/features/configurations/part-number.ts new file mode 100644 index 00000000..e462ff58 --- /dev/null +++ b/src/backend/features/configurations/part-number.ts @@ -0,0 +1,20 @@ +import { clean, equalsIgnoreCase } from "../../lib/text"; + +/** Admins write this in where a generic part has no real number to give. */ +const PLACEHOLDER_PART_NUMBER = new RegExp("^n/a$", "i"); + +/** + * The part number when it identifies the part, and nothing when it doesn't — a + * placeholder, or a repeat of the name it sits under. One rule for indexing and + * display alike, so a number nobody can search for is never shown either. + */ +export function meaningfulPartNumber( + partNumber: string | undefined | null, + name?: string | null +): string | undefined { + const text = clean(partNumber); + if (!text || PLACEHOLDER_PART_NUMBER.test(text)) { + return undefined; + } + return equalsIgnoreCase(text, name) ? undefined : text; +} diff --git a/src/backend/features/configurations/utils.ts b/src/backend/features/configurations/utils.ts index d085bad1..578812ff 100644 --- a/src/backend/features/configurations/utils.ts +++ b/src/backend/features/configurations/utils.ts @@ -90,11 +90,14 @@ export function evaluateCondition( * The page for a part, in descending precision: a description that is already a * url, then the vendor the part number names, then the taggings standing in. */ +/** A description holding a link is the link, rather than a description. */ +const ABSOLUTE_URL = new RegExp("^https?://", "i"); + export function getPartUrl( record: PartMetadata, vendors: Vendor[] = [] ): string | undefined { - if (record.description && /^https?:\/\//i.test(record.description)) { + if (record.description && ABSOLUTE_URL.test(record.description)) { return record.description; } // WCP-123 -> WCP diff --git a/src/backend/features/library/vendors.ts b/src/backend/features/library/vendors.ts index bc2c7f02..81fd8e98 100644 --- a/src/backend/features/library/vendors.ts +++ b/src/backend/features/library/vendors.ts @@ -1,3 +1,8 @@ +import { clean } from "../../lib/text"; + +/** A part number's leading letters, which name the vendor that sells it. */ +const VENDOR_PREFIX = new RegExp("^([A-Za-z]+)-"); + /** The vendors an insertable can come from, and how they are displayed. */ export enum Vendor { AM = "AM", @@ -20,7 +25,7 @@ export enum Vendor { * either as its code or as its full name. */ export function parseVendor(vendor: string | undefined): Vendor | undefined { - const text = vendor?.trim().toUpperCase(); + const text = clean(vendor)?.toUpperCase(); if (!text) { return undefined; } @@ -38,7 +43,7 @@ export function parseVendor(vendor: string | undefined): Vendor | undefined { export function parseVendorFromPartNumber( partNumber: string | undefined ): Vendor | undefined { - return parseVendor(/^([A-Za-z]+)-/.exec(partNumber?.trim() ?? "")?.[1]); + return parseVendor(VENDOR_PREFIX.exec(clean(partNumber) ?? "")?.[1]); } /** diff --git a/src/backend/features/load/parse-configuration-records.ts b/src/backend/features/load/parse-configuration-records.ts index 70eed9c8..1ffe291c 100644 --- a/src/backend/features/load/parse-configuration-records.ts +++ b/src/backend/features/load/parse-configuration-records.ts @@ -31,6 +31,7 @@ import type { } from "../../lib/onshape/types"; import { type LoadContext, getOnshapeApiFromContext } from "./context"; import { ONSHAPE_STEP_RETRIES } from "./steps"; +import { clean } from "../../lib/text"; /** Configurations fetched per workflow step. */ const BATCH_SIZE = 20; @@ -59,8 +60,7 @@ export const INDEXING_ISSUE_TYPES = [ BuildIssueType.CONFIGURATION_LIMIT_EXCEEDED, BuildIssueType.MANUAL_INDEXING_REQUIRED, BuildIssueType.MULTIPLE_PARTS, - BuildIssueType.UNSTABLE_COMPOSITE, - BuildIssueType.NO_PART_NUMBER + BuildIssueType.UNSTABLE_COMPOSITE ]; /** Whether to index an insertable, and how to flag it if we don't. */ @@ -107,12 +107,6 @@ export function decideIndexing( return { shouldIndex, buildIssues: [], configurations }; } -/** Trims a raw metadata value; a missing or blank one becomes `null`. */ -function normalizeText(value: string | undefined | null): string | undefined { - const trimmed = value?.trim(); - return trimmed ? trimmed : undefined; -} - /** What a part studio's parts resolve to, before build issues are decided. */ export interface PartsEvaluation { /** True when more than one part could be the one to index. */ @@ -170,11 +164,11 @@ export function parsePartStudioRecord( const part = evaluation.partToUse; return { configuration, - partNumber: normalizeText(part?.partNumber), - name: normalizeText(part?.name), - description: normalizeText(part?.description), - material: normalizeText(part?.material?.displayName), - vendor: normalizeText(part?.vendor), + partNumber: clean(part?.partNumber), + name: clean(part?.name), + description: clean(part?.description), + material: clean(part?.material?.displayName), + vendor: clean(part?.vendor), hasMultipleParts: evaluation.hasMultipleParts, isOpenComposite: evaluation.isOpenComposite }; @@ -192,10 +186,10 @@ const METADATA_FIELDS = { /** Reads a metadata property value as text; materials arrive as `{displayName}`. */ function readMetadataValue(value: unknown): string | undefined { if (typeof value === "string") { - return normalizeText(value); + return clean(value); } if (value && typeof value === "object" && "displayName" in value) { - return normalizeText((value as { displayName?: string }).displayName); + return clean((value as { displayName?: string }).displayName); } return undefined; } diff --git a/src/backend/lib/text.test.ts b/src/backend/lib/text.test.ts new file mode 100644 index 00000000..24bf40f0 --- /dev/null +++ b/src/backend/lib/text.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from "vitest"; +import { clean, equalsIgnoreCase } from "./text"; + +describe("clean", () => { + it("trims the value", () => { + expect(clean(" WCP-1025 ")).toBe("WCP-1025"); + }); + + it.each([undefined, null, "", " "])( + "reads %s as nothing at all", + (value) => { + expect(clean(value)).toBeUndefined(); + } + ); +}); + +describe("equalsIgnoreCase", () => { + it("ignores case and surrounding space", () => { + expect(equalsIgnoreCase(" Spacer ", "spacer")).toBe(true); + }); + + it("separates different text", () => { + expect(equalsIgnoreCase("Spacer", "Standoff")).toBe(false); + }); + + // Two parts with nothing to say are not thereby the same part. + it("does not equate two blanks with a value", () => { + expect(equalsIgnoreCase(" ", undefined)).toBe(true); + expect(equalsIgnoreCase("Spacer", undefined)).toBe(false); + }); +}); diff --git a/src/backend/lib/text.ts b/src/backend/lib/text.ts new file mode 100644 index 00000000..5d0b3fc5 --- /dev/null +++ b/src/backend/lib/text.ts @@ -0,0 +1,15 @@ +/** Trimming and casing, defined once: both sides import this leaf. */ + +/** A value's meaningful text, or nothing when it is blank. */ +export function clean(text: string | undefined | null): string | undefined { + const trimmed = text?.trim(); + return trimmed ? trimmed : undefined; +} + +/** Whether two values say the same thing, ignoring case and surrounding space. */ +export function equalsIgnoreCase( + a: string | undefined | null, + b: string | undefined | null +): boolean { + return clean(a)?.toLowerCase() === clean(b)?.toLowerCase(); +} diff --git a/src/frontend/lib/part-number.test.ts b/src/frontend/lib/part-number.test.ts deleted file mode 100644 index fe629aa9..00000000 --- a/src/frontend/lib/part-number.test.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { displayPartNumber } from "./part-number"; - -describe("displayPartNumber", () => { - it.each(["N/A", "n/a", " N/a "])("hides the placeholder %s", (value) => { - expect(displayPartNumber(value)).toBeUndefined(); - }); - - it("hides a number that only repeats the name it sits under", () => { - expect(displayPartNumber(" spacer ", "Spacer")).toBeUndefined(); - }); - - it("keeps a real number, trimmed", () => { - expect(displayPartNumber(" WCP-1025 ", "Gearbox")).toBe("WCP-1025"); - }); - - it("keeps one that merely contains the placeholder", () => { - expect(displayPartNumber("NA-1234")).toBe("NA-1234"); - }); -}); diff --git a/src/frontend/lib/part-number.ts b/src/frontend/lib/part-number.ts index e0987111..4935e52b 100644 --- a/src/frontend/lib/part-number.ts +++ b/src/frontend/lib/part-number.ts @@ -1,17 +1 @@ -/** Admins write this in where a generic part has no real number to give. */ -const PLACEHOLDER_PART_NUMBER = /^n\/a$/i; - -/** - * The part number to show, or nothing when it identifies nothing — a - * placeholder, or a repeat of the name it sits under. - */ -export function displayPartNumber( - partNumber: string | undefined, - name?: string -): string | undefined { - const text = partNumber?.trim(); - if (!text || PLACEHOLDER_PART_NUMBER.test(text)) { - return undefined; - } - return text.toLowerCase() === name?.trim().toLowerCase() ? undefined : text; -} +export { meaningfulPartNumber as displayPartNumber } from "@backend/features/configurations/part-number"; From 6dcb0f6b333521376732ac38b6a25d8f6e0b6fae Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 12:41:22 +0000 Subject: [PATCH 15/40] Index a part number as an identifier, not as a description MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One tokenizer ran over every field, so a part number was read as prose: 217-2600 became 217 and 2600, TTB-0016 lost its zeros, and the fraction in TTB-0016-5/32 was folded into a decimal. None of that identifies the part any more. Tokenizing is now field-aware. A part number is indexed as typed plus its segments, so it is found whole or by either half, zeros included. A name keeps the decimal canonicalization its sizes need, since the standards write the same measurement as 1/2" and 0.5". A query has no field, so it offers both readings — minus the pieces that would only flood: a bare size is not split into digits, and the lone letters left by splitting `n/a` are dropped. Commas no longer stick to the word before them, which the dimensions in a name are full of. The frontend scorer reads each field the same way, and the leading-zero underline hack goes with the zeros it worked around. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- .../features/search/search-index.test.ts | 304 +++++++++++++++++- src/backend/features/search/search-index.ts | 163 ++++++---- src/frontend/features/search/search.test.ts | 117 +++---- src/frontend/features/search/search.ts | 67 ++-- 4 files changed, 502 insertions(+), 149 deletions(-) diff --git a/src/backend/features/search/search-index.test.ts b/src/backend/features/search/search-index.test.ts index 7a82df11..d9029f47 100644 --- a/src/backend/features/search/search-index.test.ts +++ b/src/backend/features/search/search-index.test.ts @@ -1,8 +1,202 @@ import { describe, expect, it } from "vitest"; -import { toSearchRecords } from "./search-index"; +import { + buildSearchDb, + normalizeForMatch, + processTerm, + tokenize, + tokenizeName, + tokenizePartNumber, + tokenizeQuery, + toSearchRecords +} from "./search-index"; +import { LibraryOut } from "../library/contract"; +import { ElementType } from "../../lib/onshape/element-type"; import { Vendor } from "../library/vendors"; import { configurationRecord as record } from "../../../__test_utils__/configuration-fixtures"; +// A part number identifies the part; splitting or folding it makes it name a +// different one, so it is indexed as typed alongside its segments. +describe("tokenizePartNumber", () => { + it("keeps the number whole, and adds its segments", () => { + expect(tokenizePartNumber("WCP-1025")).toEqual([ + "wcp-1025", + "wcp", + "1025" + ]); + }); + + it("keeps leading zeros, which spell the segment", () => { + expect(tokenizePartNumber("TTB-0016")).toEqual([ + "ttb-0016", + "ttb", + "0016" + ]); + }); + + it("leaves a fraction inside a number alone", () => { + expect(tokenizePartNumber("TTB-0016-5/32")).toEqual([ + "ttb-0016-5/32", + "ttb", + "0016", + "5", + "32" + ]); + }); + + it("does not read a number as a quantity", () => { + expect(tokenizePartNumber("217-2600")).toEqual([ + "217-2600", + "217", + "2600" + ]); + }); + + it.each(["", " "])("has nothing to say about %s", (value) => { + expect(tokenizePartNumber(value)).toEqual([]); + }); +}); + +// A name describes the part, so its sizes are read as sizes. +describe("tokenizeName", () => { + it("splits on punctuation, keeping the words whole", () => { + expect(tokenizeName('1" Linear (REV)')).toEqual([ + '1"', + "Linear", + "REV" + ]); + expect(tokenizeName("Bearings & Bushings #X-Contact")).toEqual([ + "Bearings", + "Bushings", + "X", + "Contact" + ]); + }); + + // The standards write the same measurement both ways, so one decimal form + // is what lets either spelling find the other. + it("canonicalizes fractions and decimals to a 2-dp decimal", () => { + expect(tokenizeName("1/2")).toEqual(["0.5"]); + expect(tokenizeName(".5")).toEqual(["0.5"]); + expect(tokenizeName("0.50")).toEqual(["0.5"]); + expect(tokenizeName("3/4")).toEqual(["0.75"]); + expect(tokenizeName("1-1/2")).toEqual(["1.5"]); + expect(tokenizeName("1/3")).toEqual(["0.33"]); + }); + + it.each([ + ['1/2" Hex Bearing (1.125" OD, 0.313" WD, Flanged)', '0.5"'], + // Stored to 2dp, so `1.125` and `1.13` are one size. + ['1/2" Hex Bearing (1.125" OD, 0.313" WD, Flanged)', '1.13"'], + ['#10-32 x 2.5" L SHCS', "10"], + // Sizes are stored to 2dp, so `.159` and `.16` are one size. + [".159 ID x SplineXL OD MotionX Hub", "0.16"] + ])("reads the sizes in %s", (name, size) => { + expect(tokenizeName(name)).toContain(size); + }); + + it("keeps a thread spec's halves apart", () => { + expect(tokenizeName("#10-32 Screw")).toEqual(["10", "32", "Screw"]); + }); + + // The standards list a part's dimensions in a comma-separated aside. + it("does not leave a comma stuck to the word before it", () => { + expect(tokenizeName('1.125" OD, Flanged')).toEqual([ + '1.13"', + "OD", + "Flanged" + ]); + }); + + // The mark is what makes `1"` a size rather than a prefix of 1.5 and 16T. + it("keeps an inch mark on the number it measures", () => { + expect(tokenizeName('1" Hex Shaft')).toEqual(['1"', "Hex", "Shaft"]); + expect(tokenizeName('1/2" Hex')).toEqual(['0.5"', "Hex"]); + expect(tokenizeName('1"x2" Tube')).toEqual(['1"', 'x2"', "Tube"]); + }); + + it("still drops quotes that quote something", () => { + expect(tokenizeName('The "Long" Bracket')).toEqual([ + "The", + "Long", + "Bracket" + ]); + }); +}); + +describe("processTerm", () => { + it.each(["MAXSpline", "MaxSpline"])("splits %s into its words", (term) => { + expect(processTerm(term)).toEqual( + expect.arrayContaining(["max", "spline", "maxspline"]) + ); + }); + + it.each([ + ["SplineXL", ["spline", "xl"]], + ["roboRIO", ["robo", "rio"]], + ["MAXTube", ["max", "tube"]] + ])("splits the product name %s", (term, words) => { + expect(processTerm(term)).toEqual(expect.arrayContaining(words)); + }); + + // Its segments are already separate tokens; splitting the code again would + // only invent words inside it. + it("leaves a part number whole", () => { + expect(processTerm("WCP-1025", "partNumbers")).toEqual(["wcp-1025"]); + }); +}); + +describe("tokenize", () => { + it("reads each field the way that field is written", () => { + expect(tokenize("TTB-0016-5/32", "partNumbers")).toContain("0016"); + expect(tokenize("TTB-0016-5/32", "partNames")).toEqual([ + "TTB", + "16", + "0.16" + ]); + }); +}); + +// A query has no field, so it has to offer both readings: the caller may have +// typed a size or a part number. +describe("tokenizeQuery", () => { + it("offers the part number as typed, and as a name would read it", () => { + expect(tokenizeQuery("TTB-0016")).toEqual( + expect.arrayContaining(["ttb", "0016", "ttb-0016"]) + ); + }); + + // `1` prefix-matches every number in the library, so a size is not split + // into the segments a part number would be. + it("does not split a bare size into its digits", () => { + expect(tokenizeQuery("1/2")).toEqual(["0.5", "1/2"]); + }); + + it("leaves an ordinary word alone", () => { + expect(tokenizeQuery("bearing")).toEqual(["bearing"]); + }); + + // Splitting the placeholder leaves `n` and `a`, and a one-letter prefix + // matches most of the library. + it.each(["n/a", "N/A"])( + "drops the letters left over from splitting %s", + (query) => { + expect(tokenizeQuery(query)).toEqual(["n/a"]); + } + ); + + it("keeps a lone digit, which names a size", () => { + expect(tokenizeQuery("1")).toEqual(["1"]); + }); +}); + +describe("normalizeForMatch", () => { + it("reads a written size and its decimal as one string", () => { + expect(normalizeForMatch('1/2" Hex')).toBe( + normalizeForMatch('.5" hex') + ); + }); +}); + describe("toSearchRecords", () => { it("drops a part number that only repeats the name", () => { const [result] = toSearchRecords([ @@ -45,4 +239,112 @@ describe("toSearchRecords", () => { it("drops a record with neither", () => { expect(toSearchRecords([record({})])).toHaveLength(0); }); + + // The placeholder an admin writes in identifies nothing, so it is dropped + // here rather than indexed and shown. + it("drops a placeholder part number", () => { + const [result] = toSearchRecords([ + record({ partNumber: "N/A", name: "Spacer" }) + ]); + expect(result).toMatchObject({ partNumber: undefined, name: "Spacer" }); + }); + + it("will not link a placeholder to a vendor", () => { + const [result] = toSearchRecords( + [record({ partNumber: "N/A", name: "Spacer" })], + [Vendor.WCP] + ); + expect(result.url).toBeUndefined(); + }); + + it("drops a record the placeholder leaves with nothing", () => { + expect(toSearchRecords([record({ partNumber: "N/A" })])).toEqual([]); + }); + + it("keeps the first of a repeated (number, name)", () => { + expect( + toSearchRecords([ + record({ partNumber: "WCP-1025", name: "Gear" }), + record({ partNumber: "WCP-1025", name: "Gear" }) + ]) + ).toHaveLength(1); + }); +}); + +function library(name: string, vendors: Vendor[] = []): LibraryOut { + return { + groupOrder: ["g1"], + groups: { + g1: { + id: "g1", + documentId: "d1", + path: { documentId: "d1", instanceId: "v1", instanceType: "v" }, + name: "Group", + isLoaded: true, + insertableOrder: ["i1"] + } + }, + insertables: { + i1: { + id: "i1", + elementId: "e1", + groupId: "g1", + documentId: "d1", + versionId: "v1", + path: { + documentId: "d1", + instanceId: "v1", + instanceType: "v", + elementId: "e1" + }, + name, + microversionId: "mv1", + isVisible: true, + supportsFasten: false, + elementType: ElementType.PART_STUDIO, + isConfigurable: false, + vendors + } + } + }; +} + +describe("buildSearchDb", () => { + /** The document as the index stored it. */ + const stored = (db: ReturnType) => + db.getStoredFields("i1") as unknown as Record; + + it("keeps a placeholder part number out of the index and the records", () => { + const db = buildSearchDb(library("Spacer"), { + i1: [ + record({ + partNumber: "N/A", + name: "Spacer", + configuration: {} + }) + ] + }); + expect(db.search("n/a")).toEqual([]); + expect(stored(db).records).toEqual([ + expect.objectContaining({ partNumber: undefined }) + ]); + }); + + // The vendor is a resolution fallback, not something to match against. + it("never searches the vendor", () => { + const db = buildSearchDb(library("Spacer", [Vendor.WCP]), { + i1: [ + record({ + partNumber: "WCP-1025", + name: "Spacer", + vendor: "WestCoast Products", + configuration: {} + }) + ] + }); + expect(db.search("westcoast")).toEqual([]); + expect(stored(db).records).toEqual([ + expect.not.objectContaining({ vendor: expect.anything() }) + ]); + }); }); diff --git a/src/backend/features/search/search-index.ts b/src/backend/features/search/search-index.ts index f0167ead..614469c5 100644 --- a/src/backend/features/search/search-index.ts +++ b/src/backend/features/search/search-index.ts @@ -7,31 +7,20 @@ import { LibraryOut } from "../library/contract"; import { Vendor } from "../library/vendors"; import { ConfigurationRecord, SearchRecord } from "../configurations/models"; import { getPartUrl } from "../configurations/utils"; +import { meaningfulPartNumber } from "../configurations/part-number"; +import { clean } from "../../lib/text"; -const deliminator = "^"; +/** Where a name breaks: punctuation and space, plus a quote used as a quote. */ +const NAME_SEPARATORS = new RegExp("(? camel case) - const camelSplit = term - .replace(/([a-z])([A-Z])/g, `$1${deliminator}$2`) - .split(deliminator); - - // Insert spaces to handle MAXTube->MAX Tube, VEXpro->VEX pro - const pascalSplit = term - .replace(/([A-Z])([A-Z][a-z])/g, `$1${deliminator}$2`) - .split(deliminator); +/** Where a part number breaks into segments, keeping the whole alongside. */ +const PART_NUMBER_SEPARATORS = new RegExp("[-/]+"); - const base = term.toLowerCase(); - - const terms = [...camelSplit, ...pascalSplit, base].map((t) => - t.toLowerCase() - ); - // Deduplicate - return Array.from(new Set(terms)); -} +/** camelCase and PascalCase boundaries: MAXSpline -> max spline, MAXTube -> max tube. */ +const WORD_BOUNDARIES = new RegExp( + "(?<=[a-z])(?=[A-Z])|(?<=[A-Z])(?=[A-Z][a-z])", + "g" +); // A mixed number, simple fraction, decimal (incl. leading-dot), or plain // integer. Alternatives are ordered longest-first so `1-1/2` is consumed whole, @@ -51,7 +40,8 @@ function toDecimal(value: number): string { /** * Rewrites numbers and fractions to one 2-dp decimal, at index and query time - * alike — which is what lets the raw fragments go unstored. + * alike — which is what lets the raw fragments go unstored. Names only: a part + * number is an identifier, and 217-2600 is not two thousand six hundred. */ function canonicalizeNumbers(text: string): string { return text.replace( @@ -95,17 +85,92 @@ export function normalizeForMatch(text: string): string { return canonicalizeNumbers(text).toLowerCase(); } -export function tokenize(text: string): string[] { - // Canonicalize before splitting: fractions span `/` and `-`. Casing stays, +/** + * A name's words, with its sizes in the one decimal spelling. The inch mark + * stays on its number, so `1"` is a size rather than a prefix of `1.5` and `16t`. + */ +export function tokenizeName(text: string): string[] { + // Canonicalized before splitting: fractions span `/` and `-`. Casing stays, // since processTerm splits on camelCase. - return ( - canonicalizeNumbers(text) - // An inch mark stays on its number and ends the token, so `1"` is a - // size of its own rather than a prefix of `1.5`, `10`, and `16t`. - .replace(/(\d")/g, `$1${deliminator}`) - .split(/(? !/^[a-z]$/i.test(token)); +} + +/** + * Adds the words inside a compound term, so `MAXSpline` is found by `spline`. + * A part number is left whole: its segments are already separate tokens. + */ +export function processTerm(term: string, field?: string): string[] { + const base = term.toLowerCase(); + if (isPartNumberField(field)) { + return [base]; + } + const words = term.split(WORD_BOUNDARIES).map((word) => word.toLowerCase()); + return Array.from(new Set([...words, base])); } export interface SearchDocument { @@ -155,22 +220,11 @@ function uniqueJoin(values: (string | undefined)[]): string { ).join(" "); } -/** - * A part number repeating the name identifies nothing — it is what a generic - * part is given for want of a real one — so it is neither shown nor searched. - */ -function withoutRepeatedPartNumber( - record: ConfigurationRecord -): ConfigurationRecord { - const repeated = - record.partNumber?.trim().toLowerCase() === - record.name?.trim().toLowerCase(); - return repeated ? { ...record, partNumber: undefined } : record; -} - /** * Keeps the first of each distinct (part number, name) in enumeration order and - * drops records with neither. First-wins is what keeps the latest revision. + * drops records with neither. First-wins is what keeps the latest revision. A + * number that identifies nothing is dropped here, so it never reaches the + * index, the stored records, or a vendor url. */ export function toSearchRecords( records: ConfigurationRecord[], @@ -179,20 +233,21 @@ export function toSearchRecords( const seen = new Set(); const searchRecords: SearchRecord[] = []; for (const raw of records) { - const record = withoutRepeatedPartNumber(raw); - if (!record.partNumber && !record.name) { + const partNumber = meaningfulPartNumber(raw.partNumber, raw.name); + const name = clean(raw.name); + if (!partNumber && !name) { continue; } - const key = JSON.stringify([record.partNumber, record.name]); + const key = JSON.stringify([partNumber, name]); if (seen.has(key)) { continue; } seen.add(key); searchRecords.push({ - partNumber: record.partNumber, - name: record.name, - url: getPartUrl(record, vendors), - configuration: record.configuration + partNumber, + name, + url: getPartUrl({ ...raw, partNumber }, vendors), + configuration: raw.configuration }); } return searchRecords; diff --git a/src/frontend/features/search/search.test.ts b/src/frontend/features/search/search.test.ts index 89066f67..46ab0bc6 100644 --- a/src/frontend/features/search/search.test.ts +++ b/src/frontend/features/search/search.test.ts @@ -2,8 +2,6 @@ import { describe, expect, it } from "vitest"; import MiniSearch from "minisearch"; import { buildSearchDb, - processTerm, - tokenize, type SearchDocument } from "@backend/features/search/search-index"; import { doSearch, type Position } from "./search"; @@ -25,72 +23,6 @@ const record = ( const search = (searchDb: MiniSearch, query: string) => doSearch(searchDb, query, undefined, undefined, true); -describe("processTerm", () => { - it.each(["MAXSpline", "MaxSpline"])("splits %s into its words", (term) => { - expect(processTerm(term)).toEqual( - expect.arrayContaining(["max", "spline", "maxspline"]) - ); - }); -}); - -describe("tokenize", () => { - it("splits on punctuation, keeping the words whole", () => { - expect(tokenize('1" Linear (REV)')).toEqual(['1"', "Linear", "REV"]); - expect(tokenize("10-32 Bearings & Bushings #X-Contact")).toEqual([ - "10", - "32", - "Bearings", - "Bushings", - "X", - "Contact" - ]); - }); - - it("canonicalizes fractions and decimals to a 2-dp decimal", () => { - expect(tokenize("1/2")).toEqual(["0.5"]); - expect(tokenize(".5")).toEqual(["0.5"]); - expect(tokenize("0.50")).toEqual(["0.5"]); - expect(tokenize("3/4")).toEqual(["0.75"]); - expect(tokenize("1-1/2")).toEqual(["1.5"]); - expect(tokenize("1.5")).toEqual(["1.5"]); - expect(tokenize("1/3")).toEqual(["0.33"]); - }); - - it("keeps a leading-zero part segment out of a mixed number", () => { - // "0016" numbers the part; only the "5/32" is a size. - expect(tokenize("TTB-0016-5/32")).toEqual(["TTB", "16", "0.16"]); - }); - - it("drops leading zeros so either spelling of a segment matches", () => { - expect(tokenize("TTB-0016")).toEqual(["TTB", "16"]); - expect(tokenize("TTB-16")).toEqual(["TTB", "16"]); - }); - - it("leaves thread specs and part numbers untouched", () => { - expect(tokenize("10-32")).toEqual(["10", "32"]); - expect(tokenize("217-2600")).toEqual(["217", "2600"]); - }); - - it("canonicalizes a fraction inside a name", () => { - expect(tokenize("1/2 Bearing")).toEqual(["0.5", "Bearing"]); - }); - - // The mark is what makes `1"` a size rather than a prefix of 1.5 and 16T. - it("keeps an inch mark on the number it measures", () => { - expect(tokenize('1" Hex Shaft')).toEqual(['1"', "Hex", "Shaft"]); - expect(tokenize('1/2" Hex')).toEqual(['0.5"', "Hex"]); - expect(tokenize('Bearing 1"')).toEqual(["Bearing", '1"']); - }); - - it("still drops quotes that quote something", () => { - expect(tokenize('The "Long" Bracket')).toEqual([ - "The", - "Long", - "Bracket" - ]); - }); -}); - function library(name = "Bracket"): LibraryOut { return { groupOrder: ["g1"], @@ -290,6 +222,49 @@ describe("doSearch size matching", () => { }); }); +// A part number is a code: it retrieves its part whole, by either half, and +// with the zeros and separators it was written with. +describe("doSearch part numbers", () => { + const searchDb = buildSearchDb(library("Hex Standoff"), { + i1: [ + record("WCP-1025", {}, "Standoff"), + record("TTB-0016-5/32", { size: "small" }, "Small Standoff") + ] + }); + + it.each(["WCP-1025", "wcp-1025", "WCP", "1025"])( + "finds the part by %s", + (query) => { + const { hits } = search(searchDb, query); + expect(hits[0]?.id).toBe("i1"); + } + ); + + it("keeps the zeros a segment was written with", () => { + expect(search(searchDb, "0016").hits[0]?.partNumber).toBe( + "TTB-0016-5/32" + ); + }); + + it("picks the record whose number was typed out in full", () => { + expect(search(searchDb, "WCP-1025").hits[0].partNumber).toBe( + "WCP-1025" + ); + expect(search(searchDb, "TTB-0016-5/32").hits[0].partNumber).toBe( + "TTB-0016-5/32" + ); + }); + + // The placeholder never reaches the index, so it matches nothing rather + // than every part an admin left it on. + it("returns nothing for the placeholder", () => { + const withPlaceholders = buildSearchDb(library("Spacer"), { + i1: [record("N/A", {}, "Spacer")] + }); + expect(search(withPlaceholders, "n/a").hits).toEqual([]); + }); +}); + describe("doSearch highlighting", () => { /** The characters `positions` underline, merged the way applyRanges does. */ function highlighted(text: string, positions: Position[]): string { @@ -356,8 +331,8 @@ describe("doSearch highlighting", () => { ).toBe("217"); }); - // The segment after a leading-zero one used to be folded into a mixed - // number, leaving nothing in the text for the query to underline. + // A part number is indexed as typed, so the whole of what was typed + // is there in the text to underline, dash and zeros included. it("underlines a leading-zero segment of the part number", () => { const { hits } = search( buildSearchDb(library(), { @@ -370,7 +345,7 @@ describe("doSearch highlighting", () => { hits[0].partNumber!, hits[0].partNumberPositions ?? [] ) - ).toBe("TTB0016"); + ).toBe("TTB-0016"); }); it("underlines the typed prefix of the part name", () => { diff --git a/src/frontend/features/search/search.ts b/src/frontend/features/search/search.ts index 2ae81908..c883ac73 100644 --- a/src/frontend/features/search/search.ts +++ b/src/frontend/features/search/search.ts @@ -3,7 +3,8 @@ import { Vendor } from "@backend/features/library/vendors"; import { SearchDocument, normalizeForMatch, - tokenize + tokenizeName, + tokenizePartNumber } from "@backend/features/search/search-index"; import { ParameterValues, @@ -173,10 +174,10 @@ function matchedRecord( ): SearchRecord | undefined { const matchedFields = Object.values(result.match).flat(); const byNumber = matchedFields.includes("partNumbers") - ? findBestRecord(query, document.records, (r) => r.partNumber) + ? findBestRecord(query, document.records, (r) => r.partNumber, LITERAL) : undefined; const byName = matchedFields.includes("partNames") - ? findBestRecord(query, document.records, (r) => r.name) + ? findBestRecord(query, document.records, (r) => r.name, DESCRIPTIVE) : undefined; const best = [byNumber, byName] @@ -191,6 +192,27 @@ interface RecordMatch { score: number; } +/** + * How a field's text is read for scoring: a part number is compared as typed, + * a name around the decimals its sizes are indexed as. + */ +interface FieldReader { + normalize: (text: string) => string; + terms: (text: string) => string[]; +} + +/** A part number identifies: `217-2600` is a code, not a number. */ +const LITERAL: FieldReader = { + normalize: (text) => text.trim().toLowerCase(), + terms: tokenizePartNumber +}; + +/** A name describes, so `1/2`, `.5` and `0.5` are one size. */ +const DESCRIPTIVE: FieldReader = { + normalize: normalizeForMatch, + terms: (text) => tokenizeName(text).map((term) => term.toLowerCase()) +}; + /** * How well one term is answered: a term matched whole beats one matched as a * prefix, which every longer number satisfies too — `1` names the size `1"`, @@ -210,8 +232,12 @@ function termScore(valueTerms: string[], queryTerm: string): number { } /** How much of the query the value covers, term by term. */ -function coveredTerms(value: string, queryTerms: string[]): number { - const valueTerms = tokenize(value).map((term) => term.toLowerCase()); +function coveredTerms( + value: string, + queryTerms: string[], + field: FieldReader +): number { + const valueTerms = field.terms(value); return queryTerms.reduce( (score, queryTerm) => score + termScore(valueTerms, queryTerm), 0 @@ -225,7 +251,8 @@ function coveredTerms(value: string, queryTerms: string[]): number { function matchScore( value: string, normalizedQuery: string, - queryTerms: string[] + queryTerms: string[], + field: FieldReader ): number { let whole = 0; if (value === normalizedQuery) { @@ -237,7 +264,8 @@ function matchScore( } // Outweighs full term coverage, which is worth 2 a term. return ( - whole * (2 * queryTerms.length + 1) + coveredTerms(value, queryTerms) + whole * (2 * queryTerms.length + 1) + + coveredTerms(value, queryTerms, field) ); } @@ -248,21 +276,22 @@ function matchScore( function findBestRecord( query: string, records: SearchRecord[], - selector: (record: SearchRecord) => string | undefined + selector: (record: SearchRecord) => string | undefined, + field: FieldReader ): RecordMatch | undefined { - // Canonicalize the same way the index did, so a fraction/decimal query lines - // up with the stored original (e.g. `.5` matches a `"1/2 Bearing"` name). - const normalizedQuery = normalizeForMatch(query.trim()); + // Read the query the way the field was indexed, so a `.5` query lines up + // with a stored "1/2 Bearing" and a typed part number with itself. + const normalizedQuery = field.normalize(query.trim()); if (records.length === 0 || normalizedQuery === "") { return undefined; } - const queryTerms = tokenize(query).map((term) => term.toLowerCase()); + const queryTerms = field.terms(query); let best: RecordMatch | undefined; for (const record of records) { - const value = normalizeForMatch(selector(record) ?? ""); + const value = field.normalize(selector(record) ?? ""); if (!value) continue; - const score = matchScore(value, normalizedQuery, queryTerms); + const score = matchScore(value, normalizedQuery, queryTerms, field); if (score > (best?.score ?? 0)) { best = { record, score }; } @@ -310,15 +339,7 @@ function generateHighlightPositions( new RegExp(escapeRegExp(term), "g") ); for (const match of matchedLocations) { - // A number is indexed without its leading zeros, so underline the - // digits it landed inside too: `16` should light up all of `0016`. - const leadingZeros = /^\d+$/.test(term) - ? /0*$/.exec(haystack.slice(0, match.index))![0].length - : 0; - positions.push({ - start: match.index - leadingZeros, - length: length + leadingZeros - }); + positions.push({ start: match.index, length }); } } From 346be192349033136b38d89a2af04a2fbeb90bc5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 12:41:23 +0000 Subject: [PATCH 16/40] Remove the no-part-number check Some vendors genuinely sell parts without part numbers, so the warning fired as noise on them. Checking the format of a number that is there is the useful check, and that is a separate piece of work. Stored rows keep the issue until a reload recomputes them, where it renders as nothing at all rather than an error. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- .../features/build-checker/checks.test.ts | 39 +------------------ src/backend/features/build-checker/checks.ts | 28 +------------ src/backend/features/build-checker/issues.ts | 5 --- .../features/library/insertables/routes.ts | 8 +--- src/backend/features/load/load-insertable.ts | 6 +-- 5 files changed, 4 insertions(+), 82 deletions(-) diff --git a/src/backend/features/build-checker/checks.test.ts b/src/backend/features/build-checker/checks.test.ts index 6cdb9144..8a04a6ec 100644 --- a/src/backend/features/build-checker/checks.test.ts +++ b/src/backend/features/build-checker/checks.test.ts @@ -5,7 +5,6 @@ import { BuildIssueType } from "./issues"; import { DEFAULT_CANONICAL_CONFIGURATION } from "../configurations/canonical"; import { thumbnailUrl } from "../thumbnails/keys"; import { checkGroup, checkInsertable } from "./checks"; -import { configurationRecord } from "../../../__test_utils__/configuration-fixtures"; /** What uploadThumbnails returns: the element's default configuration. */ const THUMBNAILS: ThumbnailUrls = { @@ -56,14 +55,10 @@ describe("checkGroup", () => { }); }); -/** Only the part number matters to these checks. */ -const record = (partNumber?: string) => configurationRecord({ partNumber }); - describe("checkInsertable", () => { const HEALTHY_INSERTABLE = { vendors: [Vendor.REV], - thumbnailUrls: THUMBNAILS, - probes: [record("217-2600")] + thumbnailUrls: THUMBNAILS }; it("returns no issues when vendors are parsed and thumbnails generated", () => { @@ -85,36 +80,4 @@ describe("checkInsertable", () => { }); expect(issues).toEqual([{ type: BuildIssueType.THUMBNAIL_FAILED }]); }); - - it("warns when a vendor part indexed without a part number", () => { - const issues = checkInsertable({ - ...HEALTHY_INSERTABLE, - probes: [record(), record()] - }); - expect(issues).toEqual([{ type: BuildIssueType.NO_PART_NUMBER }]); - }); - - it("does not warn when only some configurations lack one", () => { - const issues = checkInsertable({ - ...HEALTHY_INSERTABLE, - probes: [record(), record("217-2600")] - }); - expect(issues).toEqual([]); - }); - - // Nobody sells it, so having no part number is the expected state. - it("does not warn about a custom part", () => { - const issues = checkInsertable({ - ...HEALTHY_INSERTABLE, - vendors: [Vendor.CUSTOM], - probes: [record()] - }); - expect(issues).toEqual([]); - }); - - // Nothing was probed, so there is nothing to conclude. - it("does not warn when the insertable is not indexed", () => { - const issues = checkInsertable({ ...HEALTHY_INSERTABLE, probes: [] }); - expect(issues).toEqual([]); - }); }); diff --git a/src/backend/features/build-checker/checks.ts b/src/backend/features/build-checker/checks.ts index 1643e83c..38ae9132 100644 --- a/src/backend/features/build-checker/checks.ts +++ b/src/backend/features/build-checker/checks.ts @@ -1,7 +1,6 @@ import { ThumbnailUrls } from "../thumbnails/types"; -import { Vendor, isCustomPart } from "../library/vendors"; +import { Vendor } from "../library/vendors"; import { addBuildIssue, BuildIssue, BuildIssueType } from "./issues"; -import type { PartMetadata } from "../configurations/models"; interface GroupCheckInput { /** Whether the Onshape document has a designated thumbnail tab/element. */ @@ -42,8 +41,6 @@ interface InsertableCheckInput { vendors: Vendor[]; /** The uploaded thumbnail URLs, or `null` when generation failed. */ thumbnailUrls: ThumbnailUrls | null; - /** Every probe of the element: its own, plus one per indexed configuration. */ - probes: (PartMetadata | null)[]; } /** @@ -63,28 +60,5 @@ export function checkInsertable(input: InsertableCheckInput): BuildIssue[] { issues = addBuildIssue(issues, { type: BuildIssueType.NO_VENDORS }); } - issues = addBuildIssue( - issues, - ...checkIndexedPartNumber(input.vendors, input.probes) - ); - return issues; } - -/** - * A custom part is expected to have no part number; anything a vendor sells - * should have one in at least one configuration. - */ -/** Every probe of an element: its own, plus one per indexed configuration. */ -export function checkIndexedPartNumber( - vendors: Vendor[], - probes: (PartMetadata | null)[] -): BuildIssue[] { - const read = probes.filter((probe) => probe !== null); - if (isCustomPart(vendors) || read.length === 0) { - return []; - } - return read.some((probe) => probe.partNumber) - ? [] - : [{ type: BuildIssueType.NO_PART_NUMBER }]; -} diff --git a/src/backend/features/build-checker/issues.ts b/src/backend/features/build-checker/issues.ts index 1d0065cd..0c948a65 100644 --- a/src/backend/features/build-checker/issues.ts +++ b/src/backend/features/build-checker/issues.ts @@ -21,7 +21,6 @@ export enum BuildIssueType { THUMBNAIL_FAILED = "thumbnail-failed", NO_THUMBNAIL_TAB = "no-thumbnail-tab", NO_VENDORS = "no-vendors", - NO_PART_NUMBER = "no-part-number", NO_PARTS = "no-parts", NO_UNHIDDEN_INSERTABLES = "no-unhidden-insertables", CONFIGURATION_LIMIT_EXCEEDED = "configuration-limit-exceeded", @@ -43,7 +42,6 @@ export type BuildIssue = | BuildIssueOf | BuildIssueOf | BuildIssueOf - | BuildIssueOf | BuildIssueOf | BuildIssueOf | BuildIssueOf @@ -62,8 +60,6 @@ export function getIssueDescription(issue: BuildIssue): string { return "No thumbnail tab set"; case BuildIssueType.NO_VENDORS: return "No vendors could be parsed"; - case BuildIssueType.NO_PART_NUMBER: - return "No part number in any configuration, though a vendor sells this"; case BuildIssueType.NO_PARTS: return "This part studio has no parts"; case BuildIssueType.NO_UNHIDDEN_INSERTABLES: @@ -97,7 +93,6 @@ export function getIssueSeverity(issue: BuildIssue): BuildIssueSeverity { case BuildIssueType.NO_THUMBNAIL_TAB: case BuildIssueType.CONFIGURATION_LIMIT_EXCEEDED: case BuildIssueType.MANUAL_INDEXING_REQUIRED: - case BuildIssueType.NO_PART_NUMBER: return BuildIssueSeverity.WARNING; case BuildIssueType.NO_VENDORS: return BuildIssueSeverity.INFO; diff --git a/src/backend/features/library/insertables/routes.ts b/src/backend/features/library/insertables/routes.ts index 4eda9937..c70440e2 100644 --- a/src/backend/features/library/insertables/routes.ts +++ b/src/backend/features/library/insertables/routes.ts @@ -39,7 +39,6 @@ import { FastenMateBuilder } from "../../../lib/onshape/objects/assembly-feature import { parseFastenInfo } from "../../load/parse-fasten"; import { getFastenQuery } from "./fasten-query"; import { addBuildIssue, clearBuildIssue } from "../../build-checker/issues"; -import { checkIndexedPartNumber } from "../../build-checker/checks"; export const insertableRoutes = getApp(); @@ -165,12 +164,7 @@ insertableRoutes.post( const buildIssues = addBuildIssue( clearBuildIssue(row.buildIssues, ...INDEXING_ISSUE_TYPES), ...indexed.buildIssues, - ...indexing.buildIssues, - // Vendors are read, not re-derived: the load path wrote them. - ...checkIndexedPartNumber(row.vendors, [ - indexed.partMetadata, - ...indexed.records - ]) + ...indexing.buildIssues ); // A configurations row exists exactly when the insertable is configurable. diff --git a/src/backend/features/load/load-insertable.ts b/src/backend/features/load/load-insertable.ts index 31b490ed..fc4eed65 100644 --- a/src/backend/features/load/load-insertable.ts +++ b/src/backend/features/load/load-insertable.ts @@ -117,11 +117,7 @@ export async function loadInsertable( const buildIssues = addBuildIssue( hasParts - ? checkInsertable({ - vendors, - thumbnailUrls, - probes: [recordsResult.partMetadata, ...recordsResult.records] - }) + ? checkInsertable({ vendors, thumbnailUrls }) : parts.buildIssues, ...recordsResult.buildIssues, ...indexing.buildIssues From 676c6d4465fa687929c90eb523e6f31d3fdb2988 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 12:48:16 +0000 Subject: [PATCH 17/40] Restore the part's own vendor as a url fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rename refactor replaced the resolution chain with an if-chain and dropped its middle arm, so a record naming McMaster-Carr resolved to the insertable's tagging instead — which its test says it should not. Put the arm back, in the new shape. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- src/backend/features/configurations/utils.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/backend/features/configurations/utils.ts b/src/backend/features/configurations/utils.ts index 578812ff..6de2ef8c 100644 --- a/src/backend/features/configurations/utils.ts +++ b/src/backend/features/configurations/utils.ts @@ -16,6 +16,7 @@ import { import { Vendor, getVendorPartUrl, + parseVendor, parseVendorFromPartNumber } from "../library/vendors"; import { LogicalOp, QuantityType, Unit } from "./enums"; @@ -100,8 +101,10 @@ export function getPartUrl( if (record.description && ABSOLUTE_URL.test(record.description)) { return record.description; } - // WCP-123 -> WCP + // WCP-123 -> WCP, then what the part says it is, then the insertable's + // tagging when it names one vendor and one only. let vendor = parseVendorFromPartNumber(record.partNumber); + vendor ??= parseVendor(record.vendor); if (!vendor && vendors.length === 1) { vendor = vendors[0]; } From 66345049df4cf4c96d2b45b1ae40c749cfa86519 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 12:59:00 +0000 Subject: [PATCH 18/40] Answer a typed letter, and store both spellings of a measurement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dropping every one-letter term took the first keystroke of every search with it, and answering as the caller types is most of what the search is for. Only a letter left behind by splitting a longer word is dropped now — the `n` and `a` of `n/a`, which was the case that wanted it. The same measurement reaches the library written both ways: one vendor writes .196 as .2 and the next writes .19. A name is stored and searched as both spellings, so either finds the part. Most numbers spell the same either way and cost nothing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- .../features/search/search-index.test.ts | 34 ++++++++-- src/backend/features/search/search-index.ts | 65 ++++++++++++++----- src/frontend/features/search/search.test.ts | 23 +++++++ 3 files changed, 100 insertions(+), 22 deletions(-) diff --git a/src/backend/features/search/search-index.test.ts b/src/backend/features/search/search-index.test.ts index d9029f47..dab6fa11 100644 --- a/src/backend/features/search/search-index.test.ts +++ b/src/backend/features/search/search-index.test.ts @@ -103,10 +103,23 @@ describe("tokenizeName", () => { expect(tokenizeName('1.125" OD, Flanged')).toEqual([ '1.13"', "OD", - "Flanged" + "Flanged", + '1.12"' ]); }); + // One vendor writes .196 as .2 and the next writes .19, so the part is + // stored as both and either spelling finds it. + it("spells a measurement as what it rounds to and what it starts", () => { + expect(tokenizeName(".196 ID Hub")).toEqual([ + "0.2", + "ID", + "Hub", + "0.19" + ]); + expect(tokenizeName('2.140" L')).toEqual(['2.14"', "L"]); + }); + // The mark is what makes `1"` a size rather than a prefix of 1.5 and 16T. it("keeps an inch mark on the number it measures", () => { expect(tokenizeName('1" Hex Shaft')).toEqual(['1"', "Hex", "Shaft"]); @@ -151,7 +164,8 @@ describe("tokenize", () => { expect(tokenize("TTB-0016-5/32", "partNames")).toEqual([ "TTB", "16", - "0.16" + "0.16", + "0.15" ]); }); }); @@ -160,9 +174,9 @@ describe("tokenize", () => { // typed a size or a part number. describe("tokenizeQuery", () => { it("offers the part number as typed, and as a name would read it", () => { - expect(tokenizeQuery("TTB-0016")).toEqual( - expect.arrayContaining(["ttb", "0016", "ttb-0016"]) - ); + expect( + tokenizeQuery("TTB-0016").map((term) => term.toLowerCase()) + ).toEqual(expect.arrayContaining(["ttb", "16", "0016", "ttb-0016"])); }); // `1` prefix-matches every number in the library, so a size is not split @@ -184,8 +198,14 @@ describe("tokenizeQuery", () => { } ); - it("keeps a lone digit, which names a size", () => { - expect(tokenizeQuery("1")).toEqual(["1"]); + // Answering as the caller types is the point, and the first keystroke is + // one character. + it.each(["l", "L", "1"])("still searches for a typed %s", (query) => { + expect(tokenizeQuery(query)).toEqual([query]); + }); + + it("keeps a letter typed beside another word", () => { + expect(tokenizeQuery("L bracket")).toEqual(["L", "bracket"]); }); }); diff --git a/src/backend/features/search/search-index.ts b/src/backend/features/search/search-index.ts index 614469c5..0c798ea2 100644 --- a/src/backend/features/search/search-index.ts +++ b/src/backend/features/search/search-index.ts @@ -33,17 +33,28 @@ function withoutLeadingZeros(digits: string): string { return digits.replace(/^0+(?=\d)/, ""); } -/** One 2-dp decimal, the single form every number is stored and queried as. */ -function toDecimal(value: number): string { - return String(Math.round(value * 100) / 100); -} +/** How a measurement is spelled to 2dp: what it rounds to, and what it starts. */ +type DecimalSpelling = (value: number) => string; + +const rounded: DecimalSpelling = (value) => + String(Math.round(value * 100) / 100); +const truncated: DecimalSpelling = (value) => + String(Math.trunc(value * 100) / 100); + +/** + * Both spellings of a measurement, since the library writes the same one either + * way: `.196` is written `.2` by one vendor and `.19` by the next. Storing and + * searching both is what lets either find the part. Most numbers spell the same + * both ways and so cost nothing. + */ +const DECIMAL_SPELLINGS: DecimalSpelling[] = [rounded, truncated]; /** * Rewrites numbers and fractions to one 2-dp decimal, at index and query time * alike — which is what lets the raw fragments go unstored. Names only: a part * number is an identifier, and 217-2600 is not two thousand six hundred. */ -function canonicalizeNumbers(text: string): string { +function canonicalizeNumbers(text: string, toDecimal: DecimalSpelling): string { return text.replace( NUMERIC_PATTERN, (match, mixedWhole, mixedNum, mixedDen, fracNum, fracDen) => { @@ -82,7 +93,7 @@ function canonicalizeNumbers(text: string): string { * lowercased, so a `.5` query lines up with a stored `"1/2 Bearing"`. */ export function normalizeForMatch(text: string): string { - return canonicalizeNumbers(text).toLowerCase(); + return canonicalizeNumbers(text, rounded).toLowerCase(); } /** @@ -90,9 +101,17 @@ export function normalizeForMatch(text: string): string { * stays on its number, so `1"` is a size rather than a prefix of `1.5` and `16t`. */ export function tokenizeName(text: string): string[] { + const tokens = new Set(); // Canonicalized before splitting: fractions span `/` and `-`. Casing stays, // since processTerm splits on camelCase. - return splitWithMarks(canonicalizeNumbers(text)); + for (const toDecimal of DECIMAL_SPELLINGS) { + for (const token of splitWithMarks( + canonicalizeNumbers(text, toDecimal) + )) { + tokens.add(token); + } + } + return Array.from(tokens); } /** Splits on `NAME_SEPARATORS`, keeping a `"` that measures its number. */ @@ -141,23 +160,39 @@ export function tokenize(text: string, field?: string): string[] { * text: the words of a name, and the literal a part number is indexed as. */ export function tokenizeQuery(text: string): string[] { - const tokens = new Set(tokenizeName(text)); - for (const word of text.trim().toLowerCase().split(/\s+/)) { + const tokens: string[] = []; + // The name reading keeps its case, for processTerm to split camelCase on, + // so the literal reading of the same word is a duplicate rather than a + // second term to search. + const seen = new Set(); + for (const word of text.trim().split(/\s+/)) { if (!word) { continue; } // Segments only for something carrying a letter, which is what a part // number does: splitting a bare `1/2` would search `1`, and a prefix // that short matches every number in the library. - for (const token of /[a-z]/.test(word) + const literal = /[a-z]/i.test(word) ? tokenizePartNumber(word) - : [word]) { - tokens.add(token); + : [word.toLowerCase()]; + for (const token of [...tokenizeName(word), ...literal]) { + if (isStrayLetter(token, word) || seen.has(token.toLowerCase())) { + continue; + } + seen.add(token.toLowerCase()); + tokens.push(token); } } - // A lone letter is the leftover of splitting something like `n/a`, and as a - // prefix it matches most of the library. A lone digit is a size, so it stays. - return Array.from(tokens).filter((token) => !/^[a-z]$/i.test(token)); + return tokens; +} + +/** + * Whether a token is a letter left behind by splitting a longer word, as `n/a` + * leaves `n` and `a`: as a prefix it matches most of the library. A letter the + * caller actually typed is a search, and answering it as they type is the point. + */ +function isStrayLetter(token: string, word: string): boolean { + return word.length > 1 && token.length === 1 && /[a-z]/i.test(token); } /** diff --git a/src/frontend/features/search/search.test.ts b/src/frontend/features/search/search.test.ts index 46ab0bc6..78a4c763 100644 --- a/src/frontend/features/search/search.test.ts +++ b/src/frontend/features/search/search.test.ts @@ -222,6 +222,29 @@ describe("doSearch size matching", () => { }); }); +// The same measurement is written .196, .2 and .19 across the library, so a +// part is stored as both spellings and either one finds it. +describe("doSearch measurements", () => { + const searchDb = buildSearchDb(library("MotionX Hub"), { + i1: [record("WCP-1", {}, ".196 ID x SplineXL OD")] + }); + + it.each([".196", ".19", ".2", "0.19"])("finds the part by %s", (query) => { + expect(search(searchDb, query).hits[0]?.id).toBe("i1"); + }); +}); + +// Results arrive as the caller types, and the first keystroke is one letter. +describe("doSearch single letters", () => { + const searchDb = buildSearchDb(library("Hex Standoff"), { + i1: [record("TTB-0016", {}, "Standoff")] + }); + + it.each(["h", "s", "t"])("answers a typed %s", (query) => { + expect(search(searchDb, query).hits[0]?.id).toBe("i1"); + }); +}); + // A part number is a code: it retrieves its part whole, by either half, and // with the zeros and separators it was written with. describe("doSearch part numbers", () => { From de85dc95966f53da8f9bff40c4f9b6b0d364b75f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 13:07:20 +0000 Subject: [PATCH 19/40] Answer the placeholder with nothing, rather than with its letters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stray-letter rule was the wrong shape for what it was protecting against. Nothing carries `n/a` — ingest drops it — but typing it split into `n` and `a`, and a one-letter prefix answers with most of the library. The placeholder is now recognized where it is typed, by the same rule that drops it where it is stored, so it finds nothing and the rest of the query is read as usual. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- .../configurations/part-number.test.ts | 15 ++++++++++++- .../features/configurations/part-number.ts | 7 ++++++- .../features/search/search-index.test.ts | 15 +++++++------ src/backend/features/search/search-index.ts | 21 ++++++++----------- 4 files changed, 38 insertions(+), 20 deletions(-) diff --git a/src/backend/features/configurations/part-number.test.ts b/src/backend/features/configurations/part-number.test.ts index ce96049a..85438a04 100644 --- a/src/backend/features/configurations/part-number.test.ts +++ b/src/backend/features/configurations/part-number.test.ts @@ -1,5 +1,18 @@ import { describe, expect, it } from "vitest"; -import { meaningfulPartNumber } from "./part-number"; +import { isPlaceholderPartNumber, meaningfulPartNumber } from "./part-number"; + +describe("isPlaceholderPartNumber", () => { + it.each(["N/A", "n/a", " N/a "])("reads %s as the placeholder", (value) => { + expect(isPlaceholderPartNumber(value)).toBe(true); + }); + + it.each(["NA-1234", "n", "WCP-1025"])( + "reads %s as a part number", + (value) => { + expect(isPlaceholderPartNumber(value)).toBe(false); + } + ); +}); describe("meaningfulPartNumber", () => { it.each(["N/A", "n/a", " N/a "])("hides the placeholder %s", (value) => { diff --git a/src/backend/features/configurations/part-number.ts b/src/backend/features/configurations/part-number.ts index e462ff58..66f98ff5 100644 --- a/src/backend/features/configurations/part-number.ts +++ b/src/backend/features/configurations/part-number.ts @@ -3,6 +3,11 @@ import { clean, equalsIgnoreCase } from "../../lib/text"; /** Admins write this in where a generic part has no real number to give. */ const PLACEHOLDER_PART_NUMBER = new RegExp("^n/a$", "i"); +/** Whether text is the placeholder, which identifies nothing anywhere. */ +export function isPlaceholderPartNumber(text: string): boolean { + return PLACEHOLDER_PART_NUMBER.test(text.trim()); +} + /** * The part number when it identifies the part, and nothing when it doesn't — a * placeholder, or a repeat of the name it sits under. One rule for indexing and @@ -13,7 +18,7 @@ export function meaningfulPartNumber( name?: string | null ): string | undefined { const text = clean(partNumber); - if (!text || PLACEHOLDER_PART_NUMBER.test(text)) { + if (!text || isPlaceholderPartNumber(text)) { return undefined; } return equalsIgnoreCase(text, name) ? undefined : text; diff --git a/src/backend/features/search/search-index.test.ts b/src/backend/features/search/search-index.test.ts index dab6fa11..8b31dac3 100644 --- a/src/backend/features/search/search-index.test.ts +++ b/src/backend/features/search/search-index.test.ts @@ -191,12 +191,15 @@ describe("tokenizeQuery", () => { // Splitting the placeholder leaves `n` and `a`, and a one-letter prefix // matches most of the library. - it.each(["n/a", "N/A"])( - "drops the letters left over from splitting %s", - (query) => { - expect(tokenizeQuery(query)).toEqual(["n/a"]); - } - ); + // Nothing carries the placeholder, and searching its letters would answer + // with whatever starts with `n` or `a`. + it.each(["n/a", "N/A"])("has nothing to search for in %s", (query) => { + expect(tokenizeQuery(query)).toEqual([]); + }); + + it("still reads the rest of a query the placeholder is in", () => { + expect(tokenizeQuery("n/a bearing")).toEqual(["bearing"]); + }); // Answering as the caller types is the point, and the first keystroke is // one character. diff --git a/src/backend/features/search/search-index.ts b/src/backend/features/search/search-index.ts index 0c798ea2..f5bb3935 100644 --- a/src/backend/features/search/search-index.ts +++ b/src/backend/features/search/search-index.ts @@ -7,7 +7,10 @@ import { LibraryOut } from "../library/contract"; import { Vendor } from "../library/vendors"; import { ConfigurationRecord, SearchRecord } from "../configurations/models"; import { getPartUrl } from "../configurations/utils"; -import { meaningfulPartNumber } from "../configurations/part-number"; +import { + isPlaceholderPartNumber, + meaningfulPartNumber +} from "../configurations/part-number"; import { clean } from "../../lib/text"; /** Where a name breaks: punctuation and space, plus a quote used as a quote. */ @@ -166,7 +169,10 @@ export function tokenizeQuery(text: string): string[] { // second term to search. const seen = new Set(); for (const word of text.trim().split(/\s+/)) { - if (!word) { + // Ingest drops the placeholder, so nothing carries it; typed, it is + // still the word for a part number nobody has, and searching its + // letters would answer with whatever starts with `n` or `a`. + if (!word || isPlaceholderPartNumber(word)) { continue; } // Segments only for something carrying a letter, which is what a part @@ -176,7 +182,7 @@ export function tokenizeQuery(text: string): string[] { ? tokenizePartNumber(word) : [word.toLowerCase()]; for (const token of [...tokenizeName(word), ...literal]) { - if (isStrayLetter(token, word) || seen.has(token.toLowerCase())) { + if (seen.has(token.toLowerCase())) { continue; } seen.add(token.toLowerCase()); @@ -186,15 +192,6 @@ export function tokenizeQuery(text: string): string[] { return tokens; } -/** - * Whether a token is a letter left behind by splitting a longer word, as `n/a` - * leaves `n` and `a`: as a prefix it matches most of the library. A letter the - * caller actually typed is a search, and answering it as they type is the point. - */ -function isStrayLetter(token: string, word: string): boolean { - return word.length > 1 && token.length === 1 && /[a-z]/i.test(token); -} - /** * Adds the words inside a compound term, so `MAXSpline` is found by `spline`. * A part number is left whole: its segments are already separate tokens. From 0652c362dd0cc34859c9d8931ecf482fc65afcef Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 13:31:26 +0000 Subject: [PATCH 20/40] Push the search and library sections from an if, not a ternary The two branches were a 35-line ternary argument, which read as one expression when it is really the same push either way. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- .../routes/app/library/$libraryId/index.tsx | 65 +++++++++---------- 1 file changed, 31 insertions(+), 34 deletions(-) diff --git a/src/frontend/routes/app/library/$libraryId/index.tsx b/src/frontend/routes/app/library/$libraryId/index.tsx index 841f4dc6..17227884 100644 --- a/src/frontend/routes/app/library/$libraryId/index.tsx +++ b/src/frontend/routes/app/library/$libraryId/index.tsx @@ -73,40 +73,37 @@ function HomeList(): ReactNode { // One slot below favorites, showing search results while a query is active // and the library otherwise. The differing `value` remounts it on the swap. - sections.push( - uiState.searchQuery - ? { - value: "search", - icon: ( - - ), - title: , - panel: ( - - ), - opened: isSearchOpen, - setOpened: setIsSearchOpen - } - : { - value: "library", - icon: ( - - ), - title: , - panel: , - opened: uiState.isLibraryOpen, - setOpened: (opened) => setUiState({ isLibraryOpen: opened }) - } - ); + if (uiState.searchQuery) { + sections.push({ + value: "search", + icon: ( + + ), + title: , + panel: ( + + ), + opened: isSearchOpen, + setOpened: setIsSearchOpen + }); + } else { + sections.push({ + value: "library", + icon: ( + + ), + title: , + panel: , + opened: uiState.isLibraryOpen, + setOpened: (opened) => setUiState({ isLibraryOpen: opened }) + }); + } const handleChange = (opened: string[]) => { for (const section of sections) { From 764b07343a65e9c8029599b9ca336427b8a19a9d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 14:15:52 +0000 Subject: [PATCH 21/40] Name the colors once, and give a themed icon its own component Seventeen icons were each wrapped in a Box to take a Mantine color, and the colors themselves were written at every control: c="red" here, color="yellow" there. AppIcon is that wrapper, and StatusColor names what each color means, so an error looks like an error everywhere and the theme's variables stay out of the markup. The build-status icons collapse into one mapping from severity, since their four branches differed only in the icon and the color. Elements become parts in the group's zero state, which is what a library holds. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- src/frontend/components/alerts.tsx | 13 +- src/frontend/components/app-icon.tsx | 43 ++++++ src/frontend/components/app-menu.tsx | 4 +- src/frontend/components/app-navbar.tsx | 8 +- src/frontend/components/app-title.tsx | 13 +- src/frontend/components/app-zero-state.tsx | 7 +- .../build-status/components/build-status.tsx | 140 ++++++++---------- .../favorites/components/favorite-button.tsx | 18 ++- .../favorites/components/favorites-list.tsx | 10 +- .../library/components/card-components.tsx | 28 ++-- .../library/components/coming-soon.tsx | 8 +- .../library/components/group-card.tsx | 8 +- .../search/components/search-errors.tsx | 19 ++- .../settings/components/settings-menu.tsx | 12 +- src/frontend/lib/style-constants.ts | 23 +++ .../library/$libraryId/groups/$groupId.tsx | 10 +- 16 files changed, 228 insertions(+), 136 deletions(-) create mode 100644 src/frontend/components/app-icon.tsx diff --git a/src/frontend/components/alerts.tsx b/src/frontend/components/alerts.tsx index db40e178..a8d6dd7e 100644 --- a/src/frontend/components/alerts.tsx +++ b/src/frontend/components/alerts.tsx @@ -1,8 +1,9 @@ import { modals } from "@mantine/modals"; -import { Box, Text } from "@mantine/core"; +import { Text } from "@mantine/core"; import { WarningIcon } from "@phosphor-icons/react"; import { AppTitle } from "./app-title"; -import { IconSize } from "../lib/style-constants"; +import { IconSize, StatusColor } from "../lib/style-constants"; +import { AppIcon } from "./app-icon"; interface OpenWarningAlertProps { title: string; @@ -14,10 +15,10 @@ function openWarningAlert(props: OpenWarningAlertProps): void { title: ( } title={props.title} diff --git a/src/frontend/components/app-icon.tsx b/src/frontend/components/app-icon.tsx new file mode 100644 index 00000000..1b7e2253 --- /dev/null +++ b/src/frontend/components/app-icon.tsx @@ -0,0 +1,43 @@ +import { Box } from "@mantine/core"; +import type { Icon, IconWeight } from "@phosphor-icons/react"; +import { ComponentPropsWithRef, ReactNode } from "react"; +import { IconSize, StatusColor } from "../lib/style-constants"; + +export interface AppIconProps + // Rendered through Box, which owns these two as style props. + extends Omit, "color" | "display"> { + icon: Icon; + /** @default IconSize.SMALL */ + size?: IconSize; + /** A theme color; without one the icon takes the surrounding text's. */ + color?: StatusColor | string; + /** @default "regular" */ + weight?: IconWeight; + /** What a screen reader calls an icon that carries meaning on its own. */ + label?: string; +} + +/** + * A Phosphor icon in a theme color. Box is what resolves the color name, and + * sizing goes through `fz` because Box writes its own `style`, which is what + * would drop the icon's own. + */ +export function AppIcon({ + icon, + size = IconSize.SMALL, + color, + weight, + label, + ...others +}: AppIconProps): ReactNode { + return ( + + ); +} diff --git a/src/frontend/components/app-menu.tsx b/src/frontend/components/app-menu.tsx index 52934994..b0834f4d 100644 --- a/src/frontend/components/app-menu.tsx +++ b/src/frontend/components/app-menu.tsx @@ -1,7 +1,7 @@ import { PropsWithChildren, ReactNode } from "react"; import { FloatingPosition, Menu, ActionIcon } from "@mantine/core"; import { DotsThreeIcon } from "@phosphor-icons/react"; -import { IconSize } from "../lib/style-constants"; +import { IconSize, StatusColor } from "../lib/style-constants"; interface AppContextMenuProps { menuItems: ReactNode; @@ -77,7 +77,7 @@ export function MenuButton(props: MenuButtonProps): ReactNode { e.stopPropagation()} diff --git a/src/frontend/components/app-navbar.tsx b/src/frontend/components/app-navbar.tsx index b26dd1c9..bfbd7ae5 100644 --- a/src/frontend/components/app-navbar.tsx +++ b/src/frontend/components/app-navbar.tsx @@ -15,7 +15,9 @@ import { BORDER, FRAME_BACKGROUND, IconSize, - PrimaryColor + PrimaryColor, + RADIUS, + StatusColor } from "../lib/style-constants"; import { ReactNode, RefObject, useRef } from "react"; import { useNavigate } from "@tanstack/react-router"; @@ -129,7 +131,7 @@ function FrcDesignBookIcon(): ReactNode { bg={PrimaryColor.FILLED} c={PrimaryColor.CONTRAST} style={{ - borderRadius: "var(--mantine-radius-sm)", + borderRadius: RADIUS, display: "grid", placeItems: "center" }} @@ -216,7 +218,7 @@ export function SettingsButton() { return ( {subtitle} @@ -72,7 +77,7 @@ interface MenuTitleProps { * number is what identifies what gets inserted. */ export function MenuTitle(props: MenuTitleProps): ReactNode { const { name, record, icon } = props; - const partNumber = displayPartNumber(record?.partNumber, name); + const partNumber = meaningfulPartNumber(record?.partNumber, name); return ( + ); interface ZeroStateProps { diff --git a/src/frontend/features/build-status/components/build-status.tsx b/src/frontend/features/build-status/components/build-status.tsx index 249359ae..631671e4 100644 --- a/src/frontend/features/build-status/components/build-status.tsx +++ b/src/frontend/features/build-status/components/build-status.tsx @@ -1,6 +1,5 @@ import { Badge, - Box, Divider, Group, HoverCard, @@ -21,7 +20,6 @@ import { XIcon } from "@phosphor-icons/react"; import { - ComponentPropsWithRef, ReactNode, createContext, use, @@ -59,10 +57,17 @@ import { MAX_COUNTED_CONFIGURATIONS, MAX_PART_NUMBER_CONFIGURATIONS } from "@backend/features/configurations/combinations"; -import { FontWeight, IconSize } from "../../../lib/style-constants"; +import { + FontWeight, + IconSize, + RADIUS, + StatusColor, + statusBackground +} from "../../../lib/style-constants"; import { RequireAccessLevel } from "../../auth/access-level"; import { useBuildStatusQuery } from "../queries"; -import { useJobStatusQuery } from "../../library/queries"; +import { useIsJobRunning } from "../../library/queries"; +import { AppIcon, type AppIconProps } from "../../../components/app-icon"; import { useSetVisibilityMutation, useToggleInsertAndFastenMutation, @@ -114,65 +119,44 @@ function useGroupBuildIssues( }, [groupStatus, insertableStatuses]); } -interface IssueIconProps - // Rendered through Box, which owns these two as style props. - extends Omit, "color" | "display"> { +interface IssueIconProps extends Omit { /** The severity to render, or null if all checks pass. */ severity: BuildIssueSeverity | null; - /** @default IconSize.SMALL */ - size?: number; } -/** Renders the icon for a build-issue severity in its severity color. */ -export function IssueIcon({ - severity, - ref, - ...others -}: IssueIconProps): ReactNode { +/** The icon each severity is drawn as; `ok` is a build with nothing to say. */ +const SEVERITY_ICONS = { + [BuildIssueSeverity.ERROR]: WarningOctagonIcon, + [BuildIssueSeverity.WARNING]: WarningIcon, + [BuildIssueSeverity.INFO]: InfoIcon, + ok: CheckIcon +}; + +/** The color a severity is spoken in; null is a build with nothing to say. */ +function severityColor(severity: BuildIssueSeverity | null): StatusColor { switch (severity) { case BuildIssueSeverity.ERROR: - return ( - - ); + return StatusColor.ERROR; case BuildIssueSeverity.WARNING: - return ( - - ); + return StatusColor.WARNING; case BuildIssueSeverity.INFO: - return ( - - ); + return StatusColor.INFO; case null: - return ( - - ); + return StatusColor.SUCCESS; } } +/** Renders the icon for a build-issue severity in its severity color. */ +export function IssueIcon({ severity, ...others }: IssueIconProps): ReactNode { + return ( + + ); +} + interface BuildStatusCardProps { /** The group/insertable name shown in the header. */ name: string; @@ -249,7 +233,7 @@ function BuildStatusHoverCard({ hoverMenu }: BuildStatusBadgeProps): ReactNode { const maxSeverity = getMaxSeverity(issues); - const jobRunning = useJobStatusQuery().data?.running ?? false; + const jobRunning = useIsJobRunning(); // Remounting is the only way to close an uncontrolled HoverCard on demand. const [cardKey, setCardKey] = useState(0); @@ -327,7 +311,7 @@ function LastModified({ }: { lastLoadedAt: number | null; }): ReactNode { - const jobRunning = useJobStatusQuery().data?.running ?? false; + const jobRunning = useIsJobRunning(); if (jobRunning) { return ( @@ -343,7 +327,7 @@ function LastModified({ @@ -364,7 +348,7 @@ function SeverityBadges({ issues }: { issues: BuildIssue[] }): ReactNode { } > All checks pass @@ -424,11 +408,14 @@ function CountBadge({ ); } -function countSeverities(issues: BuildIssue[]): { +/** How many issues of each severity a build carries. */ +interface SeverityCounts { error: number; warning: number; info: number; -} { +} + +function countSeverities(issues: BuildIssue[]): SeverityCounts { const counts = { error: 0, warning: 0, info: 0 }; for (const issue of issues) { switch (getIssueSeverity(issue)) { @@ -469,7 +456,7 @@ function IssueCallout({ issue }: { issue: BuildIssue }): ReactNode { p="xs" style={{ backgroundColor: severityBackground(severity), - borderRadius: "var(--mantine-radius-sm)" + borderRadius: RADIUS }} > {/* Nudge the icon down so it aligns with the first line of text. */} @@ -484,14 +471,7 @@ function IssueCallout({ issue }: { issue: BuildIssue }): ReactNode { /** The light background tint for a build-issue callout. */ function severityBackground(severity: BuildIssueSeverity): string { - switch (severity) { - case BuildIssueSeverity.ERROR: - return "var(--mantine-color-red-light)"; - case BuildIssueSeverity.WARNING: - return "var(--mantine-color-yellow-light)"; - case BuildIssueSeverity.INFO: - return "var(--mantine-color-blue-light)"; - } + return statusBackground(severityColor(severity)); } /** Build-status badge pre-wired for an insertable. */ @@ -571,7 +551,7 @@ export function GroupStatusBadge({ /** A dimmed section header, e.g. "Admin" or "Parsed". */ function SectionHeader({ children }: { children: ReactNode }): ReactNode { return ( - + {children} ); @@ -590,7 +570,7 @@ function ControlRow(props: {
{props.label} - + {props.description}
@@ -899,10 +879,10 @@ function ExcludedFromPropertiesIcon({ withArrow events={{ hover: true, focus: true, touch: true }} > -
@@ -924,7 +904,7 @@ function ParameterTypeBadge({ : getParameterTypeLabel(parameter.type); const badge = ( - + {label} ); @@ -978,9 +958,17 @@ function ParsedRow({ function StateValue({ value }: { value: StateRowValue }): ReactNode { if (value.kind === "bool") { return value.value ? ( - + ) : ( - + ); } @@ -994,7 +982,7 @@ function StateValue({ value }: { value: StateRowValue }): ReactNode { if (value.vendors.length === 0) { return ( - + None ); @@ -1006,7 +994,7 @@ function StateValue({ value }: { value: StateRowValue }): ReactNode { key={vendor} size="sm" variant="light" - color="blue" + color={StatusColor.INFO} title={getVendorName(vendor)} > {vendor} diff --git a/src/frontend/features/favorites/components/favorite-button.tsx b/src/frontend/features/favorites/components/favorite-button.tsx index 9d8c0a72..013c9c40 100644 --- a/src/frontend/features/favorites/components/favorite-button.tsx +++ b/src/frontend/features/favorites/components/favorite-button.tsx @@ -1,6 +1,6 @@ -import { ActionIcon, Box, Menu } from "@mantine/core"; +import { ActionIcon, Menu } from "@mantine/core"; import { HeartIcon, HeartBreakIcon } from "@phosphor-icons/react"; -import { IconSize } from "../../../lib/style-constants"; +import { IconSize, StatusColor } from "../../../lib/style-constants"; import { useMutation } from "@tanstack/react-query"; import { ReactNode, useState } from "react"; import { apiDelete, apiPost } from "../../../lib/api-client"; @@ -21,6 +21,7 @@ import { } from "../../library/library-path"; import { favoritesQueryKey } from "../../../lib/query-keys"; import { useRefreshFavorites } from "../../../lib/refresh"; +import { AppIcon } from "../../../components/app-icon"; enum Operation { ADD, @@ -149,7 +150,7 @@ export function FavoriteButton(props: FavoriteButtonProps): ReactNode { return ( { event.stopPropagation(); @@ -227,7 +228,12 @@ export function FavoriteIcon(props: FavoriteIconProps): ReactNode { // fz, not size: Box builds its own `style`, dropping the font-size that // Phosphor's `size` sets, which shrank the icon to 1em. return full ? ( - + ) : ( ); @@ -242,5 +248,7 @@ interface UnfavoriteIconProps { export function UnfavoriteIcon(props: UnfavoriteIconProps): ReactNode { const { size = IconSize.SMALL } = props; - return ; + return ( + + ); } diff --git a/src/frontend/features/favorites/components/favorites-list.tsx b/src/frontend/features/favorites/components/favorites-list.tsx index c17fc703..d7959c43 100644 --- a/src/frontend/features/favorites/components/favorites-list.tsx +++ b/src/frontend/features/favorites/components/favorites-list.tsx @@ -1,7 +1,6 @@ -import { Box } from "@mantine/core"; import { useAccessData } from "../../auth/access-level"; import { HeartBreakIcon } from "@phosphor-icons/react"; -import { IconSize } from "../../../lib/style-constants"; +import { IconSize, StatusColor } from "../../../lib/style-constants"; import { ReactNode } from "react"; import { FilteredInsertables, @@ -26,6 +25,7 @@ import { useFavoritesQuery } from "../queries"; import { useLibraryQuery } from "../../library/queries"; import { useSearchDbQuery } from "../../search/queries"; import { hasEditorAccess } from "@backend/features/auth/access-level"; +import { AppIcon } from "../../../components/app-icon"; /** * A list of current favorite cards. @@ -44,10 +44,10 @@ export function FavoritesList(): ReactNode { } /> diff --git a/src/frontend/features/library/components/card-components.tsx b/src/frontend/features/library/components/card-components.tsx index f3e49147..bbcee7ee 100644 --- a/src/frontend/features/library/components/card-components.tsx +++ b/src/frontend/features/library/components/card-components.tsx @@ -1,4 +1,4 @@ -import { Anchor, Box, Group, Menu, Stack, Table, Text } from "@mantine/core"; +import { Anchor, Group, Menu, Stack, Table, Text } from "@mantine/core"; import { ArrowSquareOutIcon, EyeSlashIcon, @@ -6,8 +6,8 @@ import { LinkIcon, PlusIcon } from "@phosphor-icons/react"; -import { IconSize } from "../../../lib/style-constants"; -import { displayPartNumber } from "../../../lib/part-number"; +import { IconSize, StatusColor } from "../../../lib/style-constants"; +import { meaningfulPartNumber } from "@backend/features/configurations/part-number"; import { copyUrlToClipboard, makeUrl, openUrlInNewTab } from "../../../lib/url"; import { PropsWithChildren, ReactNode, useCallback } from "react"; import { AppContextMenu, MenuButton } from "../../../components/app-menu"; @@ -32,6 +32,7 @@ import { ElementType } from "@backend/lib/onshape/element-type"; import { ParameterValues } from "@backend/features/configurations/models"; import { useSearch } from "@tanstack/react-router"; import { RequireAccessLevel } from "../../auth/access-level"; +import { AppIcon } from "../../../components/app-icon"; interface OpenDocumentItemsProps { /** Any Onshape path; a shell group's stops at the document. */ @@ -176,11 +177,11 @@ export function CardTitle(props: CardTitleProps) { {/* After the badge: toggling visibility would otherwise shift the badge, dragging its open hover card out from under the cursor. */} {showHiddenTag && ( - )} @@ -202,14 +203,21 @@ function PartNameAndNumber(props: PartNameAndNumberProps): ReactNode { searchHit?.partName?.toLowerCase() !== title.toLowerCase() ? searchHit?.partName : undefined; - const partNumber = displayPartNumber(searchHit?.partNumber, title); + const partNumber = meaningfulPartNumber(searchHit?.partNumber, title); if (!partName && !partNumber) { return null; } return ( - + {partName && ( } > Admin options diff --git a/src/frontend/features/library/components/coming-soon.tsx b/src/frontend/features/library/components/coming-soon.tsx index 8c884fb2..936278b2 100644 --- a/src/frontend/features/library/components/coming-soon.tsx +++ b/src/frontend/features/library/components/coming-soon.tsx @@ -1,9 +1,9 @@ -import { Box } from "@mantine/core"; import { HammerIcon } from "@phosphor-icons/react"; import { ReactNode } from "react"; import { IconSize, PrimaryColor } from "../../../lib/style-constants"; import { PageMessage } from "../../../components/app-zero-state"; import { getLibraryName, useLibraryId } from "../library-path"; +import { AppIcon } from "../../../components/app-icon"; /** Stands in for a library that is announced but has nothing to show yet. */ export function ComingSoon(): ReactNode { @@ -11,10 +11,10 @@ export function ComingSoon(): ReactNode { return ( } title={`${getLibraryName(libraryId)} is coming soon`} diff --git a/src/frontend/features/library/components/group-card.tsx b/src/frontend/features/library/components/group-card.tsx index f8fb8b4e..aaf1802b 100644 --- a/src/frontend/features/library/components/group-card.tsx +++ b/src/frontend/features/library/components/group-card.tsx @@ -5,7 +5,7 @@ import { EyeSlashIcon, TrashIcon } from "@phosphor-icons/react"; -import { IconSize } from "../../../lib/style-constants"; +import { IconSize, StatusColor } from "../../../lib/style-constants"; import { useNavigate } from "@tanstack/react-router"; import { PropsWithChildren, ReactNode } from "react"; import { GroupOut, LibraryOut } from "@backend/features/library/contract"; @@ -141,7 +141,7 @@ function ShowAllElementsMenuItem({ const mutation = useSetVisibilityMutation(insertableOrder, true); return ( } onClick={() => mutation.mutate()} > @@ -158,7 +158,7 @@ function HideAllElementsMenuItem({ const mutation = useSetVisibilityMutation(insertableOrder, false); return ( } onClick={() => mutation.mutate()} > @@ -185,7 +185,7 @@ function DeleteGroupMenuItem({ groupId }: { groupId: string }): ReactNode { return ( } - color="red" + color={StatusColor.ERROR} onClick={() => mutation.mutate()} > Delete diff --git a/src/frontend/features/search/components/search-errors.tsx b/src/frontend/features/search/components/search-errors.tsx index 3edf867c..0fa884e7 100644 --- a/src/frontend/features/search/components/search-errors.tsx +++ b/src/frontend/features/search/components/search-errors.tsx @@ -1,16 +1,17 @@ -import { Alert, Box, Button, Group, Text } from "@mantine/core"; +import { Alert, Button, Group, Text } from "@mantine/core"; import { HeartBreakIcon, InfoIcon, MagnifyingGlassIcon } from "@phosphor-icons/react"; -import { IconSize } from "../../../lib/style-constants"; +import { IconSize, StatusColor } from "../../../lib/style-constants"; import { ReactNode } from "react"; import { ClearFiltersButton } from "../../settings/components/vendor-filters"; import { FilterResult, ObjectLabel, plural } from "../search"; import { useNavigate } from "@tanstack/react-router"; import { SectionError } from "../../../components/app-zero-state"; import { useLibraryId } from "../../library/library-path"; +import { AppIcon } from "../../../components/app-icon"; function getGroupString(filtered: FilterResult, objectLabel: ObjectLabel) { if (filtered.byGroup > 1) { @@ -42,7 +43,7 @@ interface FilterCalloutProps { function Callout(props: { text: string; action: ReactNode }): ReactNode { return ( } styles={{ body: { minWidth: 0 } }} @@ -92,13 +93,17 @@ export function NoSearchResultError( const icon = objectLabel === "search result" ? ( - ) : ( - + ); if (filtered.byGroup > 0) { diff --git a/src/frontend/features/settings/components/settings-menu.tsx b/src/frontend/features/settings/components/settings-menu.tsx index 85219c03..4efd6d7a 100644 --- a/src/frontend/features/settings/components/settings-menu.tsx +++ b/src/frontend/features/settings/components/settings-menu.tsx @@ -1,7 +1,11 @@ import { DEFAULT_SETTINGS, Theme } from "@backend/features/settings/settings"; import { Button, Divider, Group, Select, Text, Title } from "@mantine/core"; import { SignOutIcon } from "@phosphor-icons/react"; -import { FontWeight, IconSize } from "../../../lib/style-constants"; +import { + FontWeight, + IconSize, + StatusColor +} from "../../../lib/style-constants"; import { ReactNode } from "react"; import { AccessLevel, @@ -18,11 +22,13 @@ import { } from "../../auth/access-level"; import { startSignOut } from "../../auth/sign-out"; import { useGetUiState, useSetUiState } from "../../../lib/ui-state"; -import { FEEDBACK_FORM_URL } from "../../../lib/url"; import { useIsConnectedToOnshape } from "../../../lib/onshape-params"; import { useLibraryId } from "../../library/library-path"; import { ReloadGroupsButton } from "../../library/components/reload-groups-button"; +/** The FRCDesign feedback form, which the setting below opens. */ +const FEEDBACK_FORM_URL = "https://forms.gle/WVXUwnrrpLGKdiBx9"; + /** * A labeled row holding a single setting control. */ @@ -122,7 +128,7 @@ function UserSettings(): ReactNode { diff --git a/src/frontend/lib/style-constants.ts b/src/frontend/lib/style-constants.ts index 6a4df66a..65d2f257 100644 --- a/src/frontend/lib/style-constants.ts +++ b/src/frontend/lib/style-constants.ts @@ -59,6 +59,23 @@ export const FRAME_BACKGROUND = */ export const TITLE_ICON_NUDGE = { transform: "translateY(-1px)" }; +/** Holds an icon or badge at its own size beside text that can outgrow the row. */ +export const NO_SHRINK = { flexShrink: 0 }; + +/** + * Paints an image in the current text color rather than its own. The url needs + * quoting: Vite inlines an asset as a data uri, which can contain apostrophes. + */ +export function maskedImage(url: string) { + return { + backgroundColor: "currentColor", + maskImage: `url("${url}")`, + maskSize: "contain", + maskRepeat: "no-repeat", + maskPosition: "center" + }; +} + /** * One height for a section header, set rather than left to the content: an * accordion is sized by its label, a group header by its menu button. From 1f020f9814ca7297cca9a07cb519a9e95dfa37d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 13:24:43 +0000 Subject: [PATCH 38/40] Give every component a props interface and one place for defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A component's props were declared three ways — an interface, an object literal at the signature, a destructure at the signature — so a reader had to find out which before reading the props. They are all interfaces now. Defaults were split the same way: destructure the rest of the props, then a `props.x ?? fallback` line below for the optional ones, which reads as though it were doing something more than defaulting. They land in the destructure. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- src/frontend/components/app-modal.tsx | 5 +++-- src/frontend/components/app-zero-state.tsx | 15 ++++++++++----- src/frontend/features/auth/access-level.tsx | 12 ++++++++---- .../library/components/card-components.tsx | 7 +++++-- .../features/library/components/group-card.tsx | 7 ++++++- .../library/components/reload-groups-button.tsx | 2 +- .../features/search/components/search-errors.tsx | 9 +++++++-- .../settings/components/settings-menu.tsx | 13 +++++++++---- .../settings/components/vendor-filters.tsx | 11 +++-------- .../app/library/$libraryId/groups/$groupId.tsx | 7 ++++++- .../routes/app/library/$libraryId/index.tsx | 7 ++++++- 11 files changed, 64 insertions(+), 31 deletions(-) diff --git a/src/frontend/components/app-modal.tsx b/src/frontend/components/app-modal.tsx index 223bcc2e..22bb5696 100644 --- a/src/frontend/components/app-modal.tsx +++ b/src/frontend/components/app-modal.tsx @@ -9,9 +9,10 @@ interface AppModalBodyProps extends PropsWithChildren { /** A modal's content, padded away from the header and footer framing it. */ export function AppModalBody(props: AppModalBodyProps): ReactNode { + const { gap = "sm", children } = props; return ( - - {props.children} + + {children} ); } diff --git a/src/frontend/components/app-zero-state.tsx b/src/frontend/components/app-zero-state.tsx index 24c5da80..42289a4a 100644 --- a/src/frontend/components/app-zero-state.tsx +++ b/src/frontend/components/app-zero-state.tsx @@ -63,12 +63,12 @@ function resolveDescription(description: ErrorProps["description"]): ReactNode { } export function SectionError(props: ErrorProps): ReactNode { - const { title, action, className } = props; + const { title, action, className, icon = DEFAULT_ERROR_ICON } = props; return ( @@ -93,14 +93,19 @@ interface PageErrorProps extends ErrorProps { } export function PageError(props: PageErrorProps): ReactNode { - const { title, action, className } = props; - const justifyUp = props.justifyUp ?? false; + const { + title, + action, + className, + icon = DEFAULT_ERROR_ICON, + justifyUp = false + } = props; const error = ( diff --git a/src/frontend/features/auth/access-level.tsx b/src/frontend/features/auth/access-level.tsx index 7bbf1d66..7914d2e6 100644 --- a/src/frontend/features/auth/access-level.tsx +++ b/src/frontend/features/auth/access-level.tsx @@ -78,15 +78,19 @@ interface RequireAccessLevelProps extends PropsWithChildren { } export function RequireAccessLevel(props: RequireAccessLevelProps) { + const { + accessLevel = AccessLevel.EDITOR, + useMaxAccessLevel = false, + children + } = props; const accessData = useAccessData(); - const requiredAccessLevel = props.accessLevel ?? AccessLevel.EDITOR; - const currentAccessLevel = props.useMaxAccessLevel + const currentAccessLevel = useMaxAccessLevel ? accessData.maxAccessLevel : accessData.currentAccessLevel; // Reads backwards: the level held is the ceiling the requirement fits under. - return isWithinAccessLevel(requiredAccessLevel, currentAccessLevel) - ? props.children + return isWithinAccessLevel(accessLevel, currentAccessLevel) + ? children : null; } diff --git a/src/frontend/features/library/components/card-components.tsx b/src/frontend/features/library/components/card-components.tsx index c63e79d4..ccb8d3a4 100644 --- a/src/frontend/features/library/components/card-components.tsx +++ b/src/frontend/features/library/components/card-components.tsx @@ -235,11 +235,14 @@ function PartNameAndNumber(props: PartNameAndNumberProps): ReactNode { } /** The part number, linked to the vendor's page for it when there is one. */ -function CardPartNumber(props: { +interface CardPartNumberProps { partNumber: string; + /** Where the query matched inside it, for underlining. */ positions?: Position[]; url?: string; -}): ReactNode { +} + +function CardPartNumber(props: CardPartNumberProps): ReactNode { const { partNumber, positions, url } = props; const text = ; if (!url) { diff --git a/src/frontend/features/library/components/group-card.tsx b/src/frontend/features/library/components/group-card.tsx index 8c4530a4..bafd5673 100644 --- a/src/frontend/features/library/components/group-card.tsx +++ b/src/frontend/features/library/components/group-card.tsx @@ -164,7 +164,12 @@ function HideAllElementsMenuItem({ ); } -function DeleteGroupMenuItem({ groupId }: { groupId: string }): ReactNode { +interface DeleteGroupMenuItemProps { + groupId: string; +} + +function DeleteGroupMenuItem(props: DeleteGroupMenuItemProps): ReactNode { + const { groupId } = props; const libraryId = useLibraryId(); const refreshLibrary = useRefreshLibrary(); diff --git a/src/frontend/features/library/components/reload-groups-button.tsx b/src/frontend/features/library/components/reload-groups-button.tsx index 9af74367..b134b47f 100644 --- a/src/frontend/features/library/components/reload-groups-button.tsx +++ b/src/frontend/features/library/components/reload-groups-button.tsx @@ -17,7 +17,7 @@ interface ReloadGroupsButtonProps { } export function ReloadGroupsButton(props: ReloadGroupsButtonProps): ReactNode { - const reloadAll = props.reloadAll ?? false; + const { reloadAll = false } = props; const libraryId = useLibraryId(); diff --git a/src/frontend/features/search/components/search-errors.tsx b/src/frontend/features/search/components/search-errors.tsx index 5becf802..08f0ee44 100644 --- a/src/frontend/features/search/components/search-errors.tsx +++ b/src/frontend/features/search/components/search-errors.tsx @@ -40,7 +40,12 @@ interface FilterCalloutProps { * Blue rather than the library accent: the strip reports on the results, so it * should read as a note beside them rather than as part of the library. */ -function Callout(props: { text: string; action: ReactNode }): ReactNode { +interface CalloutProps { + text: string; + action: ReactNode; +} + +function Callout(props: CalloutProps): ReactNode { return ( } diff --git a/src/frontend/features/settings/components/settings-menu.tsx b/src/frontend/features/settings/components/settings-menu.tsx index 3459e291..46c0bb5e 100644 --- a/src/frontend/features/settings/components/settings-menu.tsx +++ b/src/frontend/features/settings/components/settings-menu.tsx @@ -6,7 +6,7 @@ import { IconSize, StatusColor } from "../../../lib/style-constants"; -import { ReactNode } from "react"; +import { PropsWithChildren, ReactNode } from "react"; import { AccessLevel, hasEditorAccess, @@ -29,13 +29,18 @@ import { ReloadGroupsButton } from "../../library/components/reload-groups-butto /** The FRCDesign feedback form, which the setting below opens. */ const FEEDBACK_FORM_URL = "https://forms.gle/WVXUwnrrpLGKdiBx9"; -function SettingRow(props: { label: string; children: ReactNode }): ReactNode { +interface SettingRowProps extends PropsWithChildren { + label: string; +} + +function SettingRow(props: SettingRowProps): ReactNode { + const { label, children } = props; return ( - {props.label} + {label} - {props.children} + {children} ); } diff --git a/src/frontend/features/settings/components/vendor-filters.tsx b/src/frontend/features/settings/components/vendor-filters.tsx index 80dddca6..d6761b7b 100644 --- a/src/frontend/features/settings/components/vendor-filters.tsx +++ b/src/frontend/features/settings/components/vendor-filters.tsx @@ -8,21 +8,16 @@ import { useGetUiState, useSetUiState } from "../../../lib/ui-state"; import { AppContextMenu } from "../../../components/app-menu"; interface ClearFiltersButtonProps { - /** - * @default "Clear filters" - */ + /** @default "Clear filters" */ text?: string; - /** - * @default false - */ + /** @default false */ small?: boolean; } export function ClearFiltersButton(props: ClearFiltersButtonProps): ReactNode { + const { text = "Clear filters", small = false } = props; const uiState = useGetUiState(); const setUiState = useSetUiState(); - const text = props.text ?? "Clear filters"; - const small = props.small ?? false; const vendorFilters = uiState.vendorFilters; const areAllTagsActive = vendorFilters === undefined; diff --git a/src/frontend/routes/app/library/$libraryId/groups/$groupId.tsx b/src/frontend/routes/app/library/$libraryId/groups/$groupId.tsx index c6f1a008..fc1df2a7 100644 --- a/src/frontend/routes/app/library/$libraryId/groups/$groupId.tsx +++ b/src/frontend/routes/app/library/$libraryId/groups/$groupId.tsx @@ -117,7 +117,12 @@ function GroupList(): ReactNode { ); } -function GroupHeaderRow({ group }: { group: GroupOut }): ReactNode { +interface GroupHeaderRowProps { + group: GroupOut; +} + +function GroupHeaderRow(props: GroupHeaderRowProps): ReactNode { + const { group } = props; const navigate = useNavigate(); const libraryId = useLibraryId(); const menuItems = ; diff --git a/src/frontend/routes/app/library/$libraryId/index.tsx b/src/frontend/routes/app/library/$libraryId/index.tsx index 17227884..953b4a13 100644 --- a/src/frontend/routes/app/library/$libraryId/index.tsx +++ b/src/frontend/routes/app/library/$libraryId/index.tsx @@ -151,7 +151,12 @@ function HomeList(): ReactNode { } /** The library's name, and a badge when it is not simply supported. */ -function LibraryTitle({ libraryId }: { libraryId: string }): ReactNode { +interface LibraryTitleProps { + libraryId: string; +} + +function LibraryTitle(props: LibraryTitleProps): ReactNode { + const { libraryId } = props; const status = getLibraryStatus(libraryId); return ( Date: Thu, 3 Sep 2026 13:43:39 +0000 Subject: [PATCH 39/40] Split build status into the parts of the card it draws MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One file held 34 declarations and every layer of the build card at once, while every other feature here is already several files. It becomes five, each one thing the card is made of: the badges and hover card that open it, the issue severities it summarizes, the admin toggles, the parsed metadata, and the two row primitives all of them share. The seams are one-way — sections is a leaf, issues and parsed sit above it, admin above those, and the card composes them — so the imports say which layer a component belongs to. Every component here took its props as a literal type at the signature; they are interfaces now, like the rest of the frontend. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- .../build-status/components/admin-section.tsx | 210 +++++ .../build-status/components/build-status.tsx | 806 ++---------------- .../build-status/components/issues.tsx | 254 ++++++ .../components/parsed-section.tsx | 287 +++++++ .../build-status/components/sections.tsx | 41 + 5 files changed, 841 insertions(+), 757 deletions(-) create mode 100644 src/frontend/features/build-status/components/admin-section.tsx create mode 100644 src/frontend/features/build-status/components/issues.tsx create mode 100644 src/frontend/features/build-status/components/parsed-section.tsx create mode 100644 src/frontend/features/build-status/components/sections.tsx diff --git a/src/frontend/features/build-status/components/admin-section.tsx b/src/frontend/features/build-status/components/admin-section.tsx new file mode 100644 index 00000000..5adca02c --- /dev/null +++ b/src/frontend/features/build-status/components/admin-section.tsx @@ -0,0 +1,210 @@ +import { Stack, Switch, Tooltip } from "@mantine/core"; +import { ReactNode } from "react"; +import { BuildIssueSeverity } from "@backend/features/build-checker/issues"; +import { + GroupBuildStatus, + InsertableBuildStatus +} from "@backend/features/build-checker/contract"; +import { ElementType } from "@backend/lib/onshape/element-type"; +import { + AUTO_INDEX_THRESHOLD, + type ConfigurationCount, + IndexingBand, + MAX_PART_NUMBER_CONFIGURATIONS +} from "@backend/features/configurations/combinations"; +import { NO_SHRINK } from "../../../lib/style-constants"; +import { + useSetVisibilityMutation, + useToggleInsertAndFastenMutation, + useIndexConfigurationsMutation, + useToggleSortOrderMutation +} from "../../library/card-hooks"; +import { ControlRow, SectionHeader } from "./sections"; +import { IssueIcon } from "./issues"; + +interface SwitchRowProps { + label: string; + description?: string; + checked: boolean; + onToggle: () => void; +} + +/** A label (+ description) and on/off Switch row for an editable admin flag. */ +function SwitchRow(props: SwitchRowProps): ReactNode { + return ( + + } + /> + ); +} + +interface InsertableAdminSectionProps { + insertableId: string; + status: InsertableBuildStatus; + configurationCount: ConfigurationCount; +} + +/** The editable admin toggles for an insertable. */ +export function InsertableAdminSection( + props: InsertableAdminSectionProps +): ReactNode { + const { insertableId, status, configurationCount } = props; + return ( + + Admin + + + + + ); +} + +interface VisibilitySwitchProps { + insertableId: string; + isVisible: boolean; +} + +function VisibilitySwitch(props: VisibilitySwitchProps): ReactNode { + const { insertableId, isVisible } = props; + const mutation = useSetVisibilityMutation([insertableId], !isVisible); + return ( + mutation.mutate()} + /> + ); +} + +interface FastenSwitchProps { + insertableId: string; + supportsFasten: boolean; +} + +function FastenSwitch(props: FastenSwitchProps): ReactNode { + const { insertableId, supportsFasten } = props; + const mutation = useToggleInsertAndFastenMutation(insertableId); + return ( + mutation.mutate(!supportsFasten)} + /> + ); +} + +interface IndexingRowProps { + insertableId: string; + status: InsertableBuildStatus; + band: IndexingBand; +} + +/** + * A switch only where enabling indexing is the admin's call, an icon saying why + * not otherwise — past the cap it can't run, under the threshold it already has. + */ +function IndexingRow(props: IndexingRowProps): ReactNode { + const { insertableId, status, band } = props; + const mutation = useIndexConfigurationsMutation(insertableId); + + let control: ReactNode; + if (status.elementType === ElementType.ASSEMBLY) { + control = ( + + ); + } else if (band === IndexingBand.EXCEEDED) { + control = ( + + ); + } else if (band === IndexingBand.AUTOMATIC) { + control = ( + + ); + } else { + control = ( + mutation.mutate(!status.indexConfigurations)} + withThumbIndicator={false} + /> + ); + } + + return ( + + ); +} + +interface IndexingIconProps { + severity: BuildIssueSeverity | null; + tooltip: string; +} + +/** + * Stands in for the switch where there is nothing to toggle, reusing the + * build-check icons so the state reads the same as the callouts above it. + */ +function IndexingIcon(props: IndexingIconProps): ReactNode { + const { severity, tooltip } = props; + return ( + + + + ); +} + +interface GroupAdminSectionProps { + groupId: string; + status: GroupBuildStatus; +} + +/** The editable admin toggles for a group. */ +export function GroupAdminSection(props: GroupAdminSectionProps): ReactNode { + const { groupId, status } = props; + const mutation = useToggleSortOrderMutation(groupId); + return ( + + Admin + mutation.mutate(!status.sortAlphabetically)} + /> + + ); +} diff --git a/src/frontend/features/build-status/components/build-status.tsx b/src/frontend/features/build-status/components/build-status.tsx index 084354b9..56871b74 100644 --- a/src/frontend/features/build-status/components/build-status.tsx +++ b/src/frontend/features/build-status/components/build-status.tsx @@ -1,163 +1,42 @@ import { - Badge, - Box, Divider, Group, HoverCard, Loader, - ScrollArea, Stack, - Switch, Text, Tooltip } from "@mantine/core"; -import { - CheckIcon, - ClockIcon, - FileXIcon, - InfoIcon, - WarningIcon, - WarningOctagonIcon, - XIcon -} from "@phosphor-icons/react"; -import { - ReactNode, - createContext, - use, - useCallback, - useMemo, - useState -} from "react"; +import { ClockIcon } from "@phosphor-icons/react"; +import { ReactNode, createContext, use, useCallback, useState } from "react"; import { formatRelativeTime } from "../../../lib/format-time"; import { - addBuildIssue, BuildIssue, - BuildIssueSeverity, - BuildIssueType, - getIssueDescription, - getIssueSeverity, - getMaxSeverity, - hasBuildIssue + getMaxSeverity } from "@backend/features/build-checker/issues"; -import { - GroupBuildStatus, - InsertableBuildStatus -} from "@backend/features/build-checker/contract"; -import { getVendorName, Vendor } from "@backend/features/library/vendors"; -import { ElementType } from "@backend/lib/onshape/element-type"; -import { - ConfigurationParameter, - ParameterType -} from "@backend/features/configurations/models"; -import { - AUTO_INDEX_THRESHOLD, - type ConfigurationCount, - countCombinations, - countConfigurations, - IndexingBand, - MAX_COUNTED_CONFIGURATIONS, - MAX_PART_NUMBER_CONFIGURATIONS -} from "@backend/features/configurations/combinations"; +import { InsertableBuildStatus } from "@backend/features/build-checker/contract"; import { FontWeight, IconSize, NO_SHRINK, - RADIUS, - StatusColor, - statusBackground + StatusColor } from "../../../lib/style-constants"; import { RequireAccessLevel } from "../../auth/access-level"; import { useBuildStatusQuery } from "../queries"; import { useIsJobRunning } from "../../library/queries"; -import { AppIcon, type AppIconProps } from "../../../components/app-icon"; import { - useSetVisibilityMutation, - useToggleInsertAndFastenMutation, - useIndexConfigurationsMutation, - useToggleSortOrderMutation -} from "../../library/card-hooks"; - -/** Discriminated so `StateValue` renders each kind its own way. */ -export type StateRowValue = - | { kind: "bool"; value: boolean } - | { kind: "text"; text: string; dimmed?: boolean } - | { kind: "vendors"; vendors: Vendor[] }; - -/** - * Returns the build issues for an insertable, merging insertable-level and - * configuration-level issues. - */ -function getInsertableBuildIssues( - insertable: InsertableBuildStatus -): BuildIssue[] { - const configIssues = insertable.configuration?.buildIssues ?? []; - return [...insertable.buildIssues, ...configIssues]; -} - -/** - * Stored issues plus the live "no unhidden insertables" check, which needs the - * per-insertable visibility in the same response. - */ -function useGroupBuildIssues( - groupStatus: GroupBuildStatus | undefined, - insertableStatuses: Record | undefined -): BuildIssue[] { - return useMemo(() => { - if (!groupStatus) return []; - const hasUnhidden = groupStatus.insertableOrder.some( - (id) => insertableStatuses?.[id]?.isVisible - ); - // A group that never loaded has no insertables to unhide, so the failure - // is the whole story. - if ( - hasUnhidden || - hasBuildIssue(groupStatus.buildIssues, BuildIssueType.LOAD_FAILED) - ) { - return groupStatus.buildIssues; - } - return addBuildIssue(groupStatus.buildIssues, { - type: BuildIssueType.NO_UNHIDDEN_INSERTABLES - }); - }, [groupStatus, insertableStatuses]); -} - -interface IssueIconProps extends Omit { - /** The severity to render, or null if all checks pass. */ - severity: BuildIssueSeverity | null; -} - -/** The icon each severity is drawn as; `ok` is a build with nothing to say. */ -const SEVERITY_ICONS = { - [BuildIssueSeverity.ERROR]: WarningOctagonIcon, - [BuildIssueSeverity.WARNING]: WarningIcon, - [BuildIssueSeverity.INFO]: InfoIcon, - ok: CheckIcon -}; - -/** The color a severity is spoken in; null is a build with nothing to say. */ -function severityColor(severity: BuildIssueSeverity | null): StatusColor { - switch (severity) { - case BuildIssueSeverity.ERROR: - return StatusColor.ERROR; - case BuildIssueSeverity.WARNING: - return StatusColor.WARNING; - case BuildIssueSeverity.INFO: - return StatusColor.INFO; - case null: - return StatusColor.SUCCESS; - } -} - -/** Renders the icon for a build-issue severity in its severity color. */ -export function IssueIcon({ severity, ...others }: IssueIconProps): ReactNode { - return ( - - ); -} + BuildChecksSection, + IssueIcon, + SeverityBadges, + getInsertableBuildIssues, + useGroupBuildIssues +} from "./issues"; +import { + ConfigurationSection, + InsertableParsedSection, + useConfigurationCount +} from "./parsed-section"; +import { GroupAdminSection, InsertableAdminSection } from "./admin-section"; interface BuildStatusCardProps { /** The group/insertable name shown in the header. */ @@ -219,7 +98,7 @@ export function useCloseBuildCard(): () => void { * A severity icon whose hover card shows the build-status card wrapping the * given admin menu. Only rendered for editors and admins. */ -export function BuildStatusBadge(props: BuildStatusBadgeProps): ReactNode { +function BuildStatusBadge(props: BuildStatusBadgeProps): ReactNode { // Gate first so the card and its admin controls only exist for editors. return ( @@ -272,16 +151,15 @@ function BuildStatusHoverCard({ ); } -/** The card header: name + severity summary on the left, last-loaded on the right. */ -function CardHeader({ - name, - issues, - lastLoadedAt -}: { +interface CardHeaderProps { name: string; issues: BuildIssue[]; lastLoadedAt: number | null; -}): ReactNode { +} + +/** The card header: name + severity summary on the left, last-loaded on the right. */ +function CardHeader(props: CardHeaderProps): ReactNode { + const { name, issues, lastLoadedAt } = props; return ( } - > - All checks pass - - ); - } - - const counts = countSeverities(issues); - return ( - - {counts.error > 0 && ( - - )} - {counts.warning > 0 && ( - - )} - {counts.info > 0 && ( - - )} - - ); -} - -/** The badge color and singular noun for each severity. */ -const SEVERITY_BADGE: Record< - BuildIssueSeverity, - { color: string; noun: string } -> = { - [BuildIssueSeverity.ERROR]: { color: "red", noun: "error" }, - [BuildIssueSeverity.WARNING]: { color: "yellow", noun: "warning" }, - [BuildIssueSeverity.INFO]: { color: "blue", noun: "info" } -}; - -function CountBadge({ - severity, - count -}: { - severity: BuildIssueSeverity; - count: number; -}): ReactNode { - const { color, noun } = SEVERITY_BADGE[severity]; - // Don't pluralize info, e.g. "2 infos" reads wrong. - const plural = severity !== BuildIssueSeverity.INFO && count > 1 ? "s" : ""; - return ( - - {`${count} ${noun}${plural}`} - - ); -} - -/** How many issues of each severity a build carries. */ -interface SeverityCounts { - error: number; - warning: number; - info: number; -} - -function countSeverities(issues: BuildIssue[]): SeverityCounts { - const counts = { error: 0, warning: 0, info: 0 }; - for (const issue of issues) { - switch (getIssueSeverity(issue)) { - case BuildIssueSeverity.ERROR: - counts.error += 1; - break; - case BuildIssueSeverity.WARNING: - counts.warning += 1; - break; - case BuildIssueSeverity.INFO: - counts.info += 1; - break; - } - } - return counts; -} - -/** The build checks: one tinted callout per issue. Rendered only when non-empty. */ -function BuildChecksSection({ issues }: { issues: BuildIssue[] }): ReactNode { - return ( - - Build checks - {issues.map((issue) => ( - - ))} - - ); -} - -/** A single build issue rendered as a tinted callout box in its severity color. */ -function IssueCallout({ issue }: { issue: BuildIssue }): ReactNode { - const severity = getIssueSeverity(issue); - return ( - - {/* Nudge the icon down so it aligns with the first line of text. */} - - {getIssueDescription(issue)} - - ); -} - -/** The light background tint for a build-issue callout. */ -function severityBackground(severity: BuildIssueSeverity): string { - return statusBackground(severityColor(severity)); +interface InsertableStatusBadgeProps { + insertableId: string; + name: string; } /** Build-status badge pre-wired for an insertable. */ -export function InsertableStatusBadge({ - insertableId, - name -}: { - insertableId: string; - name: string; -}): ReactNode { +export function InsertableStatusBadge( + props: InsertableStatusBadgeProps +): ReactNode { + const { insertableId, name } = props; const { data } = useBuildStatusQuery(); const insertable = data?.insertables[insertableId]; if (!insertable) return null; @@ -503,14 +251,14 @@ export function InsertableStatusBadge({ ); } -/** Enumerates configurations once, for every row of the card that needs it. */ -function InsertableHoverMenu({ - insertableId, - status -}: { +interface InsertableHoverMenuProps { insertableId: string; status: InsertableBuildStatus; -}): ReactNode { +} + +/** Enumerates configurations once, for every row of the card that needs it. */ +function InsertableHoverMenu(props: InsertableHoverMenuProps): ReactNode { + const { insertableId, status } = props; const configurationCount = useConfigurationCount(status); return ( <> @@ -527,14 +275,14 @@ function InsertableHoverMenu({ ); } -/** Build-status badge pre-wired for a group (includes live visibility check). */ -export function GroupStatusBadge({ - groupId, - groupName -}: { +interface GroupStatusBadgeProps { groupId: string; groupName: string; -}): ReactNode { +} + +/** Build-status badge pre-wired for a group (includes live visibility check). */ +export function GroupStatusBadge(props: GroupStatusBadgeProps): ReactNode { + const { groupId, groupName } = props; const { data } = useBuildStatusQuery(); const groupStatus = data?.groups[groupId]; const issues = useGroupBuildIssues(groupStatus, data?.insertables); @@ -550,459 +298,3 @@ export function GroupStatusBadge({ /> ); } - -/** A dimmed section header, e.g. "Admin" or "Parsed". */ -function SectionHeader({ children }: { children: ReactNode }): ReactNode { - return ( - - {children} - - ); -} - -/** - * A label (+ description) and a right-aligned control. Usually a Switch, but a - * setting that isn't the admin's to make shows an icon saying why instead. - */ -function ControlRow(props: { - label: string; - description?: string; - control: ReactNode; -}): ReactNode { - return ( - - - {props.label} - - {props.description} - - - {props.control} - - ); -} - -/** A label (+ description) and on/off Switch row for an editable admin flag. */ -function SwitchRow(props: { - label: string; - description?: string; - checked: boolean; - onToggle: () => void; -}): ReactNode { - return ( - - } - /> - ); -} - -/** The editable admin toggles for an insertable. */ -function InsertableAdminSection({ - insertableId, - status, - configurationCount -}: { - insertableId: string; - status: InsertableBuildStatus; - configurationCount: ConfigurationCount; -}): ReactNode { - return ( - - Admin - - - - - ); -} - -function VisibilitySwitch({ - insertableId, - isVisible -}: { - insertableId: string; - isVisible: boolean; -}): ReactNode { - const mutation = useSetVisibilityMutation([insertableId], !isVisible); - return ( - mutation.mutate()} - /> - ); -} - -function FastenSwitch({ - insertableId, - supportsFasten -}: { - insertableId: string; - supportsFasten: boolean; -}): ReactNode { - const mutation = useToggleInsertAndFastenMutation(insertableId); - return ( - mutation.mutate(!supportsFasten)} - /> - ); -} - -/** - * A switch only where enabling indexing is the admin's call, an icon saying why - * not otherwise — past the cap it can't run, under the threshold it already has. - */ -function IndexingRow({ - insertableId, - status, - band -}: { - insertableId: string; - status: InsertableBuildStatus; - band: IndexingBand; -}): ReactNode { - const mutation = useIndexConfigurationsMutation(insertableId); - - let control: ReactNode; - if (status.elementType === ElementType.ASSEMBLY) { - control = ( - - ); - } else if (band === IndexingBand.EXCEEDED) { - control = ( - - ); - } else if (band === IndexingBand.AUTOMATIC) { - control = ( - - ); - } else { - control = ( - mutation.mutate(!status.indexConfigurations)} - withThumbIndicator={false} - /> - ); - } - - return ( - - ); -} - -/** - * Stands in for the switch where there is nothing to toggle, reusing the - * build-check icons so the state reads the same as the callouts above it. - */ -function IndexingIcon({ - severity, - tooltip -}: { - severity: BuildIssueSeverity | null; - tooltip: string; -}): ReactNode { - return ( - - - - ); -} - -/** The editable admin toggles for a group. */ -function GroupAdminSection({ - groupId, - status -}: { - groupId: string; - status: GroupBuildStatus; -}): ReactNode { - const mutation = useToggleSortOrderMutation(groupId); - return ( - - Admin - mutation.mutate(!status.sortAlphabetically)} - /> - - ); -} - -/** - * Enumerated rather than stored: the same shared routine the load path uses, - * and it only runs when a hover card opens. - */ -function useConfigurationCount( - status: InsertableBuildStatus -): ConfigurationCount { - const parameters = status.configuration?.parameters; - return useMemo(() => countConfigurations(parameters ?? []), [parameters]); -} - -/** The true total, which runs past the index cap the band is decided by. */ -function useDisplayedConfigurationCount( - status: InsertableBuildStatus -): number | null { - const parameters = status.configuration?.parameters; - return useMemo(() => countCombinations(parameters ?? []), [parameters]); -} - -/** Open-ended only past the counting cap, which nothing real reaches. */ -function configurationCountValue(count: number | null): StateRowValue { - if (count === null) { - return { - kind: "text", - text: `Over ${MAX_COUNTED_CONFIGURATIONS.toLocaleString()}` - }; - } - if (count === 0) { - return { kind: "text", text: "None", dimmed: true }; - } - return { kind: "text", text: count.toLocaleString() }; -} - -/** The read-only auto-detected facts for an insertable. */ -function InsertableParsedSection({ - status -}: { - status: InsertableBuildStatus; -}): ReactNode { - const count = useDisplayedConfigurationCount(status); - return ( - <> - - - Parsed - - - - - ); -} - -/** Each parameter's name, the type it takes, and whether indexing varies it. */ -function ConfigurationSection({ - parameters -}: { - parameters?: ConfigurationParameter[]; -}): ReactNode { - if (!parameters || parameters.length === 0) return null; - return ( - <> - - - Configurations - - - {parameters.map((parameter) => ( - - {parameter.name} - - - - - - ))} - - - - - ); -} - -/** - * Onshape's "exclude from affecting configured properties", the lever on the - * count. Part studios only, which Onshape itself enforces. - */ -function ExcludedFromPropertiesIcon({ - parameter -}: { - parameter: ConfigurationParameter; -}): ReactNode { - if (!parameter.isCosmetic) { - return null; - } - return ( - - - - ); -} - -/** - * The parameter's type. An enum also carries its option count, and lists the - * options on hover — the values that drive its share of the configuration count. - */ -function ParameterTypeBadge({ - parameter -}: { - parameter: ConfigurationParameter; -}): ReactNode { - const isEnum = parameter.type === ParameterType.ENUM; - const label = isEnum - ? `${getParameterTypeLabel(parameter.type)} (${parameter.options.length})` - : getParameterTypeLabel(parameter.type); - - const badge = ( - - {label} - - ); - if (!isEnum || parameter.options.length === 0) { - return badge; - } - return ( - option.name).join(", ")} - multiline - maw={260} - withArrow - events={{ hover: true, focus: true, touch: true }} - > - {badge} - - ); -} - -/** The short label for a parameter's type, shown as a badge. */ -function getParameterTypeLabel(type: ParameterType): string { - switch (type) { - case ParameterType.ENUM: - return "Enum"; - case ParameterType.BOOLEAN: - return "Boolean"; - case ParameterType.QUANTITY: - return "Quantity"; - case ParameterType.STRING: - return "Text"; - } -} - -/** A read-only label/value row in the "Parsed" section. */ -function ParsedRow({ - label, - value -}: { - label: string; - value: StateRowValue; -}): ReactNode { - return ( - - {label} - - - ); -} - -/** Renders a parsed value: a check/cross for booleans, badges for vendors. */ -function StateValue({ value }: { value: StateRowValue }): ReactNode { - if (value.kind === "bool") { - return value.value ? ( - - ) : ( - - ); - } - - if (value.kind === "text") { - return ( - - {value.text} - - ); - } - - if (value.vendors.length === 0) { - return ( - - None - - ); - } - return ( - - {value.vendors.map((vendor) => ( - - {vendor} - - ))} - - ); -} diff --git a/src/frontend/features/build-status/components/issues.tsx b/src/frontend/features/build-status/components/issues.tsx new file mode 100644 index 00000000..a195f19a --- /dev/null +++ b/src/frontend/features/build-status/components/issues.tsx @@ -0,0 +1,254 @@ +import { Badge, Group, Stack, Text } from "@mantine/core"; +import { + CheckIcon, + InfoIcon, + WarningIcon, + WarningOctagonIcon +} from "@phosphor-icons/react"; +import { ReactNode, useMemo } from "react"; +import { + addBuildIssue, + BuildIssue, + BuildIssueSeverity, + BuildIssueType, + getIssueDescription, + getIssueSeverity, + hasBuildIssue +} from "@backend/features/build-checker/issues"; +import { + GroupBuildStatus, + InsertableBuildStatus +} from "@backend/features/build-checker/contract"; +import { + IconSize, + NO_SHRINK, + RADIUS, + StatusColor, + statusBackground +} from "../../../lib/style-constants"; +import { AppIcon, type AppIconProps } from "../../../components/app-icon"; +import { SectionHeader } from "./sections"; + +/** + * Returns the build issues for an insertable, merging insertable-level and + * configuration-level issues. + */ +export function getInsertableBuildIssues( + insertable: InsertableBuildStatus +): BuildIssue[] { + const configIssues = insertable.configuration?.buildIssues ?? []; + return [...insertable.buildIssues, ...configIssues]; +} + +/** + * Stored issues plus the live "no unhidden insertables" check, which needs the + * per-insertable visibility in the same response. + */ +export function useGroupBuildIssues( + groupStatus: GroupBuildStatus | undefined, + insertableStatuses: Record | undefined +): BuildIssue[] { + return useMemo(() => { + if (!groupStatus) return []; + const hasUnhidden = groupStatus.insertableOrder.some( + (id) => insertableStatuses?.[id]?.isVisible + ); + // A group that never loaded has no insertables to unhide, so the failure + // is the whole story. + if ( + hasUnhidden || + hasBuildIssue(groupStatus.buildIssues, BuildIssueType.LOAD_FAILED) + ) { + return groupStatus.buildIssues; + } + return addBuildIssue(groupStatus.buildIssues, { + type: BuildIssueType.NO_UNHIDDEN_INSERTABLES + }); + }, [groupStatus, insertableStatuses]); +} + +interface IssueIconProps extends Omit { + /** The severity to render, or null if all checks pass. */ + severity: BuildIssueSeverity | null; +} + +/** The icon each severity is drawn as; `ok` is a build with nothing to say. */ +const SEVERITY_ICONS = { + [BuildIssueSeverity.ERROR]: WarningOctagonIcon, + [BuildIssueSeverity.WARNING]: WarningIcon, + [BuildIssueSeverity.INFO]: InfoIcon, + ok: CheckIcon +}; + +/** The color a severity is spoken in; null is a build with nothing to say. */ +function severityColor(severity: BuildIssueSeverity | null): StatusColor { + switch (severity) { + case BuildIssueSeverity.ERROR: + return StatusColor.ERROR; + case BuildIssueSeverity.WARNING: + return StatusColor.WARNING; + case BuildIssueSeverity.INFO: + return StatusColor.INFO; + case null: + return StatusColor.SUCCESS; + } +} + +/** Renders the icon for a build-issue severity in its severity color. */ +export function IssueIcon({ severity, ...others }: IssueIconProps): ReactNode { + return ( + + ); +} + +interface SeverityBadgesProps { + issues: BuildIssue[]; +} + +/** Pill badges summarizing the issue counts, or an "all clear" badge. */ +export function SeverityBadges(props: SeverityBadgesProps): ReactNode { + const { issues } = props; + if (issues.length === 0) { + return ( + } + > + All checks pass + + ); + } + + const counts = countSeverities(issues); + return ( + + {counts.error > 0 && ( + + )} + {counts.warning > 0 && ( + + )} + {counts.info > 0 && ( + + )} + + ); +} + +/** The badge color and singular noun for each severity. */ +const SEVERITY_BADGE: Record< + BuildIssueSeverity, + { color: string; noun: string } +> = { + [BuildIssueSeverity.ERROR]: { color: "red", noun: "error" }, + [BuildIssueSeverity.WARNING]: { color: "yellow", noun: "warning" }, + [BuildIssueSeverity.INFO]: { color: "blue", noun: "info" } +}; + +interface CountBadgeProps { + severity: BuildIssueSeverity; + count: number; +} + +function CountBadge(props: CountBadgeProps): ReactNode { + const { severity, count } = props; + const { color, noun } = SEVERITY_BADGE[severity]; + // Don't pluralize info, e.g. "2 infos" reads wrong. + const plural = severity !== BuildIssueSeverity.INFO && count > 1 ? "s" : ""; + return ( + + {`${count} ${noun}${plural}`} + + ); +} + +/** How many issues of each severity a build carries. */ +interface SeverityCounts { + error: number; + warning: number; + info: number; +} + +function countSeverities(issues: BuildIssue[]): SeverityCounts { + const counts = { error: 0, warning: 0, info: 0 }; + for (const issue of issues) { + switch (getIssueSeverity(issue)) { + case BuildIssueSeverity.ERROR: + counts.error += 1; + break; + case BuildIssueSeverity.WARNING: + counts.warning += 1; + break; + case BuildIssueSeverity.INFO: + counts.info += 1; + break; + } + } + return counts; +} + +interface BuildChecksSectionProps { + issues: BuildIssue[]; +} + +/** The build checks: one tinted callout per issue. Rendered only when non-empty. */ +export function BuildChecksSection(props: BuildChecksSectionProps): ReactNode { + const { issues } = props; + return ( + + Build checks + {issues.map((issue) => ( + + ))} + + ); +} + +interface IssueCalloutProps { + issue: BuildIssue; +} + +/** A single build issue rendered as a tinted callout box in its severity color. */ +function IssueCallout(props: IssueCalloutProps): ReactNode { + const { issue } = props; + const severity = getIssueSeverity(issue); + return ( + + {/* Nudge the icon down so it aligns with the first line of text. */} + + {getIssueDescription(issue)} + + ); +} + +/** The light background tint for a build-issue callout. */ +function severityBackground(severity: BuildIssueSeverity): string { + return statusBackground(severityColor(severity)); +} diff --git a/src/frontend/features/build-status/components/parsed-section.tsx b/src/frontend/features/build-status/components/parsed-section.tsx new file mode 100644 index 00000000..79872360 --- /dev/null +++ b/src/frontend/features/build-status/components/parsed-section.tsx @@ -0,0 +1,287 @@ +import { + Badge, + Divider, + Group, + ScrollArea, + Stack, + Text, + Tooltip +} from "@mantine/core"; +import { CheckIcon, FileXIcon, XIcon } from "@phosphor-icons/react"; +import { ReactNode, useMemo } from "react"; +import { InsertableBuildStatus } from "@backend/features/build-checker/contract"; +import { getVendorName, Vendor } from "@backend/features/library/vendors"; +import { + ConfigurationParameter, + ParameterType +} from "@backend/features/configurations/models"; +import { + type ConfigurationCount, + countCombinations, + countConfigurations, + MAX_COUNTED_CONFIGURATIONS +} from "@backend/features/configurations/combinations"; +import { IconSize, NO_SHRINK, StatusColor } from "../../../lib/style-constants"; +import { AppIcon } from "../../../components/app-icon"; +import { SectionHeader } from "./sections"; + +/** Discriminated so `StateValue` renders each kind its own way. */ +type StateRowValue = + | { kind: "bool"; value: boolean } + | { kind: "text"; text: string; dimmed?: boolean } + | { kind: "vendors"; vendors: Vendor[] }; + +/** + * Enumerated rather than stored: the same shared routine the load path uses, + * and it only runs when a hover card opens. + */ +export function useConfigurationCount( + status: InsertableBuildStatus +): ConfigurationCount { + const parameters = status.configuration?.parameters; + return useMemo(() => countConfigurations(parameters ?? []), [parameters]); +} + +/** The true total, which runs past the index cap the band is decided by. */ +function useDisplayedConfigurationCount( + status: InsertableBuildStatus +): number | null { + const parameters = status.configuration?.parameters; + return useMemo(() => countCombinations(parameters ?? []), [parameters]); +} + +/** Open-ended only past the counting cap, which nothing real reaches. */ +function configurationCountValue(count: number | null): StateRowValue { + if (count === null) { + return { + kind: "text", + text: `Over ${MAX_COUNTED_CONFIGURATIONS.toLocaleString()}` + }; + } + if (count === 0) { + return { kind: "text", text: "None", dimmed: true }; + } + return { kind: "text", text: count.toLocaleString() }; +} + +interface InsertableParsedSectionProps { + status: InsertableBuildStatus; +} + +/** The read-only auto-detected facts for an insertable. */ +export function InsertableParsedSection( + props: InsertableParsedSectionProps +): ReactNode { + const { status } = props; + const count = useDisplayedConfigurationCount(status); + return ( + <> + + + Parsed + + + + + ); +} + +interface ConfigurationSectionProps { + parameters?: ConfigurationParameter[]; +} + +/** Each parameter's name, the type it takes, and whether indexing varies it. */ +export function ConfigurationSection( + props: ConfigurationSectionProps +): ReactNode { + const { parameters } = props; + if (!parameters || parameters.length === 0) return null; + return ( + <> + + + Configurations + + + {parameters.map((parameter) => ( + + {parameter.name} + + + + + + ))} + + + + + ); +} + +interface ExcludedFromPropertiesIconProps { + parameter: ConfigurationParameter; +} + +/** + * Onshape's "exclude from affecting configured properties", the lever on the + * count. Part studios only, which Onshape itself enforces. + */ +function ExcludedFromPropertiesIcon( + props: ExcludedFromPropertiesIconProps +): ReactNode { + const { parameter } = props; + if (!parameter.isCosmetic) { + return null; + } + return ( + + + + ); +} + +interface ParameterTypeBadgeProps { + parameter: ConfigurationParameter; +} + +/** + * The parameter's type. An enum also carries its option count, and lists the + * options on hover — the values that drive its share of the configuration count. + */ +function ParameterTypeBadge(props: ParameterTypeBadgeProps): ReactNode { + const { parameter } = props; + const isEnum = parameter.type === ParameterType.ENUM; + const label = isEnum + ? `${getParameterTypeLabel(parameter.type)} (${parameter.options.length})` + : getParameterTypeLabel(parameter.type); + + const badge = ( + + {label} + + ); + if (!isEnum || parameter.options.length === 0) { + return badge; + } + return ( + option.name).join(", ")} + multiline + maw={260} + withArrow + events={{ hover: true, focus: true, touch: true }} + > + {badge} + + ); +} + +/** The short label for a parameter's type, shown as a badge. */ +function getParameterTypeLabel(type: ParameterType): string { + switch (type) { + case ParameterType.ENUM: + return "Enum"; + case ParameterType.BOOLEAN: + return "Boolean"; + case ParameterType.QUANTITY: + return "Quantity"; + case ParameterType.STRING: + return "Text"; + } +} + +interface ParsedRowProps { + label: string; + value: StateRowValue; +} + +/** A read-only label/value row in the "Parsed" section. */ +function ParsedRow(props: ParsedRowProps): ReactNode { + const { label, value } = props; + return ( + + {label} + + + ); +} + +interface StateValueProps { + value: StateRowValue; +} + +/** Renders a parsed value: a check/cross for booleans, badges for vendors. */ +function StateValue(props: StateValueProps): ReactNode { + const { value } = props; + if (value.kind === "bool") { + return value.value ? ( + + ) : ( + + ); + } + + if (value.kind === "text") { + return ( + + {value.text} + + ); + } + + if (value.vendors.length === 0) { + return ( + + None + + ); + } + return ( + + {value.vendors.map((vendor) => ( + + {vendor} + + ))} + + ); +} diff --git a/src/frontend/features/build-status/components/sections.tsx b/src/frontend/features/build-status/components/sections.tsx new file mode 100644 index 00000000..0f6eb8df --- /dev/null +++ b/src/frontend/features/build-status/components/sections.tsx @@ -0,0 +1,41 @@ +import { Box, Group, Text } from "@mantine/core"; +import { ReactNode } from "react"; +import { FontWeight, StatusColor } from "../../../lib/style-constants"; + +interface SectionHeaderProps { + children: ReactNode; +} + +/** A dimmed section header, e.g. "Admin" or "Parsed". */ +export function SectionHeader(props: SectionHeaderProps): ReactNode { + const { children } = props; + return ( + + {children} + + ); +} + +interface ControlRowProps { + label: string; + description?: string; + control: ReactNode; +} + +/** + * A label (+ description) and a right-aligned control. Usually a Switch, but a + * setting that isn't the admin's to make shows an icon saying why instead. + */ +export function ControlRow(props: ControlRowProps): ReactNode { + return ( + + + {props.label} + + {props.description} + + + {props.control} + + ); +} From 9a4502f11874cf980a41b0a71d8da03800d54d54 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 15:49:50 +0000 Subject: [PATCH 40/40] Cut the frontend's longest components down to what they render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The big components were doing three jobs at once: holding state, wiring mutations, and drawing. What was state or wiring is now a named hook — useInsertSelection, useSetDefaultConfigurationMutation, useHomeSections, useDefaultConfiguration, useReportSelection — and what was a self-contained block of markup is a component: InsertMenuFooter, SectionAccordion. useMenuTitle replaces the same updateModal effect written out in two menus. It takes an undefined name to mean not yet known, which is what the effects it replaces guarded for: the opener has already set a real title, so writing an empty one over it would blank the header until the query lands. The last components taking an object type literal have props interfaces too. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL --- src/frontend/components/app-title.tsx | 35 +++- .../favorites/components/favorite-menu.tsx | 96 +++++----- .../insert/components/configurations.tsx | 84 +++++---- .../insert/components/insert-menu.tsx | 165 +++++++++++------- .../library/components/group-card.tsx | 18 +- .../library/components/insertable-card.tsx | 58 +++--- .../search/components/search-results.tsx | 13 +- .../thumbnails/components/thumbnail.tsx | 120 ++++++++----- .../routes/app/library/$libraryId/index.tsx | 165 ++++++++++-------- 9 files changed, 439 insertions(+), 315 deletions(-) diff --git a/src/frontend/components/app-title.tsx b/src/frontend/components/app-title.tsx index 2f68d0e7..5f653604 100644 --- a/src/frontend/components/app-title.tsx +++ b/src/frontend/components/app-title.tsx @@ -9,7 +9,8 @@ import { Tooltip } from "@mantine/core"; import { ArrowSquareOutIcon, CheckIcon, CopyIcon } from "@phosphor-icons/react"; -import type { ReactNode } from "react"; +import { type ReactNode, useEffect } from "react"; +import { modals } from "@mantine/modals"; import type { SearchRecord } from "@backend/features/configurations/models"; import { FontWeight, @@ -90,17 +91,39 @@ export function MenuTitle(props: MenuTitleProps): ReactNode { ); } +interface UseMenuTitleProps extends Omit { + /** Undefined until known, which leaves the title the opener set. */ + name: string | undefined; +} + +/** + * Keeps a modal's header on the selection in view. The header is updated rather + * than rendered, being the modal's rather than the content's. + */ +export function useMenuTitle(modalId: string, props: UseMenuTitleProps): void { + const { name, record, icon } = props; + useEffect(() => { + if (name === undefined) { + return; + } + modals.updateModal({ + modalId, + title: + }); + }, [modalId, name, record, icon]); +} + /** The xs line box the subtitle row is otherwise sized by, floored. */ const COPY_BUTTON_SIZE = 16; /** The part number, linked to the vendor's page for it when there is one. */ -function PartNumber({ - partNumber, - url -}: { +interface PartNumberProps { partNumber: string; url?: string; -}): ReactNode { +} + +function PartNumber(props: PartNumberProps): ReactNode { + const { partNumber, url } = props; // Nowhere to send them, so offer the number itself to search with. if (!url) { return ( diff --git a/src/frontend/features/favorites/components/favorite-menu.tsx b/src/frontend/features/favorites/components/favorite-menu.tsx index 0f27eafe..aba0d013 100644 --- a/src/frontend/features/favorites/components/favorite-menu.tsx +++ b/src/frontend/features/favorites/components/favorite-menu.tsx @@ -1,10 +1,10 @@ import { modals } from "@mantine/modals"; import { AppModalBody, AppModalFooter } from "../../../components/app-modal"; -import { MenuTitle } from "../../../components/app-title"; +import { useMenuTitle } from "../../../components/app-title"; import { Button } from "@mantine/core"; import { FloppyDiskIcon } from "@phosphor-icons/react"; import { IconSize } from "../../../lib/style-constants"; -import { ReactNode, useEffect, useState } from "react"; +import { ReactNode, useState } from "react"; import { useMutation } from "@tanstack/react-query"; import { apiPost } from "../../../lib/api-client"; import { showErrorToast, showSuccessToast } from "../../../lib/notifications"; @@ -33,58 +33,27 @@ interface FavoriteMenuContentProps { defaultConfiguration?: ParameterValues; } -export function FavoriteMenuContent( - props: FavoriteMenuContentProps -): ReactNode { - const { favoriteId, modalId, defaultConfiguration } = props; - +/** + * Saves what the favorite opens with. Takes the canonical form too, so the + * cached row names the right thumbnail before the refetch answers. + */ +function useSetDefaultConfigurationMutation( + favoriteId: string, + defaultConfiguration: ParameterValues | undefined, + canonicalConfiguration: string | undefined +) { const libraryId = useLibraryId(); - const insertables = useLibraryQuery().data?.insertables; - const favoritesData = useFavoritesQuery().data; const refreshFavorites = useRefreshFavorites(); - - const [configuration, setConfiguration] = useState< - ParameterValues | undefined - >(defaultConfiguration); - // Reported by ConfigurationWrapper; names this selection's thumbnail. - // Undefined until it reports, which is what gates saving. - const [canonical, setCanonical] = useState(undefined); - const [record, setRecord] = useState(undefined); - - const favorite = favoritesData?.favorites[favoriteId]; - const insertable = - favorite && insertables - ? insertables[favorite.insertableId] - : undefined; - - const insertableName = insertable?.name; - useEffect(() => { - if (insertableName === undefined) { - return; - } - modals.updateModal({ - modalId, - title: ( - } - /> - ) - }); - }, [modalId, insertableName, record]); - - const setDefaultConfigurationMutation = useMutation({ + return useMutation({ mutationKey: ["set-default-configuration"], mutationFn: async () => { // The selection as made, not its canonical form, which would drop // a value that is the parameter's default or a hidden one. return apiPost( "/default-configuration" + toFavoritePath(favoriteId), - { body: { defaultConfiguration: configuration } } + { body: { defaultConfiguration } } ); }, - onMutate: async () => { const queryKey = favoritesQueryKey(libraryId); await queryClient.cancelQueries({ queryKey }); @@ -93,8 +62,8 @@ export function FavoriteMenuContent( getQueryUpdater((data: FavoritesData) => { const fav = data.favorites[favoriteId]; if (fav) { - fav.defaultConfiguration = configuration; - fav.canonicalConfiguration = canonical; + fav.defaultConfiguration = defaultConfiguration; + fav.canonicalConfiguration = canonicalConfiguration; } return data; }) @@ -112,6 +81,41 @@ export function FavoriteMenuContent( }, onSettled: refreshFavorites }); +} + +export function FavoriteMenuContent( + props: FavoriteMenuContentProps +): ReactNode { + const { favoriteId, modalId, defaultConfiguration } = props; + + const insertables = useLibraryQuery().data?.insertables; + const favoritesData = useFavoritesQuery().data; + + const [configuration, setConfiguration] = useState< + ParameterValues | undefined + >(defaultConfiguration); + // Reported by ConfigurationWrapper; names this selection's thumbnail. + // Undefined until it reports, which is what gates saving. + const [canonical, setCanonical] = useState(undefined); + const [record, setRecord] = useState(undefined); + + const favorite = favoritesData?.favorites[favoriteId]; + const insertable = + favorite && insertables + ? insertables[favorite.insertableId] + : undefined; + + useMenuTitle(modalId, { + name: insertable?.name, + record, + icon: + }); + + const setDefaultConfigurationMutation = useSetDefaultConfigurationMutation( + favoriteId, + configuration, + canonical + ); if (!insertable) { return null; diff --git a/src/frontend/features/insert/components/configurations.tsx b/src/frontend/features/insert/components/configurations.tsx index 77d6a975..04689b86 100644 --- a/src/frontend/features/insert/components/configurations.tsx +++ b/src/frontend/features/insert/components/configurations.tsx @@ -71,43 +71,33 @@ function handleBooleanChange(handler: Dispatch) { handler((event.target as HTMLInputElement).checked); } -export function ConfigurationWrapper(props: ConfigurationWrapperProps) { - const { - insertableId, - microversionId, - configuration, - setConfiguration, - onCanonicalConfiguration, - onRecord - } = props; - - const query = useConfigurationQuery(insertableId, microversionId); - - const search = useSearch({ from: "/app" }); - // Units come from the current document; empty when not connected to one, in - // which case each quantity renders in its own unit (see getEvaluateOptions). - const isConnected = useIsConnectedToOnshape(); - const unitInfoQuery = useUnitInfoQuery(search, isConnected); - const unitInfo = unitInfoQuery.data ?? EMPTY_UNIT_INFO; - +/** Seeds an unset configuration with every parameter's own default. */ +function useDefaultConfiguration( + parameters: ConfigurationParameter[] | undefined, + configuration: ParameterValues | undefined, + setConfiguration: Dispatch +) { useEffect(() => { - // Doing this in a useEffect rather than a .then inside useQuery to prevent some buggy behavior - // Only fill in the configuration if it isn't already set - if (!query.data || configuration) { + // In an effect rather than a .then inside useQuery, which misbehaved. + if (!parameters || configuration) { return; } - const defaultConfiguration = query.data.parameters.reduce( - (configuration, parameter) => { - configuration[parameter.id] = parameter.default; - return configuration; - }, - {} as ParameterValues + setConfiguration( + Object.fromEntries( + parameters.map((parameter) => [parameter.id, parameter.default]) + ) ); - setConfiguration(defaultConfiguration); - }, [query.data, configuration, setConfiguration]); + }, [parameters, configuration, setConfiguration]); +} - const parameters = query.data?.parameters; - const records = query.data?.records; +/** Reports the selection's canonical form, and the record it resolves to. */ +function useReportSelection( + parameters: ConfigurationParameter[] | undefined, + records: SearchRecord[] | undefined, + configuration: ParameterValues | undefined, + onCanonicalConfiguration?: (canonicalConfiguration: string) => void, + onRecord?: (record: SearchRecord | undefined) => void +) { useEffect(() => { if (!parameters || !configuration) { return; @@ -129,6 +119,36 @@ export function ConfigurationWrapper(props: ConfigurationWrapperProps) { onCanonicalConfiguration, onRecord ]); +} + +export function ConfigurationWrapper(props: ConfigurationWrapperProps) { + const { + insertableId, + microversionId, + configuration, + setConfiguration, + onCanonicalConfiguration, + onRecord + } = props; + + const query = useConfigurationQuery(insertableId, microversionId); + + const search = useSearch({ from: "/app" }); + // Units come from the current document; empty when not connected to one, in + // which case each quantity renders in its own unit (see getEvaluateOptions). + const isConnected = useIsConnectedToOnshape(); + const unitInfoQuery = useUnitInfoQuery(search, isConnected); + const unitInfo = unitInfoQuery.data ?? EMPTY_UNIT_INFO; + + const parameters = query.data?.parameters; + useDefaultConfiguration(parameters, configuration, setConfiguration); + useReportSelection( + parameters, + query.data?.records, + configuration, + onCanonicalConfiguration, + onRecord + ); // isLoading, not isPending: the units query sits disabled (and so forever // pending) when there is no document to ask. diff --git a/src/frontend/features/insert/components/insert-menu.tsx b/src/frontend/features/insert/components/insert-menu.tsx index a7274ee5..abef545b 100644 --- a/src/frontend/features/insert/components/insert-menu.tsx +++ b/src/frontend/features/insert/components/insert-menu.tsx @@ -1,14 +1,16 @@ import { useSearch } from "@tanstack/react-router"; import { ReactNode, useCallback, useEffect, useState } from "react"; -import { getFavoriteForInsertable } from "@backend/features/favorites/contract"; +import { + type Favorite, + getFavoriteForInsertable +} from "@backend/features/favorites/contract"; import { InsertableOut } from "@backend/features/library/contract"; import { ElementType } from "@backend/lib/onshape/element-type"; import { Button, Checkbox, Group } from "@mantine/core"; import { InfoIcon, PlusIcon } from "@phosphor-icons/react"; import { IconSize } from "../../../lib/style-constants"; import { AppModalBody, AppModalFooter } from "../../../components/app-modal"; -import { MenuTitle } from "../../../components/app-title"; -import { modals } from "@mantine/modals"; +import { useMenuTitle } from "../../../components/app-title"; import { showQuickInsertTip } from "../quick-insert-tip"; import { PreviewImageCard } from "../../thumbnails/components/thumbnail"; import { FavoriteButton } from "../../favorites/components/favorite-button"; @@ -40,14 +42,12 @@ interface InsertMenuContentProps { onInsert: () => void; } -export function InsertMenuContent(props: InsertMenuContentProps): ReactNode { - const { insertable, modalId, openedAt, onInsert } = props; - const favorites = useFavoritesQuery().data?.favorites; - const isSignedIn = useIsSignedIn(); - - const [configuration, setConfiguration] = useState< - ParameterValues | undefined - >(props.defaultConfiguration); +/** + * The selection the menu holds, in both the form insert sends and the canonical + * one that names a thumbnail, plus whether it still stands where it opened. + */ +function useInsertSelection(defaultConfiguration?: ParameterValues) { + const [configuration, setConfiguration] = useState(defaultConfiguration); // Reported by ConfigurationWrapper, which has the parameters and units the // canonical form needs. Empty means the element's default configuration. const [canonicalConfiguration, setCanonicalConfiguration] = useState( @@ -57,10 +57,34 @@ export function InsertMenuContent(props: InsertMenuContentProps): ReactNode { // on the card would have inserted. Absent until the parameters load. const [openedWith, setOpenedWith] = useState(); - const handleCanonicalConfiguration = useCallback((canonical: string) => { + const onCanonicalConfiguration = useCallback((canonical: string) => { setCanonicalConfiguration(canonical); setOpenedWith((opened) => opened ?? canonical); }, []); + + return { + configuration, + setConfiguration, + canonicalConfiguration, + onCanonicalConfiguration, + isUnchanged: + canonicalConfiguration === + (openedWith ?? DEFAULT_CANONICAL_CONFIGURATION) + }; +} + +export function InsertMenuContent(props: InsertMenuContentProps): ReactNode { + const { insertable, modalId, openedAt, onInsert } = props; + const favorites = useFavoritesQuery().data?.favorites; + const isSignedIn = useIsSignedIn(); + + const { + configuration, + setConfiguration, + canonicalConfiguration, + onCanonicalConfiguration, + isUnchanged + } = useInsertSelection(props.defaultConfiguration); const [record, setRecord] = useState(undefined); // A part with no parameters has one record — the element's own part data — // which no ConfigurationWrapper is mounted to report, but the title wants. @@ -70,19 +94,10 @@ export function InsertMenuContent(props: InsertMenuContentProps): ReactNode { !insertable.isConfigurable ).data?.records[0]; - // The title lives in the modal's header, so it's updated rather than - // rendered: the header follows the configuration as the user changes it. - useEffect(() => { - modals.updateModal({ - modalId, - title: ( - - ) - }); - }, [modalId, insertable.name, record, soleRecord]); + useMenuTitle(modalId, { + name: insertable.name, + record: record ?? soleRecord + }); useEffect(() => { // Only once known: pending reads as signed out, which would prompt a @@ -106,7 +121,7 @@ export function InsertMenuContent(props: InsertMenuContentProps): ReactNode { microversionId={insertable.microversionId} configuration={configuration} setConfiguration={setConfiguration} - onCanonicalConfiguration={handleCanonicalConfiguration} + onCanonicalConfiguration={onCanonicalConfiguration} onRecord={setRecord} /> ); @@ -124,43 +139,75 @@ export function InsertMenuContent(props: InsertMenuContentProps): ReactNode { /> {parameters} - - - - - - - - - - - + ); } +interface InsertMenuFooterProps { + insertable: InsertableOut; + favorite: Favorite | undefined; + configuration?: ParameterValues; + canonicalConfiguration: string; + /** Whether the selection still stands where the menu opened. */ + isUnchanged: boolean; + openedAt: number; + onInsert: () => void; +} + +/** Favoriting and the row's own menu, then the buttons that do the inserting. */ +function InsertMenuFooter(props: InsertMenuFooterProps): ReactNode { + const { + insertable, + favorite, + configuration, + canonicalConfiguration, + isUnchanged, + openedAt, + onInsert + } = props; + return ( + + + + + + + + + + + + ); +} + interface InsertButtonsProps { /** * Whether the configuration is still the one the menu opened with, which a diff --git a/src/frontend/features/library/components/group-card.tsx b/src/frontend/features/library/components/group-card.tsx index bafd5673..996f26e6 100644 --- a/src/frontend/features/library/components/group-card.tsx +++ b/src/frontend/features/library/components/group-card.tsx @@ -130,12 +130,12 @@ export function GroupAdminContextMenu({ ); } -function ShowAllElementsMenuItem({ - insertableOrder -}: { +interface AllElementsVisibilityProps { insertableOrder: string[]; -}): ReactNode { - const mutation = useSetVisibilityMutation(insertableOrder, true); +} + +function ShowAllElementsMenuItem(props: AllElementsVisibilityProps): ReactNode { + const mutation = useSetVisibilityMutation(props.insertableOrder, true); return ( { - if (props.onClick) { - props.onClick(); - } - - if (isAssemblyInPartStudio) { - openCannotDeriveAssemblyAlert(); - return; - } + const openMenu = () => { + props.onClick?.(); + if (isAssemblyInPartStudio) { + openCannotDeriveAssemblyAlert(); + return; + } + openInsertMenu({ insertable, defaultConfiguration: hitConfiguration }); + }; - openInsertMenu({ - insertable, - defaultConfiguration: hitConfiguration - }); + const thumbnail = ( + + ); + + return ( + - } + thumbnail={thumbnail} showHiddenTag={!insertable.isVisible} buildStatusBadge={ {applyRanges(text, positions ?? [])}; +} + +export function HighlightedText(props: HighlightedTextProps): ReactNode { + const { text, positions = [] } = props; + return <>{applyRanges(text, positions)}; } function applyRanges(str: string, ranges: Position[]) { diff --git a/src/frontend/features/thumbnails/components/thumbnail.tsx b/src/frontend/features/thumbnails/components/thumbnail.tsx index 13aa4629..b3a99fc1 100644 --- a/src/frontend/features/thumbnails/components/thumbnail.tsx +++ b/src/frontend/features/thumbnails/components/thumbnail.tsx @@ -10,7 +10,12 @@ import { ElementPath } from "@backend/lib/onshape/path"; import { Box, Card, Center, HoverCard, Loader } from "@mantine/core"; import { QuestionIcon } from "@phosphor-icons/react"; -import { ComponentPropsWithRef, ReactNode, useState } from "react"; +import { + ComponentPropsWithRef, + PropsWithChildren, + ReactNode, + useState +} from "react"; import { DEFAULT_CANONICAL_CONFIGURATION } from "@backend/features/configurations/canonical"; import { thumbnailUrl } from "@backend/features/thumbnails/keys"; import { SectionError } from "../../../components/app-zero-state"; @@ -174,6 +179,12 @@ interface PreviewImageProps { largeThumbnailUrl?: string; } +/** A stored size, so the bytes a preview fetch returns are worth caching. */ +const PREVIEW_SIZE = ThumbnailSize.LARGE; + +/** Sized to the preview's footprint rather than to a row's. */ +const PREVIEW_SPINNER_SIZE = 36; + /** How often to re-check while the worker is still standing in the default. */ const PREVIEW_POLL_MS = 4000; @@ -195,41 +206,29 @@ function useLastRenderedUrl(image?: LoadedImage): string | undefined { return lastRendered; } -export function PreviewImage(props: PreviewImageProps): ReactNode { - const { - path, - insertableId, - microversionId, - canonicalConfiguration, - largeThumbnailUrl - } = props; - // A stored size, so the bytes this fetch returns are worth caching. - const size = ThumbnailSize.LARGE; - const isSignedIn = useIsSignedIn(); - const isConnected = useIsConnectedToOnshape(); - const isFetchingConfiguration = useIsFetchingConfiguration( - insertableId, - microversionId - ); - const targetElementType = useTargetElementType(); - +/** + * Polls for the configuration's render, which the worker produces in a + * workflow: the first request starts one, and the element default stands in + * until it lands. + */ +function usePreviewThumbnail(props: PreviewImageProps, enabled: boolean) { + const { path, insertableId, microversionId, canonicalConfiguration } = + props; // A url per poll: the browser caches images by url for the life of the // page, so reusing one leaves the stand-in up however often we refetch. const pollUrl = (attempt: number) => thumbnailUrl({ elementId: path.elementId, microversionId, - size, + size: PREVIEW_SIZE, canonicalConfiguration, renderThumbnail: attempt % RENDER_EVERY_POLLS === 0, insertableId, attempt }); - // The worker renders configurations in a workflow, so the first request - // starts one and stands in the element default until it lands. const queryKey = ["thumbnail", pollUrl(0)]; - const thumbnailQuery = useQuery({ + const query = useQuery({ queryKey, queryFn: ({ signal, client }) => loadImageResult( @@ -241,20 +240,50 @@ export function PreviewImage(props: PreviewImageProps): ReactNode { refetchInterval: (query) => query.state.data?.isFallback ? PREVIEW_POLL_MS : false, retry: 2, - enabled: !isFetchingConfiguration && isSignedIn === true + enabled }); - const lastRenderedUrl = useLastRenderedUrl(thumbnailQuery.data); + return { query, lastRenderedUrl: useLastRenderedUrl(query.data) }; +} + +interface PreviewBoxProps extends PropsWithChildren { + heightAndWidth: HeightAndWidth; +} + +/** Holds the preview's own footprint, whatever is being shown in it. */ +function PreviewBox(props: PreviewBoxProps): ReactNode { + const { heightAndWidth, children } = props; + return ( +
+ {children} +
+ ); +} - const heightAndWidth = getHeightAndWidth(size, 0.7); +export function PreviewImage(props: PreviewImageProps): ReactNode { + const { insertableId, microversionId, largeThumbnailUrl } = props; + const isSignedIn = useIsSignedIn(); + const isConnected = useIsConnectedToOnshape(); + const isFetchingConfiguration = useIsFetchingConfiguration( + insertableId, + microversionId + ); + const targetElementType = useTargetElementType(); + const { query, lastRenderedUrl } = usePreviewThumbnail( + props, + !isFetchingConfiguration && isSignedIn === true + ); + + const heightAndWidth = getHeightAndWidth(PREVIEW_SIZE, 0.7); + const spinner = ( + + + + ); // Not known yet: the stored thumbnail would be swapped for the live preview // a moment later. if (isSignedIn === undefined) { - return ( -
- -
- ); + return spinner; } // Not signed in: no live Onshape preview, so show the stored thumbnail @@ -264,21 +293,16 @@ export function PreviewImage(props: PreviewImageProps): ReactNode { ); } - // Placeholder data is the previous configuration's render, so the spinner - // has to cover it too: what is on screen is not what was asked for. - const isWaiting = - thumbnailQuery.isPlaceholderData || thumbnailQuery.data?.isFallback; - - if (thumbnailQuery.isError) { + if (query.isError) { const action = targetElementType === ElementType.ASSEMBLY ? "insert" : "derive"; return ( -
+ -
- ); - } else if (!thumbnailQuery.data) { - return ( -
- -
+ ); } + if (!query.data) { + return spinner; + } // A stand-in must not displace a render the user already has. const previewUrl = - thumbnailQuery.data.isFallback && lastRenderedUrl + query.data.isFallback && lastRenderedUrl ? lastRenderedUrl - : thumbnailQuery.data.url; + : query.data.url; + // Placeholder data is the previous configuration's render, so the spinner + // has to cover it too: what is on screen is not what was asked for. + const isWaiting = query.isPlaceholderData || query.data.isFallback; return ( <> diff --git a/src/frontend/routes/app/library/$libraryId/index.tsx b/src/frontend/routes/app/library/$libraryId/index.tsx index 953b4a13..b1b5b43b 100644 --- a/src/frontend/routes/app/library/$libraryId/index.tsx +++ b/src/frontend/routes/app/library/$libraryId/index.tsx @@ -49,7 +49,8 @@ interface Section { setOpened: (opened: boolean) => void; } -function HomeList(): ReactNode { +/** The sections the home list shows, in the order they are stacked. */ +function useHomeSections(): Section[] { const uiState = useGetUiState(); const setUiState = useSetUiState(); // Not persisted: search results open on every visit, unlike the library. @@ -57,94 +58,106 @@ function HomeList(): ReactNode { const libraryId = useLibraryId(); const isSignedIn = useIsSignedIn(); - const sections: Section[] = []; - // Favorites are per-user and hidden until signed in. - if (isSignedIn) { - sections.push({ - value: "favorites", - icon: , - title: , - panel: , - opened: uiState.isFavoritesOpen, - setOpened: (opened) => setUiState({ isFavoritesOpen: opened }) - }); - } + const favorites: Section = { + value: "favorites", + icon: , + title: , + panel: , + opened: uiState.isFavoritesOpen, + setOpened: (opened) => setUiState({ isFavoritesOpen: opened }) + }; + + const search: Section = { + value: "search", + icon: ( + + ), + title: , + panel: ( + + ), + opened: isSearchOpen, + setOpened: setIsSearchOpen + }; + + const library: Section = { + value: "library", + icon: , + title: , + panel: , + opened: uiState.isLibraryOpen, + setOpened: (opened) => setUiState({ isLibraryOpen: opened }) + }; // One slot below favorites, showing search results while a query is active // and the library otherwise. The differing `value` remounts it on the swap. - if (uiState.searchQuery) { - sections.push({ - value: "search", - icon: ( - - ), - title: , - panel: ( - - ), - opened: isSearchOpen, - setOpened: setIsSearchOpen - }); - } else { - sections.push({ - value: "library", - icon: ( - - ), - title: , - panel: , - opened: uiState.isLibraryOpen, - setOpened: (opened) => setUiState({ isLibraryOpen: opened }) - }); - } + return [ + ...(isSignedIn ? [favorites] : []), + uiState.searchQuery ? search : library + ]; +} +interface SectionAccordionProps { + sections: Section[]; +} + +/** Stacks the sections, each opening and closing on its own. */ +function SectionAccordion(props: SectionAccordionProps): ReactNode { + const { sections } = props; const handleChange = (opened: string[]) => { for (const section of sections) { section.setOpened(opened.includes(section.value)); } }; + return ( + section.opened) + .map((section) => section.value)} + onChange={handleChange} + styles={{ + // On the control, so a collapsed section still divides from + // the next one; content closes off an open one. + control: { + borderBottom: BORDER, + minHeight: SECTION_HEADER_HEIGHT + }, + // Its own padding would outgrow that height. + label: { paddingBlock: 0 }, + content: { padding: 0, borderBottom: BORDER }, + icon: TITLE_ICON_NUDGE + }} + > + {sections.map((section) => ( + + + {section.title} + + {section.panel} + + ))} + + ); +} + +function HomeList(): ReactNode { + const sections = useHomeSections(); return ( <> - section.opened) - .map((section) => section.value)} - onChange={handleChange} - styles={{ - // On the control, so a collapsed section still divides from - // the next one; content closes off an open one. - control: { - borderBottom: BORDER, - minHeight: SECTION_HEADER_HEIGHT - }, - // Its own padding would outgrow that height. - label: { paddingBlock: 0 }, - content: { padding: 0, borderBottom: BORDER }, - icon: TITLE_ICON_NUDGE - }} - > - {sections.map((section) => ( - - - {section.title} - - {section.panel} - - ))} - + );