diff --git a/.changeset/view-switcher-chart-label-and-icon.md b/.changeset/view-switcher-chart-label-and-icon.md new file mode 100644 index 000000000..a7e2fd5f8 --- /dev/null +++ b/.changeset/view-switcher-chart-label-and-icon.md @@ -0,0 +1,29 @@ +--- +"@object-ui/plugin-view": patch +--- + +fix(view): the chart view gets a label and an icon in the view switcher — objectui#2916 + +`ViewSwitcher`'s two exhaustive `Record` maps — `DEFAULT_VIEW_LABELS` +and `DEFAULT_VIEW_ICONS` — were each missing the `chart` key. `chart` is a member +of `ViewType` and `plugin-charts` is a registered view, so a chart tab rendered +with no icon and with its raw type key `chart` as the label, while every sibling +view showed a glyph and a capitalized name. + +Both maps now carry `chart`, using the same `BarChart3` glyph and `'Chart'` label +that `plugin-list`'s switcher, `app-shell`'s `ObjectView`/`CreateViewDialog`, and +the `console.objectView.viewTypeChart` translation already agree on — so the +switcher no longer disagrees with the rest of the UI. An explicit per-view +`label`/`icon` still overrides the default, unchanged. + +Why the compiler did not catch it: `@object-ui/plugin-view` had no `type-check` +script, so `Record` — the exhaustiveness guard that exists precisely +to make a missing member a compile error — was never evaluated by CI. The package +now type-checks both its sources and its tests, and its `DEBT` entry in +`scripts/check-type-check-coverage.mjs` is deleted. Compiling the tests for the +first time also surfaced three unused destructured spy parameters, and the +package's one remaining reported error (a `dnd-kit` `SyntheticListenerMap` +mismatch in `ViewTabBar`) is fixed by typing the listener bag as `dnd-kit`'s own +exported `DraggableSyntheticListeners` rather than a hand-written structural fork. + +Refs objectui#2911, objectui#2915. diff --git a/packages/plugin-view/package.json b/packages/plugin-view/package.json index e687f7585..23a0c4eed 100644 --- a/packages/plugin-view/package.json +++ b/packages/plugin-view/package.json @@ -17,6 +17,7 @@ "scripts": { "build": "vite build", "test": "vitest run", + "type-check": "tsc --noEmit && tsc -p tsconfig.test.json", "lint": "eslint ." }, "dependencies": { diff --git a/packages/plugin-view/src/ViewSwitcher.tsx b/packages/plugin-view/src/ViewSwitcher.tsx index 794968923..4f9a70424 100644 --- a/packages/plugin-view/src/ViewSwitcher.tsx +++ b/packages/plugin-view/src/ViewSwitcher.tsx @@ -24,6 +24,7 @@ import { SchemaRenderer } from '@object-ui/react'; import type { ViewSwitcherSchema, ViewType } from '@object-ui/types'; import { Activity, + BarChart3, Calendar, FileText, GanttChartSquare, @@ -64,6 +65,7 @@ const DEFAULT_VIEW_LABELS: Record = { map: 'Map', gallery: 'Gallery', gantt: 'Gantt', + chart: 'Chart', tree: 'Tree', }; @@ -77,6 +79,7 @@ const DEFAULT_VIEW_ICONS: Record = { map: Map, gallery: Images, gantt: GanttChartSquare, + chart: BarChart3, tree: ListTree, }; diff --git a/packages/plugin-view/src/ViewTabBar.tsx b/packages/plugin-view/src/ViewTabBar.tsx index 485fc2e62..2ad5456b7 100644 --- a/packages/plugin-view/src/ViewTabBar.tsx +++ b/packages/plugin-view/src/ViewTabBar.tsx @@ -78,6 +78,7 @@ import { useSensor, useSensors, type DragEndEvent, + type DraggableSyntheticListeners, } from '@dnd-kit/core'; import { SortableContext, @@ -210,7 +211,7 @@ const SortableTab: React.FC<{ children: (props: { setNodeRef: (node: HTMLElement | null) => void; style: React.CSSProperties; - listeners: Record void> | undefined; + listeners: DraggableSyntheticListeners; attributes: Record; isDragging: boolean; }) => React.ReactNode; @@ -413,7 +414,7 @@ export const ViewTabBar: React.FC = ({ const visibilityIcon = getVisibilityIcon(view); const buildTabContent = (dragHandleProps?: { - listeners: Record void> | undefined; + listeners: DraggableSyntheticListeners; attributes: Record; isDragging?: boolean; }) => ( diff --git a/packages/plugin-view/src/__tests__/ObjectView.test.tsx b/packages/plugin-view/src/__tests__/ObjectView.test.tsx index 0534f6a6d..276bd30fd 100644 --- a/packages/plugin-view/src/__tests__/ObjectView.test.tsx +++ b/packages/plugin-view/src/__tests__/ObjectView.test.tsx @@ -626,7 +626,7 @@ describe('ObjectView', () => { showSort: false, }; - const renderListViewSpy = vi.fn(({ schema: listSchema }: any) => ( + const renderListViewSpy = vi.fn((_props: any) => (
Custom ListView
)); @@ -651,7 +651,7 @@ describe('ObjectView', () => { objectName: 'contacts', }; - const renderListViewSpy = vi.fn(({ schema: listSchema }: any) => ( + const renderListViewSpy = vi.fn((_props: any) => (
Custom ListView
)); @@ -744,7 +744,7 @@ describe('ObjectView', () => { objectName: 'contacts', }; - const renderListViewSpy = vi.fn(({ schema: listSchema }: any) => ( + const renderListViewSpy = vi.fn((_props: any) => (
Custom ListView
)); diff --git a/packages/plugin-view/src/__tests__/ViewSwitcher.test.tsx b/packages/plugin-view/src/__tests__/ViewSwitcher.test.tsx new file mode 100644 index 000000000..464488096 --- /dev/null +++ b/packages/plugin-view/src/__tests__/ViewSwitcher.test.tsx @@ -0,0 +1,94 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import { ViewSwitcher } from '../ViewSwitcher'; +import type { ViewSwitcherSchema, ViewType } from '@object-ui/types'; + +// Mock @object-ui/react to avoid circular dependency issues; mirrors +// ObjectView.test.tsx, including the data-invalidation bus that +// @object-ui/components imports at module-eval time. +vi.mock('@object-ui/react', async () => { + const React = await import('react'); + return { + SchemaRenderer: ({ schema }: any) => ( +
+ {schema?.type} +
+ ), + SchemaRendererContext: React.createContext(null), + subscribeDataChanges: () => () => {}, + notifyDataChanged: () => {}, + }; +}); + +// Every member of `ViewType`. Declared as `ViewType[]` rather than inferred, so +// adding a member to the union without extending this list is a compile error +// here too — the same guard `DEFAULT_VIEW_LABELS` / `DEFAULT_VIEW_ICONS` get +// from being `Record` (#2916). +const ALL_VIEW_TYPES: ViewType[] = [ + 'list', + 'detail', + 'grid', + 'kanban', + 'calendar', + 'timeline', + 'map', + 'gallery', + 'gantt', + 'chart', + 'tree', +]; + +function schemaFor(types: ViewType[]): ViewSwitcherSchema { + return { + type: 'view-switcher', + variant: 'buttons', + views: types.map(type => ({ type })), + }; +} + +describe('ViewSwitcher default view labels and icons', () => { + it('renders a non-empty label and an icon for every ViewType', () => { + render(); + + for (const type of ALL_VIEW_TYPES) { + // A missing entry falls back to the raw type key for the label and to no + // icon at all, which is what a hole in either Record map + // looked like on screen. + const button = screen + .getAllByRole('button') + .find(b => b.textContent?.trim().toLowerCase() === type || b.textContent?.trim() === type); + expect(button, `no button rendered for view type "${type}"`).toBeDefined(); + expect(button!.querySelector('svg'), `view type "${type}" rendered without an icon`).not.toBeNull(); + } + }); + + it('labels the chart view "Chart" rather than falling back to the type key', () => { + render(); + + expect(screen.getByText('Chart')).toBeInTheDocument(); + expect(screen.queryByText('chart')).toBeNull(); + }); + + it('still lets an explicit label and icon override the defaults', () => { + render( + + ); + + expect(screen.getByText('Revenue')).toBeInTheDocument(); + expect(screen.queryByText('Chart')).toBeNull(); + }); +}); diff --git a/packages/plugin-view/tsconfig.json b/packages/plugin-view/tsconfig.json index c1d2d5764..810c68491 100644 --- a/packages/plugin-view/tsconfig.json +++ b/packages/plugin-view/tsconfig.json @@ -2,7 +2,19 @@ "extends": "../../tsconfig.json", "compilerOptions": { "outDir": "dist", - "jsx": "react-jsx" + "jsx": "react-jsx", + // Drop the root tsconfig's source-tree `paths` (keeping only the local `@/*` + // alias) so `@object-ui/*` and `@objectstack/spec` resolve through the + // workspace dependency's built `.d.ts` instead of pulling sibling package + // sources in as program inputs — ~104 TS6059 rootDir errors otherwise (#2915). + "baseUrl": ".", + "paths": { + "@/*": ["src/*"] + } }, - "include": ["src"] + "include": ["src"], + // Tests are type-checked by `tsconfig.test.json`, which the `type-check` + // script chains: they need `@testing-library/jest-dom`'s global matcher + // augmentation, and this project is the build, so they must not reach `dist`. + "exclude": ["node_modules", "dist", "**/*.test.ts", "**/*.test.tsx"] } diff --git a/packages/plugin-view/tsconfig.test.json b/packages/plugin-view/tsconfig.test.json new file mode 100644 index 000000000..396812499 --- /dev/null +++ b/packages/plugin-view/tsconfig.test.json @@ -0,0 +1,25 @@ +{ + // Type-checks this package's TESTS, which `tsconfig.json` excludes. + // See `packages/types/tsconfig.test.json` for why that exclusion was a hole: + // the build correctly keeps tests out of `dist`, but nothing else compiled + // them, so a test could assert a contract the compiler never checked. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + // The package build emits `dist`; this project emits nothing. + "composite": false, + // Naming `types` at all switches off automatic `@types/*` inclusion, so + // BOTH entries are load-bearing: + // - jest-dom: the `toBeInTheDocument` matchers `ObjectView.test.tsx` uses + // are a global augmentation, not an import, and it does not live under + // `@types/` so it is never picked up automatically. + // - node: the test program pulls in `ObjectView.tsx`, which gates its + // spec-compliance warning on `process.env.NODE_ENV`. + "types": ["@testing-library/jest-dom", "node"], + // Drop the root tsconfig's source-tree `paths` so `@object-ui/*` and + // `@objectstack/spec` resolve through the workspace dependency's built + // `.d.ts` instead of pulling sibling sources in as program inputs (TS6059). + "paths": {} + }, + "include": ["src/**/*.test.ts", "src/**/*.test.tsx"] +} diff --git a/scripts/check-type-check-coverage.mjs b/scripts/check-type-check-coverage.mjs index c14bbdef2..557c9ee8f 100644 --- a/scripts/check-type-check-coverage.mjs +++ b/scripts/check-type-check-coverage.mjs @@ -39,9 +39,9 @@ const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); // `"type-check": "tsc --noEmit"`, then delete the entry. Counts are from the // #2911 sweep (bare `tsc --noEmit` with the `paths` override its type-checked // peers already carry, so the TS6059 rootDir noise is excluded). -const DEBT = { - "@object-ui/plugin-view": { errors: 3, issue: 2916, note: "Record missing the 'chart' key" }, -}; +// Empty, and worth keeping that way: every workspace package now either carries +// a `type-check` script or is declared in one of the exemption lists below. +const DEBT = {}; // Packages that are not compiled at all: documentation snippets with no build // script and no tsconfig, whose sources are read rather than run. Re-validated