From c7571dfdc74bcf01338f535693fc75c3a7e940a3 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Thu, 30 Jul 2026 21:09:11 +0800 Subject: [PATCH] fix(view): the chart view gets a label and an icon in the view switcher (#2916) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 at all and with its raw type key as the label, while every sibling view showed a glyph and a capitalized name. The label and glyph are not a fresh product call: `plugin-list`'s own switcher, `app-shell`'s `ObjectView` and `CreateViewDialog`, and the `console.objectView.viewTypeChart` translation already agree on `BarChart3` and `'Chart'`, so adopting them makes the switcher stop disagreeing with the rest of the UI. An explicit per-view `label`/`icon` still overrides the default. Why the compiler never caught a hole in a map whose whole purpose is to be exhaustive: `@object-ui/plugin-view` had no `type-check` script, so CI never evaluated `Record`. The package now type-checks its sources (`tsc --noEmit`) and, chained off the same script, its tests (`tsconfig.test.json`) — and its `DEBT` entry in `scripts/check-type-check-coverage.mjs` is deleted, as that guard requires once the gap closes. Reverting either `chart` entry now fails the build, which is how this fix was verified rather than assumed. The package's third reported error is fixed too: `ViewTabBar` typed the drag listener bag as a hand-written `Record void>`, a structural fork of a type `dnd-kit` already exports. It now uses `DraggableSyntheticListeners`, so the fork cannot drift again. Compiling the tests for the first time surfaced two real findings. Three `renderListView` spies destructured a `listSchema` they never read (every assertion reads the schema off `mock.calls`); they now take `_props`, matching the argless spies already in that file. And the test program pulls in `ObjectView.tsx`, which gates a spec-compliance warning on `process.env.NODE_ENV` — so `tsconfig.test.json` names `node` alongside `@testing-library/jest-dom`, exactly as `plugin-report` (the other package whose source reads `process.env`) already does. New `ViewSwitcher.test.tsx` covers the user-visible half the type gate cannot: that every `ViewType` renders a label and an icon, that `chart` reads "Chart" rather than falling back to its type key, and that an explicit label/icon still wins. Both regression tests fail with either `chart` entry removed. Refs #2911, #2915 Co-Authored-By: Claude Fable 5 --- .../view-switcher-chart-label-and-icon.md | 29 ++++++ packages/plugin-view/package.json | 1 + packages/plugin-view/src/ViewSwitcher.tsx | 3 + packages/plugin-view/src/ViewTabBar.tsx | 5 +- .../src/__tests__/ObjectView.test.tsx | 6 +- .../src/__tests__/ViewSwitcher.test.tsx | 94 +++++++++++++++++++ packages/plugin-view/tsconfig.json | 16 +++- packages/plugin-view/tsconfig.test.json | 25 +++++ scripts/check-type-check-coverage.mjs | 1 - 9 files changed, 172 insertions(+), 8 deletions(-) create mode 100644 .changeset/view-switcher-chart-label-and-icon.md create mode 100644 packages/plugin-view/src/__tests__/ViewSwitcher.test.tsx create mode 100644 packages/plugin-view/tsconfig.test.json 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 0000000000..a7e2fd5f89 --- /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 e687f7585f..23a0c4eedc 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 794968923d..4f9a704242 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 485fc2e62a..2ad5456b7c 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 0534f6a6de..276bd30fd8 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 0000000000..4644880964 --- /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 c1d2d57642..810c684918 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 0000000000..3968124991 --- /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 cf58755df5..d35e476fd8 100644 --- a/scripts/check-type-check-coverage.mjs +++ b/scripts/check-type-check-coverage.mjs @@ -40,7 +40,6 @@ const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); // #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" }, "@object-ui/layout": { errors: 2, issue: 2918, note: "nav type 'component' is implemented but absent from NavigationItemType and its zod enum" }, };