Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
… CommandPalette, DashboardRenderer, AppManagementPage, and SpecCompliance test Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes console/plugin UI crashes caused by rendering I18nLabel objects ({ key, defaultValue }) directly as React children, by resolving these labels to strings before rendering and adjusting spec compliance tests accordingly.
Changes:
- Updated multiple Console UI surfaces (layout, sidebar, command palette, system app listing) to render
I18nLabelviaresolveI18nLabel()instead of raw objects. - Updated Dashboard plugin renderer to safely resolve widget/dashboard titles and descriptions before rendering.
- Relaxed spec compliance tests to accept
label/descriptionas either strings or I18nLabel objects.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/plugin-dashboard/src/DashboardRenderer.tsx | Adds a local label resolver and applies it to widget/schema titles/descriptions to prevent React child crashes. |
| apps/console/src/pages/system/AppManagementPage.tsx | Resolves app labels/descriptions when rendering the app management list. |
| apps/console/src/components/ConsoleLayout.tsx | Resolves active app label before passing to header/branding title. |
| apps/console/src/components/CommandPalette.tsx | Resolves labels for command items and search values to avoid rendering I18nLabel objects. |
| apps/console/src/components/AppSidebar.tsx | Resolves app switcher and condensed nav labels before rendering. |
| apps/console/src/tests/SpecCompliance.test.tsx | Updates compliance assertions to accept I18nLabel objects for label/description. |
| /** Resolve an I18nLabel (string or {key, defaultValue}) to a plain string. */ | ||
| function resolveLabel(label: string | { key?: string; defaultValue?: string } | undefined): string | undefined { | ||
| if (label === undefined || label === null) return undefined; | ||
| if (typeof label === 'string') return label; | ||
| return label.defaultValue || label.key; | ||
| } |
There was a problem hiding this comment.
In resolveLabel, the object shape ({ key?: string; defaultValue?: string }) doesn’t match the repo’s I18nLabel spec (key is required and params may be present). Consider typing this as string | I18nLabel | undefined (import I18nLabel from @object-ui/types) and keeping the resolver aligned with that shape so it won’t drift from the spec over time.
| expect(app.label).toBeDefined(); | ||
| expect(typeof app.label).toBe('string'); | ||
| expect(['string', 'object']).toContain(typeof app.label); | ||
| if (typeof app.label === 'object') { | ||
| expect(app.label).toHaveProperty('key'); | ||
| } |
There was a problem hiding this comment.
The updated label type check treats null as an object (typeof null === 'object'). Since toBeDefined() doesn’t exclude null, this test can behave unexpectedly. Consider explicitly asserting app.label/app.description are not null before the object-shape checks (and optionally that key is a string).
| <LayoutGrid className="h-4 w-4" /> | ||
| </div> | ||
| <div className="min-w-0 flex-1"> | ||
| <div className="flex items-center gap-2"> | ||
| <span className="font-medium truncate">{app.label || app.name}</span> | ||
| <span className="font-medium truncate">{resolveI18nLabel(app.label) || app.name}</span> |
There was a problem hiding this comment.
Within this app card, the checkbox aria-label is still built from app.label directly, which can stringify to "[object Object]" when label is an I18nLabel. Use resolveI18nLabel(app.label) (or fallback to app.name) for the aria-label so screen readers get a meaningful label.
CRM app metadata uses I18nLabel objects (
{key, defaultValue}) forlabelanddescriptionfields. Several components rendered these directly as React children, crashing withObjects are not valid as a React child.Component fixes — wrap labels with
resolveI18nLabel()ConsoleLayout.tsx—activeApp.labelpassed unresolved toAppHeaderappName prop and branding title interpolationAppSidebar.tsx— app switcher dropdown (line 304) and condensed nav item labels (line 533)CommandPalette.tsx— allitem.label/app.labelin<span>children andvaluestring propsDashboardRenderer.tsx—widget.title,schema.title,schema.descriptionrendered as children; added localresolveLabelhelper since@object-ui/reactdoesn't export the utilityAppManagementPage.tsx—app.labelandapp.descriptionin system page listingTest fix
SpecCompliance.test.tsx—typeof app.label/app.descriptionassertions updated to accept I18nLabel objects with shape validation:Full suite: 366 files, 6723 tests passing, 0 failures.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.