Skip to content

Commit eee9dae

Browse files
Copilothotlong
andcommitted
fix: address all review feedback — FQN prefix, nested default override, a11y, collapsible sync
- Fix ObjectDesignerConfigSchema nested default overriding includeSystem back to false (line 550) - Fix isSystemObject() to match FQN format (sys__user) and legacy format (sys_user) - Fix system namespace prefix display to use namespace-based format - Add aria-label and type="button" on filter toggle for accessibility - Fix Collapsible onOpenChange to handle open/close explicitly instead of blind toggle - Add regression test for ObjectDesignerConfigSchema.objectManager.defaultFilter.includeSystem Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent ba7c4c6 commit eee9dae

3 files changed

Lines changed: 23 additions & 7 deletions

File tree

apps/studio/src/components/app-sidebar.tsx

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,13 +130,22 @@ const PROTOCOL_GROUPS: ProtocolGroup[] = [
130130
/** Types that are internal / should be hidden from the sidebar */
131131
const HIDDEN_TYPES = new Set(['plugin', 'plugins', 'kind', 'app', 'apps', 'package']);
132132

133-
/** System object name prefix — objects with this prefix are grouped under "System" */
134-
const SYSTEM_OBJECT_PREFIX = 'sys_';
133+
/** System namespace used for FQN-based names (e.g., sys__user) */
134+
const SYSTEM_NAMESPACE = 'sys';
135+
136+
/** System object FQN prefix (namespace + double underscore separator) */
137+
const SYSTEM_FQN_PREFIX = `${SYSTEM_NAMESPACE}__`;
138+
139+
/** Legacy system object name prefix (namespace + single underscore) */
140+
const SYSTEM_LEGACY_PREFIX = `${SYSTEM_NAMESPACE}_`;
135141

136142
/** Check if an object item is a system object */
137143
function isSystemObject(item: any): boolean {
144+
if (item.isSystem === true) return true;
145+
if (item.namespace === SYSTEM_NAMESPACE) return true;
138146
const name = item.name || item.id || '';
139-
return item.isSystem === true || name.startsWith(SYSTEM_OBJECT_PREFIX);
147+
// Match FQN format (sys__user) or legacy format (sys_user)
148+
return name.startsWith(SYSTEM_FQN_PREFIX) || name.startsWith(SYSTEM_LEGACY_PREFIX);
140149
}
141150

142151
/** Icon mapping for package types */
@@ -368,7 +377,9 @@ export function AppSidebar({
368377
{/* System objects filter toggle for Data group */}
369378
{group.key === 'data' && systemObjects.length > 0 && (
370379
<button
380+
type="button"
371381
title={showSystemInData ? 'Hide system objects' : 'Show system objects'}
382+
aria-label={showSystemInData ? 'Hide system objects' : 'Show system objects'}
372383
onClick={(e) => { e.stopPropagation(); setShowSystemInData(!showSystemInData); }}
373384
className="ml-1 shrink-0 rounded p-0.5 text-sidebar-foreground/50 hover:text-sidebar-foreground hover:bg-sidebar-accent transition-colors"
374385
>
@@ -470,7 +481,11 @@ export function AppSidebar({
470481
{systemObjects.length > 0 && (
471482
<Collapsible
472483
open={expandedTypes.has('_system_objects') || !!searchQuery}
473-
onOpenChange={() => toggleTypeExpanded('_system_objects')}
484+
onOpenChange={(open) => {
485+
const isExpanded = expandedTypes.has('_system_objects');
486+
if (open && !isExpanded) toggleTypeExpanded('_system_objects');
487+
if (!open && isExpanded) toggleTypeExpanded('_system_objects');
488+
}}
474489
asChild
475490
>
476491
<SidebarMenuItem>
@@ -497,8 +512,8 @@ export function AppSidebar({
497512
onClick={() => onSelectObject(itemName)}
498513
>
499514
<span className="truncate">
500-
{itemName.startsWith(SYSTEM_OBJECT_PREFIX) && (
501-
<span className="text-muted-foreground font-mono text-xs">{SYSTEM_OBJECT_PREFIX}</span>
515+
{(itemName.startsWith(SYSTEM_FQN_PREFIX) || itemName.startsWith(SYSTEM_LEGACY_PREFIX)) && (
516+
<span className="text-muted-foreground font-mono text-xs">{SYSTEM_NAMESPACE}:</span>
502517
)}
503518
{itemLabel}
504519
</span>

packages/spec/src/studio/object-designer.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,7 @@ describe('ObjectDesignerConfigSchema', () => {
448448
expect(result.erDiagram.enabled).toBe(true);
449449
expect(result.objectManager).toBeDefined();
450450
expect(result.objectManager.defaultDisplayMode).toBe('table');
451+
expect(result.objectManager.defaultFilter.includeSystem).toBe(true);
451452
expect(result.objectPreview).toBeDefined();
452453
expect(result.objectPreview.tabs.length).toBe(8);
453454
});

packages/spec/src/studio/object-designer.zod.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,7 @@ export const ObjectDesignerConfigSchema = z.object({
547547
defaultSortField: 'label',
548548
defaultSortDirection: 'asc',
549549
defaultFilter: {
550-
includeSystem: false,
550+
includeSystem: true,
551551
includeAbstract: false,
552552
},
553553
showFieldCount: true,

0 commit comments

Comments
 (0)