-
Couldn't load subscription status.
- Fork 11
Fix JsonForms i18n integration and add coverage #1762
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughA new i18n helper with translate and translateError functions is introduced, with reactive message synchronization via watchers. Vue components are refactored to consolidate JsonForms props into single bound objects. Comprehensive test coverage validates locale-switching and live message updates. Changes
Sequence DiagramsequenceDiagram
participant Component
participant I18nHelper as useJsonFormsI18n
participant Vue_I18n
Note over Component,Vue_I18n: Initialization
Component->>I18nHelper: Call useJsonFormsI18n()
I18nHelper->>Vue_I18n: useI18n() to get locale, messages
I18nHelper->>I18nHelper: Create ref<JsonFormsI18nState>
I18nHelper->>I18nHelper: Set up watch(locale)
I18nHelper->>I18nHelper: Set up watch(messages[locale])
I18nHelper-->>Component: Return reactive state ref
Note over Component,Vue_I18n: Locale Change
Vue_I18n->>Vue_I18n: Update locale to French
I18nHelper->>I18nHelper: Locale watcher triggers
I18nHelper->>I18nHelper: Update state with new locale
I18nHelper->>I18nHelper: Regenerate translate & translateError
Component->>Component: Re-render with new translations
Note over Component,Vue_I18n: Runtime Message Update
Component->>Vue_I18n: mergeLocaleMessage(locale, newMessages)
Vue_I18n->>Vue_I18n: Update messages[locale]
I18nHelper->>I18nHelper: Messages watcher triggers (deep)
I18nHelper->>I18nHelper: Regenerate translate & translateError
Component->>Component: Re-render with updated messages
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The refactoring introduces new translation logic with fallback handling and reactive watchers in the helper, requiring careful verification of message prioritization and synchronization behavior. Component changes are repetitive and straightforward. Test coverage validates the i18n behavior across locale switching and live updates. Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| i18n.global.mergeLocaleMessage('en', { | ||
| jsonforms: { | ||
| test: { | ||
| provider: { | ||
| label: 'Provider label (updated)', | ||
| description: 'Updated English description', | ||
| }, | ||
| }, | ||
| }, | ||
| errors: { | ||
| required: 'Field is required (updated)', | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use correct path when updating error translation messages
The new i18n test updates error strings via mergeLocaleMessage under a top‑level errors key, but translateError looks for messages under jsonforms.errors.*. As written, the merged value is never read, so the assertion that 'Field is required (updated)' appears will always time out and the spec cannot pass. Move the errors object inside jsonforms (mirroring the initial message structure) or adjust the code under test to read from the root key.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (1)
web/src/helpers/jsonforms-i18n.ts (1)
47-69: Locale and message watchers ensure reactive translation updates.The watchers correctly force state updates when locale or messages change, ensuring JsonForms re-renders with new translations. Note that when locale changes, both watchers trigger (first from locale change, second from the watched messages object changing from
messages['en']tomessages['fr']), causing two state updates. This redundancy is harmless but could be optimized if needed.Consider combining the watchers to eliminate the double update on locale change:
- watch( - locale, - (currentLocale) => { - state.value = { - locale: currentLocale, - translate, - translateError, - }; - }, - { immediate: true } - ); - - watch( - () => messages.value?.[locale.value], - () => { - state.value = { - locale: locale.value, - translate, - translateError, - }; - }, - { deep: true } - ); + watch( + [locale, () => messages.value?.[locale.value]], + ([currentLocale]) => { + state.value = { + locale: currentLocale, + translate, + translateError, + }; + }, + { immediate: true, deep: true } + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
web/__test__/helpers/jsonforms-i18n.test.ts(1 hunks)web/src/components/ApiKey/ApiKeyCreate.vue(2 hunks)web/src/components/ConnectSettings/ConnectSettings.standalone.vue(2 hunks)web/src/helpers/jsonforms-i18n.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/components/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
Some Vue files may require explicit imports like ref or computed due to Nuxt auto-imports not applying in tests
Files:
web/src/components/ConnectSettings/ConnectSettings.standalone.vueweb/src/components/ApiKey/ApiKeyCreate.vue
{**/*.test.ts,**/__test__/{components,store}/**/*.ts}
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
{**/*.test.ts,**/__test__/{components,store}/**/*.ts}: Use .rejects.toThrow() without arguments when asserting that async functions throw; avoid checking exact error message strings unless the message format is explicitly under test
Focus tests on observable behavior and outcomes, not implementation details such as exact error messages
Use await nextTick() for DOM update assertions and flushPromises() for complex async chains; always await async operations before asserting
Place module mock declarations (vi.mock) at the top level of the test file to avoid hoisting issues
Use factory functions in vi.mock calls to define mocks and avoid hoisting pitfalls
Use vi.spyOn() to specify return values or behavior of methods under test
Reset/clear mocks between tests using vi.clearAllMocks() (and vi.resetAllMocks() when appropriate) to ensure isolation
Do not rely on Nuxt auto-imports in tests; import required Vue utilities explicitly in test files
Remember that vi.mock calls are hoisted; avoid mixing mock declarations and module mocks incorrectly
Files:
web/__test__/helpers/jsonforms-i18n.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript import specifiers with .js extensions for ESM compatibility
Never use the any type; prefer precise typing
Avoid type casting; model proper types from the start
Files:
web/__test__/helpers/jsonforms-i18n.test.tsweb/src/helpers/jsonforms-i18n.ts
{api,web}/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{api,web}/**/*.{test,spec}.{ts,tsx}: For error tests, use .rejects.toThrow() without arguments; avoid asserting exact error messages unless that format is the subject
Focus tests on behavior, not implementation details
Avoid brittle tests tied to exact error or log wording
Use mocks as nouns, not verbs
Always await async operations before making assertions
Place all mock declarations at the top level; use factory functions for module mocks; clear mocks between tests
Files:
web/__test__/helpers/jsonforms-i18n.test.ts
web/__test__/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place Vue component tests under web/test; run with pnpm test
Files:
web/__test__/helpers/jsonforms-i18n.test.ts
web/__test__/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/__test__/**/*.{test,spec}.{ts,tsx}: Use mount from Vue Test Utils for component testing
Stub complex child components that aren’t the focus of the test
Mock external dependencies and services in component tests
Test component behavior and output, not implementation details
Use createTestingPinia() for mocking stores in components
Prefer semantic queries like find('button') over data-test IDs
Use await nextTick() for DOM updates before assertions
For store tests, use createPinia() and setActivePinia
Only use createTestingPinia when its special features are needed
Let stores initialize with natural default state; don’t mock the store under test
Files:
web/__test__/helpers/jsonforms-i18n.test.ts
🧠 Learnings (1)
📚 Learning: 2025-03-12T13:35:43.900Z
Learnt from: pujitm
PR: unraid/api#1211
File: web/codegen.ts:14-14
Timestamp: 2025-03-12T13:35:43.900Z
Learning: The JSON scalar type in web/codegen.ts was temporarily changed from 'string' to 'any' for compatibility with JsonForms integration. This change facilitates the implementation of the Connect settings web component.
Applied to files:
web/src/components/ConnectSettings/ConnectSettings.standalone.vue
🧬 Code graph analysis (1)
web/__test__/helpers/jsonforms-i18n.test.ts (1)
web/src/helpers/jsonforms-i18n.ts (1)
useJsonFormsI18n(16-72)
🪛 GitHub Actions: CI - Main (API)
web/__test__/helpers/jsonforms-i18n.test.ts
[error] 1-1: Testing failure in jsonforms i18n tests: Unable to find an element with the text 'English description'. This indicates translations rendering did not match expectations in the active locale.
🪛 GitHub Check: Test API
web/__test__/helpers/jsonforms-i18n.test.ts
[failure] 91-91: test/helpers/jsonforms-i18n.test.ts > useJsonFormsI18n > responds to updated translations for the active locale
TestingLibraryElementError: Found multiple elements with the text: Provider label (en)
Here are the matching elements:
Ignored nodes: comments, script, style
<label
class="label required"
for="#/properties/provider-input"
Provider label (en)
<span
class="asterisk"
*
Ignored nodes: comments, script, style
<label
class="label required"
for="#/properties/provider2-input"
Provider label (en)
<span
class="asterisk"
*
(If this is intentional, then use the *AllBy* variant of the query (like queryAllByText, getAllByText, or findAllByText)).
Ignored nodes: comments, script, style
<input
class="input"
id="#/properties/provider-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
<input
class="input"
id="#/properties/provider2-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
Ignored nodes: comments, script, style
<input
class="input"
id="#/properties/provider-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
<input
class="input"
id="#/properties/provider2-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
[failure] 101-101: test/helpers/jsonforms-i18n.test.ts > useJsonFormsI18n > translates labels, descriptions, and errors when the locale changes
TestingLibraryElementError: Unable to find an element with the text: English description. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.
Ignored nodes: comments, script, style
<input
class="input"
id="#/properties/provider-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
[failure] 91-91: test/helpers/jsonforms-i18n.test.ts > useJsonFormsI18n > responds to updated translations for the active locale
TestingLibraryElementError: Found multiple elements with the text: Provider label (en)
Here are the matching elements:
Ignored nodes: comments, script, style
<label
class="label required"
for="#/properties/provider-input"
Provider label (en)
<span
class="asterisk"
*
Ignored nodes: comments, script, style
<label
class="label required"
for="#/properties/provider2-input"
Provider label (en)
<span
class="asterisk"
*
(If this is intentional, then use the *AllBy* variant of the query (like queryAllByText, getAllByText, or findAllByText)).
Ignored nodes: comments, script, style
<input
class="input"
id="#/properties/provider-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
<input
class="input"
id="#/properties/provider2-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
Ignored nodes: comments, script, style
<input
class="input"
id="#/properties/provider-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
<input
class="input"
id="#/properties/provider2-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
[failure] 101-101: test/helpers/jsonforms-i18n.test.ts > useJsonFormsI18n > translates labels, descriptions, and errors when the locale changes
TestingLibraryElementError: Unable to find an element with the text: English description. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.
Ignored nodes: comments, script, style
<input
class="input"
id="#/properties/provider-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
🔇 Additional comments (5)
web/src/helpers/jsonforms-i18n.ts (3)
6-14: LGTM: Type coercion utility is sound.The
toStringIfNeededhelper correctly handles strings, null/undefined, and other types, ensuring translation results are always strings as expected by JsonForms.
19-30: LGTM: Translation function with proper fallback chain.The
translatefunction correctly implements a fallback strategy: use translated value if available, otherwisedefaultMessage, thenid. The use oftoStringIfNeededensures consistent string output.
32-39: LGTM: Error translation follows JsonForms conventions.The
translateErrorfunction correctly constructs i18n keys from error keywords and provides sensible fallbacks.web/src/components/ConnectSettings/ConnectSettings.standalone.vue (1)
132-132: LGTM: v-bind spreads consolidated props.Using
v-bind="jsonFormsProps"cleanly applies all JsonForms configuration without repetitive individual bindings.web/src/components/ApiKey/ApiKeyCreate.vue (1)
481-481: LGTM: Props spread with v-bind.Clean application of consolidated JsonForms props.
| const schema = { | ||
| type: 'object', | ||
| properties: { | ||
| provider: { | ||
| type: 'string', | ||
| minLength: 1, | ||
| i18n: 'jsonforms.test.provider', | ||
| }, | ||
| }, | ||
| required: ['provider'], | ||
| } as const; | ||
|
|
||
| const uischema = { | ||
| type: 'Control', | ||
| scope: '#/properties/provider', | ||
| } as const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Schema description not rendering in tests.
The test expects to find "English description" (line 101), but static analysis shows it's not rendered. The vanilla renderers may not display schema descriptions by default. Consider adding uischema options to enable description display:
const uischema = {
type: 'Control',
scope: '#/properties/provider',
+ options: {
+ showDescription: true,
+ },
} as const;Alternatively, verify that the vanilla renderers support description rendering and whether additional configuration is needed.
🤖 Prompt for AI Agents
In web/__test__/helpers/jsonforms-i18n.test.ts around lines 11 to 26, the test
expects the string "English description" but the current schema and uischema
don't render schema descriptions by default; update the provider property's
schema to include a description field with the expected text and add uischema
options to enable description rendering (e.g., add an options object that turns
on description display supported by your vanilla renderers), or alternatively
adjust the test to use a renderer/config that shows schema descriptions if your
current renderers cannot; ensure the final setup actually renders the
description string the test asserts.
| const renderJsonForms = async (locale: 'en' | 'fr' = 'en') => { | ||
| const i18n = createI18n({ | ||
| legacy: false, | ||
| locale, | ||
| messages, | ||
| }); | ||
|
|
||
| const utils = render(TestHarness, { | ||
| global: { | ||
| plugins: [i18n], | ||
| }, | ||
| }); | ||
|
|
||
| await screen.findByText('Provider label (en)'); | ||
|
|
||
| return { i18n, ...utils }; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add cleanup between tests to prevent duplicate DOM elements.
Static analysis shows "Found multiple elements with the text: Provider label (en)" at line 91, indicating previous test renders aren't being cleaned up. Add cleanup to prevent test pollution:
-import { describe, expect, it } from 'vitest';
+import { afterEach, describe, expect, it } from 'vitest';
+import { cleanup } from '@testing-library/vue';
// ... schemas and component ...
describe('useJsonFormsI18n', () => {
+ afterEach(() => {
+ cleanup();
+ });
+
it('translates labels, descriptions, and errors when the locale changes', async () => {As per coding guidelines.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: Test API
[failure] 91-91: test/helpers/jsonforms-i18n.test.ts > useJsonFormsI18n > responds to updated translations for the active locale
TestingLibraryElementError: Found multiple elements with the text: Provider label (en)
Here are the matching elements:
Ignored nodes: comments, script, style
<label
class="label required"
for="#/properties/provider-input"
Provider label (en)
<span
class="asterisk"
*
Ignored nodes: comments, script, style
<label
class="label required"
for="#/properties/provider2-input"
Provider label (en)
<span
class="asterisk"
*
(If this is intentional, then use the *AllBy* variant of the query (like queryAllByText, getAllByText, or findAllByText)).
Ignored nodes: comments, script, style
<input
class="input"
id="#/properties/provider-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
<input
class="input"
id="#/properties/provider2-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
Ignored nodes: comments, script, style
<input
class="input"
id="#/properties/provider-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
<input
class="input"
id="#/properties/provider2-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
[failure] 91-91: test/helpers/jsonforms-i18n.test.ts > useJsonFormsI18n > responds to updated translations for the active locale
TestingLibraryElementError: Found multiple elements with the text: Provider label (en)
Here are the matching elements:
Ignored nodes: comments, script, style
<label
class="label required"
for="#/properties/provider-input"
Provider label (en)
<span
class="asterisk"
*
Ignored nodes: comments, script, style
<label
class="label required"
for="#/properties/provider2-input"
Provider label (en)
<span
class="asterisk"
*
(If this is intentional, then use the *AllBy* variant of the query (like queryAllByText, getAllByText, or findAllByText)).
Ignored nodes: comments, script, style
<input
class="input"
id="#/properties/provider-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
<input
class="input"
id="#/properties/provider2-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
Ignored nodes: comments, script, style
<input
class="input"
id="#/properties/provider-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
<input
class="input"
id="#/properties/provider2-input"
/>
</div>
<div
class="error"
>
jsonforms.test.provider.error.custom
</div>
</div>
🤖 Prompt for AI Agents
In web/__test__/helpers/jsonforms-i18n.test.ts around lines 78 to 94, tests are
leaving rendered DOM between runs causing "Found multiple elements with the
text: Provider label (en)"; add test cleanup to unmount/remove previous renders
by importing the testing-library cleanup helper and invoking it after each test
(e.g., add an afterEach(cleanup) hook) or call cleanup() in a beforeEach to
ensure a fresh DOM before renderJsonForms is used.
| it('responds to updated translations for the active locale', async () => { | ||
| const { i18n } = await renderJsonForms('en'); | ||
|
|
||
| i18n.global.mergeLocaleMessage('en', { | ||
| jsonforms: { | ||
| test: { | ||
| provider: { | ||
| label: 'Provider label (updated)', | ||
| description: 'Updated English description', | ||
| }, | ||
| }, | ||
| }, | ||
| errors: { | ||
| required: 'Field is required (updated)', | ||
| }, | ||
| }); | ||
|
|
||
| await nextTick(); | ||
| await screen.findByText('Provider label (updated)'); | ||
|
|
||
| expect(screen.getByText('Updated English description')).toBeTruthy(); | ||
| expect(screen.getByText('Field is required (updated)')).toBeTruthy(); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incorrect translation key for error messages.
Line 124-126 updates errors.required, but the translateError function in the helper (line 33 of jsonforms-i18n.ts) constructs keys as jsonforms.errors.{keyword}. The error translation will not work as expected.
Apply this diff:
i18n.global.mergeLocaleMessage('en', {
jsonforms: {
test: {
provider: {
label: 'Provider label (updated)',
description: 'Updated English description',
},
},
+ errors: {
+ required: 'Field is required (updated)',
+ },
},
- errors: {
- required: 'Field is required (updated)',
- },
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('responds to updated translations for the active locale', async () => { | |
| const { i18n } = await renderJsonForms('en'); | |
| i18n.global.mergeLocaleMessage('en', { | |
| jsonforms: { | |
| test: { | |
| provider: { | |
| label: 'Provider label (updated)', | |
| description: 'Updated English description', | |
| }, | |
| }, | |
| }, | |
| errors: { | |
| required: 'Field is required (updated)', | |
| }, | |
| }); | |
| await nextTick(); | |
| await screen.findByText('Provider label (updated)'); | |
| expect(screen.getByText('Updated English description')).toBeTruthy(); | |
| expect(screen.getByText('Field is required (updated)')).toBeTruthy(); | |
| }); | |
| it('responds to updated translations for the active locale', async () => { | |
| const { i18n } = await renderJsonForms('en'); | |
| i18n.global.mergeLocaleMessage('en', { | |
| jsonforms: { | |
| test: { | |
| provider: { | |
| label: 'Provider label (updated)', | |
| description: 'Updated English description', | |
| }, | |
| }, | |
| errors: { | |
| required: 'Field is required (updated)', | |
| }, | |
| }, | |
| }); | |
| await nextTick(); | |
| await screen.findByText('Provider label (updated)'); | |
| expect(screen.getByText('Updated English description')).toBeTruthy(); | |
| expect(screen.getByText('Field is required (updated)')).toBeTruthy(); | |
| }); |
🤖 Prompt for AI Agents
In web/__test__/helpers/jsonforms-i18n.test.ts around lines 112 to 134, the test
merges error translations under the top-level key "errors.required" but the
helper's translateError builds keys as "jsonforms.errors.{keyword}", so the
error translation isn't found; update the merged locale messages to place the
error translation under jsonforms.errors.required (i.e., nest the errors object
inside the jsonforms object as jsonforms: { errors: { required: 'Field is
required (updated)' } }) so the translateError lookup matches and the test
asserts the updated error text correctly.
| const jsonFormsConfig = { | ||
| restrict: false, | ||
| trim: false, | ||
| }; | ||
| const renderers = [...jsonFormsRenderers]; | ||
| const jsonFormsProps = computed(() => ({ | ||
| renderers, | ||
| config: jsonFormsConfig, | ||
| ajv: jsonFormsAjv, | ||
| i18n: jsonFormsI18n.value, | ||
| })); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
LGTM: Consistent JsonForms prop consolidation pattern.
The jsonFormsProps computed centralizes JsonForms configuration, matching the pattern in ConnectSettings.standalone.vue and promoting consistency across the codebase.
🤖 Prompt for AI Agents
In web/src/components/ApiKey/ApiKeyCreate.vue around lines 109 to 121, there is
no issue — the jsonFormsProps computed consolidates JsonForms configuration
consistently; leave this code as-is to match the pattern used in
ConnectSettings.standalone.vue and no change is required.
| const jsonFormsProps = computed(() => ({ | ||
| renderers, | ||
| config: jsonFormsConfig, | ||
| ajv: jsonFormsAjv, | ||
| i18n: jsonFormsI18n.value, | ||
| })); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
LGTM: Consolidating JsonForms props improves maintainability.
The computed jsonFormsProps object centralizes JsonForms configuration (renderers, config, ajv, i18n) into a single source of truth, simplifying prop management and aligning with the pattern used in ApiKeyCreate.vue.
🤖 Prompt for AI Agents
In web/src/components/ConnectSettings/ConnectSettings.standalone.vue around
lines 97 to 102, the new computed jsonFormsProps consolidates renderers, config,
ajv, and i18n into a single prop object; keep this refactor as-is, ensure the
component consumers are updated to use jsonFormsProps instead of individual
props, and mirror the same pattern used in ApiKeyCreate.vue for consistency.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68faf98cb39483238b1db61aa3e6106a
Summary by CodeRabbit
New Features
Refactor
Tests