Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
When a NavigationItem of type 'object' has a viewName (e.g. 'calendar',
'pipeline'), append /view/{viewName} to the generated href so the
correct list view is activated on navigation.
Changes:
- Add viewName property to NavigationItem interface
- Add viewName to Zod validation schema
- Update resolveHref in NavigationRenderer to append /view/{viewName}
- Update MobileBottomNav href generation in AppSchemaRenderer
- Add test for viewName href generation
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
|
@copilot 仔细评估是否已经完成了任务中的所有要求，目前我测试了日历识图还是显示成表格 |
…rch)
Three additional places in the console app built object navigation hrefs
without considering viewName:
- findFirstRoute() in App.tsx (home redirect)
- Mobile bottom nav in AppSidebar.tsx
- SearchResultsPage.tsx search results
All now append /view/{viewName} when present, matching the layout fix.
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
发现了 3 处遗漏 —— 除了
已在 a436d40 中全部修复，所有 6 处 href 生成逻辑现在都会在 |
|
@copilot 还是不行，看到的还是表格视图从头开始排查每一个细节，看问题到底出在哪里 |
Root cause: defineStack() validates config through Zod schemas which strip non-standard properties like listViews from objects. The mergeViewsIntoObjects() call in objectstack.shared.ts was being undone by the subsequent defineStack(). Fix: Apply defineStack() first for validation, then re-merge views from the raw config into the validated objects. Applied to both: - apps/console/objectstack.shared.ts (MSW/browser mode) - objectstack.config.ts (server mode) Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
找到了根本原因 —— 问题不在 href 生成，而是 视图数据管道。
结果： 修复方案（15ff2e6）：先执行
|
There was a problem hiding this comment.
Pull request overview
This PR fixes navigation to named object views (e.g. calendar/kanban) by (1) preserving viewName in generated hrefs and (2) ensuring listViews survives defineStack() validation so the console can resolve the correct view type instead of falling back to the default grid.
Changes:
- Add
viewName?: stringtoNavigationItemand its Zod schema. - Append
/view/${viewName}to object navigation hrefs across layout + console entry points, plus a layout unit test. - Re-merge stack-level
views[].listViewsinto validated objects afterdefineStack()(console MSW mode + server/dev config).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/types/src/app.ts | Adds viewName to NavigationItem type so apps can declare a target named view. |
| packages/types/src/zod/app.zod.ts | Updates Zod validation to preserve viewName in navigation configs. |
| packages/layout/src/NavigationRenderer.tsx | Updates resolveHref() to include /view/<viewName> for object nav items. |
| packages/layout/src/AppSchemaRenderer.tsx | Updates mobile bottom nav href generation to include /view/<viewName>. |
| packages/layout/src/tests/NavigationRenderer.test.tsx | Adds a unit test covering viewName href generation in NavigationRenderer. |
| apps/console/src/App.tsx | Updates initial home redirect route selection to include viewName when present. |
| apps/console/src/components/AppSidebar.tsx | Updates console mobile bottom nav links to include /view/<viewName>. |
| apps/console/src/components/SearchResultsPage.tsx | Updates search result hrefs for object items to include /view/<viewName>. |
| apps/console/objectstack.shared.ts | Applies defineStack() first, then re-merges listViews back into validated objects (MSW/browser). |
| objectstack.config.ts | Adds merged views to the validated stack and re-merges listViews into objects after defineStack() (server/dev). |
| if (!items || items.length === 0) return ''; | ||
| for (const item of items) { | ||
| if (item.type === 'object') return `${item.objectName}`; | ||
| if (item.type === 'object') return item.viewName ? `${item.objectName}/view/${item.viewName}` : `${item.objectName}`; |
There was a problem hiding this comment.
findFirstRoute() now includes viewName, but it still assumes item.objectName is present for type: 'object'. If the first visible object nav item is missing objectName, this will return undefined (or undefined/view/...) and break the app’s root redirect. Consider skipping object items without objectName and continuing the search.
| if (item.type === 'object') return item.viewName ? `${item.objectName}/view/${item.viewName}` : `${item.objectName}`; | |
| if (item.type === 'object') { | |
| // Skip object items that do not have a valid objectName | |
| if (!item.objectName) continue; | |
| return item.viewName ? `${item.objectName}/view/${item.viewName}` : `${item.objectName}`; | |
| } |
| const validated = defineStack(sharedConfig as Parameters<typeof defineStack>[0]); | ||
| export default { | ||
| ...validated, | ||
| objects: mergeViewsIntoObjects(validated.objects || [], allConfigs), | ||
| }; |
There was a problem hiding this comment.
This change re-merges listViews after defineStack() (objects: mergeViewsIntoObjects(validated.objects || [], allConfigs)), but sharedConfig.objects already ran mergeViewsIntoObjects() before validation. Since defineStack() strips listViews, the pre-merge is now redundant work and makes the pipeline harder to follow. Consider constructing sharedConfig.objects without listViews and applying mergeViewsIntoObjects() only once after validation.
| const objectPath = `${basePath}/${item.objectName ?? ''}`; | ||
| return { href: item.viewName ? `${objectPath}/view/${item.viewName}` : objectPath, external: false }; |
There was a problem hiding this comment.
resolveHref() builds an object href even when item.objectName is missing (it becomes ${basePath}/), and with viewName present it can produce a double-slash path like ${basePath}//view/<name>. There are existing configs/tests that create NavigationItem objects without objectName (e.g. designer hidden items), so this can cause navigation to unexpectedly route to the app root. Consider returning '#' (non-navigable) when objectName is absent, consistent with the dashboard/page/report cases.
| const objectPath = `${basePath}/${item.objectName ?? ''}`; | |
| return { href: item.viewName ? `${objectPath}/view/${item.viewName}` : objectPath, external: false }; | |
| if (!item.objectName) { | |
| return { href: '#', external: false }; | |
| } | |
| const objectPath = `${basePath}/${item.objectName}`; | |
| return { | |
| href: item.viewName ? `${objectPath}/view/${item.viewName}` : objectPath, | |
| external: false, | |
| }; |
| href = `${basePath}/${item.objectName}`; | ||
| if (item.viewName) href += `/view/${item.viewName}`; |
There was a problem hiding this comment.
MobileBottomNav constructs object hrefs with ${basePath}/${item.objectName} without guarding for a missing objectName. This can yield routes like /apps/foo/undefined and will now also append /view/${item.viewName} onto that. Since other code paths allow NavigationItem objects without objectName (e.g. designer-created hidden items), it would be safer to keep href = '#' unless objectName is present.
| href = `${basePath}/${item.objectName}`; | |
| if (item.viewName) href += `/view/${item.viewName}`; | |
| if (item.objectName) { | |
| href = `${basePath}/${item.objectName}`; | |
| if (item.viewName) href += `/view/${item.viewName}`; | |
| } else { | |
| href = '#'; | |
| } |
| {leaves.map((item) => { | ||
| const NavIcon = resolveIcon(item.icon); | ||
| let href = '#'; | ||
| if (item.type === 'object') href = `${basePath}/${item.objectName}`; | ||
| if (item.type === 'object') { | ||
| href = `${basePath}/${item.objectName}`; |
There was a problem hiding this comment.
There’s test coverage for href generation in packages/layout/src/__tests__/AppSchemaRenderer.test.tsx, but no test asserting the new viewName → /view/<name> behavior for the mobile bottom nav. Adding a focused test would help prevent regressions since href logic is duplicated across multiple components.
| return navItems.map((item: any) => { | ||
| let href = '#'; | ||
| if (item.type === 'object') href = `${baseUrl}/${item.objectName}`; | ||
| if (item.type === 'object') { |
There was a problem hiding this comment.
SearchResultsPage builds object hrefs using ${baseUrl}/${item.objectName} without checking that objectName exists. Navigation configs produced by the designer can include type: 'object' items without objectName, which would cause these results to show up with /apps/<app>/undefined (and possibly /view/... appended). Consider only creating an href when objectName is present (otherwise keep '#' so it gets filtered out).
| if (item.type === 'object') { | |
| if (item.type === 'object' && item.objectName) { |
| href = `${baseUrl}/${item.objectName}`; | ||
| if (item.viewName) href += `/view/${item.viewName}`; |
There was a problem hiding this comment.
Console mobile bottom nav constructs object hrefs with ${baseUrl}/${item.objectName} without guarding for missing objectName. If a type: 'object' navigation item lacks objectName, this produces /apps/<app>/undefined (and now may append /view/...). Consider leaving href as '#' unless objectName is defined, consistent with other target types.
| href = `${baseUrl}/${item.objectName}`; | |
| if (item.viewName) href += `/view/${item.viewName}`; | |
| if (item.objectName) { | |
| href = `${baseUrl}/${item.objectName}`; | |
| if (item.viewName) href += `/view/${item.viewName}`; | |
| } |
NavigationItemof type'object'with aviewName(e.g.calendar,pipeline) generated hrefs that dropped the view segment, always falling back to the default grid view. Additionally,defineStack()from@objectstack/specstrippedlistViewsfrom object definitions via Zod validation, so even with the correct URL the console'sObjectViewnever saw the calendar/kanban view definitions and always fell back to grid.Changes
Part 1: href generation — append
/view/${viewName}when presentpackages/types/src/app.ts— Add optionalviewNametoNavigationItempackages/types/src/zod/app.zod.ts— AddviewNameto Zod schemapackages/layout/src/NavigationRenderer.tsx—resolveHrefappends/view/${viewName}when presentpackages/layout/src/AppSchemaRenderer.tsx— Same fix inMobileBottomNavinline href logicapps/console/src/App.tsx—findFirstRoute()includesviewNamefor home redirectapps/console/src/components/AppSidebar.tsx— Console mobile bottom nav includesviewNameapps/console/src/components/SearchResultsPage.tsx— Search results includeviewNamepackages/layout/src/__tests__/NavigationRenderer.test.tsx— Test for viewName href generationPart 2: view data pipeline — preserve
listViewsthroughdefineStack()apps/console/objectstack.shared.ts— Re-mergelistViewsinto objects afterdefineStack()validation strips them (MSW/browser mode)objectstack.config.ts— Same fix plus add missingviewsarray to the merged config (server mode)defineStack()validates config through Zod schemas which strip non-standard properties likelistViewsfrom objects. The existingmergeViewsIntoObjects()call was being undone by the subsequentdefineStack(). The fix appliesdefineStack()first for validation, then re-merges views from the raw configs into the validated objects. Without this,ObjectViewreceivedobjectDef.listViews = undefinedand always fell back to a default grid view regardless of the URL.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.