Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
… cancel confirmation and save draft Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
… ROADMAP 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
This PR integrates @object-ui/i18n into @object-ui/plugin-designer so AppCreationWizard and NavigationDesigner can render localized UI text (with an English fallback when no I18nProvider is present), and adds UX support for cancel confirmation and draft saving in the app creation flow.
Changes:
- Add
useDesignerTranslationhook and wiret()through AppCreationWizard + NavigationDesigner. - Add cancel confirmation dialog (using
useConfirmDialog) and newonSaveDraftcallback to AppCreationWizard. - Extend i18n locale packs (
appDesignerkeys) and update/expand unit tests and roadmap notes.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds workspace link for @object-ui/i18n dependency. |
| packages/plugin-designer/package.json | Declares @object-ui/i18n dependency for plugin-designer. |
| packages/plugin-designer/src/hooks/useDesignerTranslation.ts | Adds safe translation hook with English fallback defaults for designer UIs. |
| packages/plugin-designer/src/hooks/index.ts | Re-exports useDesignerTranslation. |
| packages/plugin-designer/src/AppCreationWizard.tsx | Replaces UI strings with t(), adds cancel confirm dialog + onSaveDraft. |
| packages/plugin-designer/src/NavigationDesigner.tsx | Replaces UI strings with t(), converts type/quick-add labels to translation keys. |
| packages/plugin-designer/src/tests/AppCreationWizard.test.tsx | Adds coverage for cancel confirm, save draft, and i18n fallback rendering. |
| packages/plugin-designer/src/tests/NavigationDesigner.test.tsx | Adds coverage for i18n fallback text in NavigationDesigner. |
| packages/i18n/src/locales/en.ts | Adds new appDesigner translation keys (English). |
| packages/i18n/src/locales/zh.ts | Adds new appDesigner translation keys (Chinese). |
| packages/i18n/src/locales/ja.ts | Adds new appDesigner translation keys (Japanese). |
| packages/i18n/src/locales/ko.ts | Adds new appDesigner translation keys (Korean). |
| packages/i18n/src/locales/de.ts | Adds new appDesigner translation keys (German). |
| packages/i18n/src/locales/fr.ts | Adds new appDesigner translation keys (French). |
| packages/i18n/src/locales/es.ts | Adds new appDesigner translation keys (Spanish). |
| packages/i18n/src/locales/ar.ts | Adds new appDesigner translation keys (Arabic). |
| packages/i18n/src/locales/ru.ts | Adds new appDesigner translation keys (Russian). |
| packages/i18n/src/locales/pt.ts | Adds new appDesigner translation keys (Portuguese). |
| ROADMAP.md | Updates roadmap checklist/status to reflect i18n + UX enhancements + test counts. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| if (confirmed) onCancel(); | ||
| } else { | ||
| onCancel?.(); | ||
| } | ||
| }, [draft.name, draft.title, onCancel, confirmDialog, t]); |
There was a problem hiding this comment.
handleCancel depends on the entire confirmDialog object, which is recreated every render (the hook returns a new object literal). This makes the useCallback ineffective and can cause unnecessary re-renders downstream. Prefer destructuring stable members (e.g. confirmDialog.confirm) and depending on those, or drop useCallback if memoization isn’t needed.
| (parentId: string, type: NavigationItemType) => { | ||
| const newItem: NavigationItem = { | ||
| id: createId(type), | ||
| type, | ||
| label: type === 'separator' ? '' : `New ${NAV_TYPE_META[type].label}`, | ||
| label: type === 'separator' ? '' : `New ${t(NAV_TYPE_META[type].labelKey)}`, |
There was a problem hiding this comment.
addChild uses t(...) but the useCallback dependency list doesn’t include t, so it can capture a stale translation function (and typically triggers hooks-lint warnings). Add t to the dependency array (or avoid useCallback here).
| id: createId(type), | ||
| type, | ||
| label: type === 'separator' ? '' : `New ${NAV_TYPE_META[type].label}`, | ||
| label: type === 'separator' ? '' : `New ${t(NAV_TYPE_META[type].labelKey)}`, |
There was a problem hiding this comment.
The default label is built with a hardcoded English prefix (New ${...}), so non-English locales will still display “New …”. Consider using a dedicated translation key (optionally with interpolation) instead of string concatenation.
| label: type === 'separator' ? '' : `New ${t(NAV_TYPE_META[type].labelKey)}`, | |
| label: | |
| type === 'separator' | |
| ? '' | |
| : t('navigationDesigner.newItemLabel', { | |
| label: t(NAV_TYPE_META[type].labelKey), | |
| }), |
| id: createId(type), | ||
| type, | ||
| label: type === 'separator' ? '' : `New ${NAV_TYPE_META[type].label}`, | ||
| label: type === 'separator' ? '' : `New ${t(NAV_TYPE_META[type].labelKey)}`, |
There was a problem hiding this comment.
This path also hardcodes the English New ${...} prefix, which won’t localize correctly. Use a dedicated translation key (optionally with interpolation) rather than concatenating an English prefix.
| label: type === 'separator' ? '' : `New ${t(NAV_TYPE_META[type].labelKey)}`, | |
| label: | |
| type === 'separator' | |
| ? '' | |
| : t('navigationDesigner.newItemLabel', { | |
| item: t(NAV_TYPE_META[type].labelKey), | |
| }), |
| @@ -290,9 +288,9 @@ function BasicInfoStep({ draft, templates, readOnly, onChange }: BasicInfoStepPr | |||
| className="block w-full rounded-md border border-gray-300 px-3 py-2 text-sm shadow-sm outline-none transition-colors focus:border-blue-500 focus:ring-1 focus:ring-blue-500 disabled:cursor-not-allowed disabled:bg-gray-50" | |||
| > | |||
| <option value="">None</option> | |||
There was a problem hiding this comment.
The template select’s empty option label is still hardcoded as “None”. If the wizard is intended to be fully i18n-integrated, this should come from t(...) (you may need to add a shared key like common.none or an appDesigner.none).
| <option value="">None</option> | |
| <option value="">{t('common.none')}</option> |
| </div> | ||
| )} | ||
| <span className="text-sm font-semibold text-gray-800">{title || 'App Title'}</span> | ||
| <span className="text-sm font-semibold text-gray-800">{title || t('appDesigner.appTitle')}</span> |
There was a problem hiding this comment.
In the branding preview, the fallback title uses t('appDesigner.appTitle') (which translates to “Title”), so the preview will show “Title” instead of a meaningful placeholder like “App Title”. Consider adding a separate placeholder key (e.g. appDesigner.appTitlePlaceholder) and using that here.
| <span className="text-sm font-semibold text-gray-800">{title || t('appDesigner.appTitle')}</span> | |
| <span className="text-sm font-semibold text-gray-800"> | |
| {title || t('appDesigner.appTitlePlaceholder')} | |
| </span> |
| @@ -661,7 +664,7 @@ function BrandingStep({ branding, title, readOnly, onChange }: BrandingStepProps | |||
| Logo | |||
There was a problem hiding this comment.
The branding preview still hardcodes the “Logo” placeholder text. If the wizard is intended to be fully i18n-integrated, this should be translated via t(...).
| Logo | |
| {t('appDesigner.logoPlaceholder')} |
| data-testid="cancel-confirm-dialog" | ||
| className="fixed inset-0 z-50 flex items-center justify-center bg-black/40" | ||
| role="dialog" | ||
| aria-modal="true" | ||
| > |
There was a problem hiding this comment.
The confirm modal uses role="dialog"/aria-modal="true" but doesn’t provide an accessible name/description via aria-labelledby/aria-describedby. Add IDs for the title/message and wire them up (or add an aria-label) so screen readers announce the dialog correctly.
AppCreationWizard and NavigationDesigner hardcoded English strings despite i18n keys existing in all 10 locale files. Also missing: cancel confirmation on unsaved changes and save-as-draft support.
i18n Integration
@object-ui/i18ndependency to@object-ui/plugin-designeruseDesignerTranslationsafe wrapper hook (falls back to English defaults when noI18nProvider— same pattern asuseListViewTranslationin plugin-list)AppCreationWizard.tsxandNavigationDesigner.tsxwitht()callsappDesignersection across all 10 locale files (en, zh, ja, ko, de, fr, es, ar, ru, pt)Cancel Confirmation Dialog
useConfirmDialoghook — renders inline modal with Discard/Keep Editing actionsNavigationDesigner i18n
NAV_TYPE_METAandQUICK_ADD_TYPESswitched from hardcodedlabeltolabelKeyreferences resolved viat()tthreaded through recursiveNavItemRowfor type badges, aria-labels, and empty statesTests
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.