fix(view): the chart view gets a label and an icon in the view switcher (#2916) - #3040
Merged
Conversation
…er (#2916) `ViewSwitcher`'s two exhaustive `Record<ViewType, …>` 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<ViewType, …>`. 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<string, (...args: any[]) => 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 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Conflict was structural, not semantic: #3039 (issue #2918, the `layout` nav-type gap) and this branch (issue #2916) each removed a different entry from the same three-line `DEBT` object in `scripts/check-type-check-coverage.mjs`. Both removals are correct and independent, so the resolution keeps neither — `DEBT` is now empty, which the guard reports as `0 known-broken`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2916
The gap
ViewSwitcher's two exhaustiveRecord<ViewType, …>maps —DEFAULT_VIEW_LABELSandDEFAULT_VIEW_ICONS— were each missing thechartkey.chartis a member ofViewTypeandplugin-chartsis 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 were already decided
The issue flagged this as a product call. It turns out not to be an open one — three other surfaces and the translation bundle already agree, so the switcher was the only place disagreeing:
plugin-listViewSwitcherBarChart3'Chart'app-shellObjectViewBarChart3app-shellCreateViewDialogBarChart3t('console.objectView.viewTypeChart')en.tstranslation'Chart'This PR adopts the same pair. An explicit per-view
label/iconstill overrides the default, unchanged.Why the compiler never caught a hole in a map built to be exhaustive
@object-ui/plugin-viewhad notype-checkscript, so CI never evaluatedRecord<ViewType, …>. The package now type-checks its sources (tsc --noEmit) and, chained off the same script, its tests (tsconfig.test.json) — and itsDEBTentry inscripts/check-type-check-coverage.mjsis deleted, as that guard requires once the gap closes.The gate is verified rather than assumed: removing either
chartentry reproduces the issue's exactTS2741and fails two tests.The other two reported errors
ViewTabBar/ dnd-kit. It typed the drag listener bag as a hand-writtenRecord<string, (...args: any[]) => void>— a structural fork of a type dnd-kit already exports. NowDraggableSyntheticListeners, so the fork cannot drift again.renderListViewspies destructured alistSchemathey never read (every assertion reads the schema offmock.calls); they now take_props, matching the argless spies already in that file. And the test program pulls inObjectView.tsx, which gates a spec-compliance warning onprocess.env.NODE_ENV— sotsconfig.test.jsonnamesnodealongside@testing-library/jest-dom, exactly asplugin-report(the other package whose source readsprocess.env) already does.Tests
New
ViewSwitcher.test.tsxcovers the user-visible half the type gate cannot: that everyViewTyperenders a label and an icon, thatchartreads "Chart" rather than falling back to its type key, and that an explicit label/icon still wins. ItsViewType[]list is declared, not inferred, so adding a union member without extending it is a compile error there too.Verified locally:
tsc --noEmit && tsc -p tsconfig.test.json— clean (was 3 errors + 4 uncompiled test files)check-type-check-coverage.mjs—42/45viatype-check(was 41/45), tests20/35(was 19/35)vitest run packages/plugin-view packages/plugin-list/src/__tests__— 313 passed / 17 filespnpm --filter @object-ui/plugin-view build— cleaneslint— 0 errorschartentry removed,TS2741returns and 2 of the 3 new tests fail on the icon and the label fallbackRefs #2911, #2915
🤖 Generated with Claude Code