chore: Replace inline actions in config panel with controlled #17316
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughRenames incoming prop to initialComponent, introduces local currentComponent state and a JSON-based componentComparison utility, shifts per-property editing into edit-mode SelectPropertyEditor with Save/Cancel, and centralises tests via new testConfigUtils helpers for open/save/cancel flows. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ConfigEditor as Config{String|Number|Array}Properties
participant SelectEditor as SelectPropertyEditor
participant FieldEditor as Edit*Value
participant Handler as handleComponentUpdate
User->>ConfigEditor: Click property to open editor
ConfigEditor->>ConfigEditor: currentComponent = initialComponent
User->>SelectEditor: Enter edit mode
SelectEditor->>SelectEditor: set editMode = true
SelectEditor->>FieldEditor: Render FieldEditor(currentComponent, setCurrentComponent)
User->>FieldEditor: Modify value
FieldEditor->>ConfigEditor: setCurrentComponent(updated)
alt User saves
User->>SelectEditor: Click Save
SelectEditor->>ConfigEditor: onSave()
ConfigEditor->>Handler: handleComponentUpdate(currentComponent)
SelectEditor->>SelectEditor: set editMode = false
else User cancels
User->>SelectEditor: Click Cancel
SelectEditor->>ConfigEditor: onCancel()
ConfigEditor->>ConfigEditor: currentComponent = initialComponent
SelectEditor->>SelectEditor: set editMode = false
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts (1)
2-2: Remove unused import.The
FormItemtype is imported but never used in this test file.🔎 Proposed fix
import { componentMocks } from '@altinn/ux-editor/testing/componentMocks'; -import { FormItem } from '../../../types/FormItem'; import { componentComparison } from './ConfigPropertiesUtils';src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx (1)
87-100: Consider makingisLoadingconfigurable if save operations can be async.
isLoading={false}is hardcoded. If save operations in consumers become asynchronous in the future, you may want to expose this as a prop.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.tssrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.tssrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.module.csssrc/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
🧰 Additional context used
🧠 Learnings (21)
📓 Common learnings
Learnt from: framitdavid
Repo: Altinn/altinn-studio PR: 16372
File: src/Designer/frontend/libs/studio-components/src/components/index.ts:34-34
Timestamp: 2025-09-23T08:53:19.508Z
Learning: In the Altinn Studio codebase, when moving components from studio-components-legacy to studio-components, the team prefers to handle the component migration and remaining import updates in separate PRs to maintain focused, atomic changes.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16095
File: src/App/app-template-dotnet/src/App/models/model.cs:10-29
Timestamp: 2025-08-15T05:53:32.529Z
Learning: When moving app templates in Altinn Studio, mirkoSekulic prefers to keep the PR scope strictly limited to the move operation itself, with all content changes (including code quality improvements, naming convention fixes, and attribute cleanup) deferred to separate PRs for better maintainability and atomic changes.
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-10T12:02:06.348Z
Learning: In the Altinn Studio repository, the team convention is to remove irrelevant checkboxes from the PR template description rather than leaving them unmarked. This makes it clear that the author has intentionally considered each item and removed non-applicable ones, helping to avoid merging PRs with incomplete checklists.
Learnt from: mgunnerud
Repo: Altinn/altinn-studio PR: 16187
File: frontend/resourceadm/components/ResourcePageInputs/ResourceSwitchInput.tsx:3-3
Timestamp: 2025-08-25T13:55:27.042Z
Learning: In the Altinn Studio codebase, when migrating from studio/components-legacy to studio/components, PRs should focus only on components that have available replacements in the new package. Components without new implementations should remain in the legacy package until replacements are available, supporting an incremental migration approach.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 14575
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/OverrideFields/Summary2OverrideDisplayType.tsx:47-58
Timestamp: 2025-02-07T09:50:33.290Z
Learning: Avoid suggesting StudioCombobox component in future PRs for the Altinn Studio repository. A new replacement component will be introduced later.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 16123
File: frontend/libs/studio-components/src/types/OverridableComponentProps.ts:3-6
Timestamp: 2025-08-15T08:21:51.564Z
Learning: ErlingHauan prefers to keep PRs that move types from studio-components-legacy to studio-components focused strictly on the move operation, deferring type implementation changes or improvements to separate PRs for better maintainability and atomic changes.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:36:57.397Z
Learning: When a developer copies a component from `studio-components-legacy` to `studio-components` in the Altinn Studio repository and has not added a deprecation comment for the component in `studio-components-legacy`, suggest adding the deprecation comment in the PR review.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16191
File: .dockerignore:3-3
Timestamp: 2025-08-26T12:35:44.069Z
Learning: In the Altinn Studio repository, mirkoSekulic is implementing a phased directory restructuring approach where backend and testdata are moved first, followed by separate moves for readme, frontend, and development directories into the Designer directory. This ensures atomic changes and prevents path reference issues during the transition.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:37:01.381Z
Learning: When reviewing PRs in the Altinn Studio repository where a developer copies a component from `studio-components-legacy` to `studio-components`, suggest adding a `deprecated` JSDoc comment to the component in `studio-components-legacy` if one hasn't been added. The deprecation comment should recommend using the component from `studio/components` instead.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 16740
File: src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx:63-71
Timestamp: 2025-10-25T11:48:49.152Z
Learning: In src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx, the NameConfig component is intentionally rendered in two places: once in the mainConfig section and once inside the "Text" accordion section. This duplication is by design.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/PropertiesHeader.tsx:28-30
Timestamp: 2025-07-02T09:04:49.873Z
Learning: In the Altinn Studio UX editor, the PropertiesHeader component can safely access schema.properties directly without null checks because the parent ComponentConfigPanel component handles the loading state and only renders PropertiesHeader after the schema is fetched via useComponentSchemaQuery.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/PropertiesHeader.tsx:28-30
Timestamp: 2025-07-02T09:04:49.873Z
Learning: In the Altinn Studio UX editor, the PropertiesHeader component can safely access schema.properties directly without null checks because the parent ComponentConfigPanel component handles the loading state and only renders PropertiesHeader after the schema is fetched via useComponentSchemaQuery.
📚 Learning: 2025-10-25T11:48:49.152Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 16740
File: src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx:63-71
Timestamp: 2025-10-25T11:48:49.152Z
Learning: In src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx, the NameConfig component is intentionally rendered in two places: once in the mainConfig section and once inside the "Text" accordion section. This duplication is by design.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.tssrc/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.tssrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.tsx
📚 Learning: 2025-11-25T08:21:14.748Z
Learnt from: CR
Repo: Altinn/altinn-studio PR: 0
File: src/App/frontend/CLAUDE.md:0-0
Timestamp: 2025-11-25T08:21:14.748Z
Learning: Applies to src/App/frontend/src/layout/**/*.{ts,tsx} : Layout components must have standardized structure with `config.ts`, `Component.tsx`, `index.tsx`, and generated `config.generated.ts` files
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.tssrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.tsx
📚 Learning: 2025-10-26T21:09:38.402Z
Learnt from: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16742
File: src/Designer/frontend/packages/schema-editor/src/components/SchemaInspector/ItemDataComponent.module.css:23-25
Timestamp: 2025-10-26T21:09:38.402Z
Learning: In the Altinn Studio codebase, the design system is migrating from `--fds-*` prefixed CSS variables to `--ds-*` prefixed variables. The `--fds-*` prefix will be removed in the future, so usage of `--ds-*` variables (like `--ds-size-4`) alongside existing `--fds-*` variables (like `--fds-spacing-4`) in the same file is intentional during this transition period.
<!--
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.module.css
📚 Learning: 2025-11-25T08:21:14.748Z
Learnt from: CR
Repo: Altinn/altinn-studio PR: 0
File: src/App/frontend/CLAUDE.md:0-0
Timestamp: 2025-11-25T08:21:14.748Z
Learning: Applies to src/App/frontend/**/*.test.{ts,tsx} : Use `renderWithProviders` utility from `src/test/renderWithProviders.tsx` when testing components that require form layout context
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts
📚 Learning: 2025-10-28T20:54:18.494Z
Learnt from: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16787
File: src/Designer/frontend/packages/ux-editor-v3/src/components/toolbar/RuleModal.test.tsx:70-70
Timestamp: 2025-10-28T20:54:18.494Z
Learning: Test inconsistencies in the ux-editor-v3 package do not need to be fixed, as indicated by the maintainer for the file `src/Designer/frontend/packages/ux-editor-v3/src/components/toolbar/RuleModal.test.tsx`.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts
📚 Learning: 2025-07-15T06:14:47.860Z
Learnt from: wrt95
Repo: Altinn/altinn-studio PR: 15850
File: frontend/app-development/features/appSettings/components/TabsContent/Tabs/AboutTab/AppConfigForm/StatusRadioGroup/StatusRadioGroup.test.tsx:47-56
Timestamp: 2025-07-15T06:14:47.860Z
Learning: In StatusRadioGroup tests for Altinn Studio, the onChangeStatus callback is called with two parameters: the selected value and the previous value. This is why test assertions like `expect(onChangeStatus).toHaveBeenCalledWith('Completed', '');` use both parameters, even though the TypeScript interface only shows one parameter.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx
📚 Learning: 2025-06-16T12:41:06.726Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15659
File: frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/CodeListDialog/CodeListDialog.test.tsx:145-151
Timestamp: 2025-06-16T12:41:06.726Z
Learning: In the Altinn Studio codebase, TomasEng has confirmed that using `expect(screen.getByRole('dialog')).toBeVisible` directly with `waitFor` is acceptable and equivalent to wrapping it in a function callback. This pattern is intentionally used in their testing setup.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx
📚 Learning: 2025-11-03T08:16:33.782Z
Learnt from: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16865
File: src/Designer/frontend/app-preview/src/components/PreviewControlHeader/PreviewControlHeader.tsx:44-50
Timestamp: 2025-11-03T08:16:33.782Z
Learning: In PreviewControlHeader.tsx (app-preview package), JamalAlabdullah confirmed that the StudioSelect component for layout set selection is acceptable with an empty label prop (label={''}), indicating this is an intentional design decision for that specific component.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.tsx
📚 Learning: 2025-11-03T18:58:18.996Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 16876
File: src/Designer/frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListsPage/CodeListsPage.tsx:17-23
Timestamp: 2025-11-03T18:58:18.996Z
Learning: In src/Designer/frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListsPage/CodeListsPage.tsx, the CodeListsPage component is intentionally an uncontrolled component where the codeLists prop only initializes state on mount and does not synchronize when the prop changes. This is because the createCodeListMap function generates random UUIDs for keys, making it impure. Re-calling it after the first render would reassign keys and significantly disrupt the UI (unmounting/remounting components). The component manages its own state for draft/editing purposes, and the prop is not expected to change to anything that doesn't correspond to the internal state.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.tsx
📚 Learning: 2025-09-26T12:03:56.908Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-26T12:03:56.908Z
Learning: In the Altinn Studio codebase, when reviewing migration from studio/components-legacy to studio/components for the StudioExpression component, the ExpressionTexts type interface does not require additional keys like `dataModel`, `resource`, or `missingDataModelLabel`. TypeScript compilation will catch any missing required properties, so successful builds indicate proper type compliance.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.tsx
📚 Learning: 2025-07-02T09:02:34.292Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.tsx
📚 Learning: 2025-09-25T08:50:58.936Z
Learnt from: Jondyr
Repo: Altinn/altinn-studio PR: 16431
File: src/Designer/frontend/packages/ux-editor/src/converters/formLayoutConverters/externalLayoutToInternal.ts:28-33
Timestamp: 2025-09-25T08:50:58.936Z
Learning: The Altinn Studio codebase contains two parallel UX editor packages: `ux-editor` (current/newer) and `ux-editor-v3` (legacy). When reviewing code changes, need to distinguish between these versions as they have different API implementations, particularly around datamodel binding conversions.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-09-26T12:03:51.850Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-26T12:03:51.850Z
Learning: In PR #16443, TomasEng clarified that the ExpressionTexts type in studio/components does not require additional keys like dataModel, resource, or missingDataModelLabel that were suggested in a review comment. TypeScript compilation would catch any missing required properties, confirming the migration is correctly implemented.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.tsx
📚 Learning: 2025-07-03T14:54:51.389Z
Learnt from: mgunnerud
Repo: Altinn/altinn-studio PR: 15826
File: frontend/resourceadm/components/ResourcePageInputs/ResourceLanguageTextField.tsx:95-95
Timestamp: 2025-07-03T14:54:51.389Z
Learning: The StudioTextfield component at frontend/libs/studio-components/src/components/StudioTextfield/StudioTextfield.tsx has a typing limitation where it only accepts `Ref<HTMLInputElement>` even when rendering textarea elements (when multiline=true). This creates a type mismatch between the ref type and the actual element type, requiring workarounds like removing ref props when both input and textarea elements need to be supported.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-01-13T12:44:45.751Z
Learnt from: Jondyr
Repo: Altinn/altinn-studio PR: 14379
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/Summary2OverrideEntry.tsx:63-70
Timestamp: 2025-01-13T12:44:45.751Z
Learning: In the Altinn Studio codebase, when using StudioProperty.Button component, it's expected to pass `false` as the value prop when no meaningful value is available (e.g., when componentNameType is undefined). This is the intended behavior and should not be changed to handle undefined cases differently.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.tsx
📚 Learning: 2025-05-30T14:19:53.714Z
Learnt from: nkylstad
Repo: Altinn/altinn-studio PR: 15550
File: frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/SpecificMainConfig/ImageMainConfig.test.tsx:17-17
Timestamp: 2025-05-30T14:19:53.714Z
Learning: In the SpecificMainConfig test files, the standard pattern is to use 'ComponentMainConfig' as the top-level describe block name, with nested describe blocks named after the specific component type being tested (e.g., 'Image', 'Options', 'Summary2').
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts
📚 Learning: 2025-07-02T09:04:49.873Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/PropertiesHeader.tsx:28-30
Timestamp: 2025-07-02T09:04:49.873Z
Learning: In the Altinn Studio UX editor, the PropertiesHeader component can safely access schema.properties directly without null checks because the parent ComponentConfigPanel component handles the loading state and only renders PropertiesHeader after the schema is fetched via useComponentSchemaQuery.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.tsx
📚 Learning: 2025-02-10T11:46:29.320Z
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 14398
File: frontend/libs/studio-components/src/components/StudioCodelistEditor/StudioCodeListEditorRow/StudioCodeListEditorRow.tsx:0-0
Timestamp: 2025-02-10T11:46:29.320Z
Learning: The Numberfield component handles number validation internally and only calls onChange with valid number or undefined values, making additional validation in parent components unnecessary.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx
📚 Learning: 2025-06-16T12:19:07.682Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15495
File: frontend/app-development/layout/VersionDialog/VersionDialogTableRow.tsx:5-13
Timestamp: 2025-06-16T12:19:07.682Z
Learning: In the Altinn Studio codebase, the VersionDialog components work with major version numbers only (e.g., 4, 8) rather than full semantic versions. The latestVersion prop in VersionDialogTableRow should be typed as number since it represents maximum supported major version constants like MAXIMUM_SUPPORTED_FRONTEND_VERSION = 4.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx
📚 Learning: 2025-08-07T13:08:11.913Z
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15981
File: frontend/libs/studio-components/src/components/StudioLabelAsParagraph/StudioLabelAsParagraph.tsx:10-10
Timestamp: 2025-08-07T13:08:11.913Z
Learning: In the Altinn Studio codebase, when using components with the `asChild` prop pattern (like from digdir/designsystemet-react), the ref should be placed on the child DOM element rather than forwarded to the parent component. This pattern was established in PR #15998 where Card asChild was used with StudioFieldset, and the ref was passed to the StudioFieldset (child) not the Card (parent). This approach provides correct TypeScript typing automatically since the ref points directly to the actual DOM element being rendered.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx
🧬 Code graph analysis (6)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx (1)
src/Designer/development/setup.js (1)
waitFor(2-2)
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx (1)
src/Designer/frontend/libs/studio-components/src/components/StudioFormActions/index.ts (1)
StudioFormActions(1-1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts (2)
src/Designer/frontend/packages/ux-editor/src/testing/componentMocks.ts (1)
componentMocks(215-248)src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.ts (1)
componentComparison(8-13)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.tsx (3)
src/Designer/frontend/packages/ux-editor/src/hooks/index.ts (1)
useComponentPropertyLabel(9-9)src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx (1)
SelectPropertyEditor(18-53)src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.ts (1)
componentComparison(8-13)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx (3)
src/Designer/frontend/packages/ux-editor/src/hooks/index.ts (1)
useComponentPropertyLabel(9-9)src/Designer/frontend/packages/ux-editor/src/components/config/editModal/EditNumberValue.tsx (1)
EditNumberValue(26-83)src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx (1)
SelectPropertyEditor(18-53)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.tsx (2)
src/Designer/frontend/packages/ux-editor/src/hooks/index.ts (1)
useComponentPropertyLabel(9-9)src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.ts (1)
componentComparison(8-13)
🔇 Additional comments (8)
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.module.css (1)
1-13: LGTM!The CSS updates correctly use the new
--ds-*design system tokens and the structure aligns well with the component's new edit-mode UI usingStudioFormActions.src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.ts (1)
8-12: JSON.stringify comparison is acceptable for this use case, but be aware of property order sensitivity.The implementation is straightforward and works well for detecting changes in component objects. Note that
JSON.stringifyis sensitive to property ordering, so{a:1, b:2}and{b:2, a:1}would compare as different. This should be fine here since the objects are derived from the same source via spreading, preserving property order.src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx (2)
10-12: Good practice adding mock cleanup.Adding
jest.clearAllMocks()inbeforeEachensures proper test isolation between test cases.
128-133: Well-structured save helper with proper async handling.The
saveChangeshelper correctly waits for the save button to be enabled before clicking, which properly handles the asynchronous state update flow.src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx (2)
74-82: Correct order of operations in handlers.Setting
editMode(false)before calling the callback ensures the UI exits edit mode immediately, providing responsive feedback to the user before the potentially async save/cancel operations complete.
7-16: Breaking change:childrenis now required.The
childrenprop is now required (no?modifier). All consumers ofSelectPropertyEditorhave been updated to pass children. Verified in ConfigArrayProperties, ConfigStringProperties, and ConfigNumberProperties.src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.tsx (1)
16-76: LGTM!The controlled editing pattern is correctly implemented:
- Local
currentComponentstate properly tracks editsonSavecommits the current state,onCancelresets to initial- Child
EditStringValuecorrectly receivescurrentComponentand updates viasetCurrentComponent- Memoization dependencies are correct
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.tsx (1)
16-75: LGTM!The controlled editing flow is correctly implemented with consistent patterns across the config property editors. The
currentComponentstate properly tracks edits, and theSelectPropertyEditorintegration with save/cancel/disabled logic is sound.
...rontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx
Show resolved
Hide resolved
...tend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts
Outdated
Show resolved
Hide resolved
github-actions
bot
added
area/ui-editor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx (2)
54-54: Consider renaming the CSS class for clarity.The class name
viewModeis applied when rendering edit mode content (lines 54-70), which could be confusing. Consider renaming toeditModeoreditModeContainerfor better code clarity.🔎 Proposed class name update
- <div className={cn(classes.viewMode, className)}> + <div className={cn(classes.editMode, className)}>Don't forget to update the corresponding CSS module class name.
67-67: Consider whether async save operations might be needed.The
isLoadingprop is hardcoded tofalse. While parent components can disable the save button viaisSaveDisabledduring async operations, having a dedicated loading state could provide better UX with loading indicators.This is acceptable as-is if the parent's
isSaveDisabledpattern sufficiently handles async scenarios.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: framitdavid
Repo: Altinn/altinn-studio PR: 16372
File: src/Designer/frontend/libs/studio-components/src/components/index.ts:34-34
Timestamp: 2025-09-23T08:53:19.508Z
Learning: In the Altinn Studio codebase, when moving components from studio-components-legacy to studio-components, the team prefers to handle the component migration and remaining import updates in separate PRs to maintain focused, atomic changes.
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-10T12:02:06.348Z
Learning: In the Altinn Studio repository, the team convention is to remove irrelevant checkboxes from the PR template description rather than leaving them unmarked. This makes it clear that the author has intentionally considered each item and removed non-applicable ones, helping to avoid merging PRs with incomplete checklists.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16095
File: src/App/app-template-dotnet/src/App/models/model.cs:10-29
Timestamp: 2025-08-15T05:53:32.529Z
Learning: When moving app templates in Altinn Studio, mirkoSekulic prefers to keep the PR scope strictly limited to the move operation itself, with all content changes (including code quality improvements, naming convention fixes, and attribute cleanup) deferred to separate PRs for better maintainability and atomic changes.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 14575
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/OverrideFields/Summary2OverrideDisplayType.tsx:47-58
Timestamp: 2025-02-07T09:50:33.290Z
Learning: Avoid suggesting StudioCombobox component in future PRs for the Altinn Studio repository. A new replacement component will be introduced later.
Learnt from: mgunnerud
Repo: Altinn/altinn-studio PR: 16187
File: frontend/resourceadm/components/ResourcePageInputs/ResourceSwitchInput.tsx:3-3
Timestamp: 2025-08-25T13:55:27.042Z
Learning: In the Altinn Studio codebase, when migrating from studio/components-legacy to studio/components, PRs should focus only on components that have available replacements in the new package. Components without new implementations should remain in the legacy package until replacements are available, supporting an incremental migration approach.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:36:57.397Z
Learning: When a developer copies a component from `studio-components-legacy` to `studio-components` in the Altinn Studio repository and has not added a deprecation comment for the component in `studio-components-legacy`, suggest adding the deprecation comment in the PR review.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 16123
File: frontend/libs/studio-components/src/types/OverridableComponentProps.ts:3-6
Timestamp: 2025-08-15T08:21:51.564Z
Learning: ErlingHauan prefers to keep PRs that move types from studio-components-legacy to studio-components focused strictly on the move operation, deferring type implementation changes or improvements to separate PRs for better maintainability and atomic changes.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16191
File: .dockerignore:3-3
Timestamp: 2025-08-26T12:35:44.069Z
Learning: In the Altinn Studio repository, mirkoSekulic is implementing a phased directory restructuring approach where backend and testdata are moved first, followed by separate moves for readme, frontend, and development directories into the Designer directory. This ensures atomic changes and prevents path reference issues during the transition.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:37:01.381Z
Learning: When reviewing PRs in the Altinn Studio repository where a developer copies a component from `studio-components-legacy` to `studio-components`, suggest adding a `deprecated` JSDoc comment to the component in `studio-components-legacy` if one hasn't been added. The deprecation comment should recommend using the component from `studio/components` instead.
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 16064
File: frontend/libs/studio-components/src/components/StudioCheckboxGroup/StudioGetCheckboxProps.ts:1-1
Timestamp: 2025-08-12T05:59:20.146Z
Learning: In the Altinn Studio codebase, deep exports from digdir/designsystemet-react should be avoided. The team prefers using shared wrappers for design system imports to prevent breakage from upstream internal path changes, but such refactoring should be done in separate PRs to maintain focused scope.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 16740
File: src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx:63-71
Timestamp: 2025-10-25T11:48:49.152Z
Learning: In src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx, the NameConfig component is intentionally rendered in two places: once in the mainConfig section and once inside the "Text" accordion section. This duplication is by design.
📚 Learning: 2025-11-03T08:16:33.782Z
Learnt from: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16865
File: src/Designer/frontend/app-preview/src/components/PreviewControlHeader/PreviewControlHeader.tsx:44-50
Timestamp: 2025-11-03T08:16:33.782Z
Learning: In PreviewControlHeader.tsx (app-preview package), JamalAlabdullah confirmed that the StudioSelect component for layout set selection is acceptable with an empty label prop (label={''}), indicating this is an intentional design decision for that specific component.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-10-25T11:48:49.152Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 16740
File: src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx:63-71
Timestamp: 2025-10-25T11:48:49.152Z
Learning: In src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx, the NameConfig component is intentionally rendered in two places: once in the mainConfig section and once inside the "Text" accordion section. This duplication is by design.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-09-25T08:50:58.936Z
Learnt from: Jondyr
Repo: Altinn/altinn-studio PR: 16431
File: src/Designer/frontend/packages/ux-editor/src/converters/formLayoutConverters/externalLayoutToInternal.ts:28-33
Timestamp: 2025-09-25T08:50:58.936Z
Learning: The Altinn Studio codebase contains two parallel UX editor packages: `ux-editor` (current/newer) and `ux-editor-v3` (legacy). When reviewing code changes, need to distinguish between these versions as they have different API implementations, particularly around datamodel binding conversions.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-09-26T12:03:56.908Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-26T12:03:56.908Z
Learning: In the Altinn Studio codebase, when reviewing migration from studio/components-legacy to studio/components for the StudioExpression component, the ExpressionTexts type interface does not require additional keys like `dataModel`, `resource`, or `missingDataModelLabel`. TypeScript compilation will catch any missing required properties, so successful builds indicate proper type compliance.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-07-02T09:02:34.292Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-09-26T12:03:51.850Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-26T12:03:51.850Z
Learning: In PR #16443, TomasEng clarified that the ExpressionTexts type in studio/components does not require additional keys like dataModel, resource, or missingDataModelLabel that were suggested in a review comment. TypeScript compilation would catch any missing required properties, confirming the migration is correctly implemented.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-11-03T18:58:18.996Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 16876
File: src/Designer/frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListsPage/CodeListsPage.tsx:17-23
Timestamp: 2025-11-03T18:58:18.996Z
Learning: In src/Designer/frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListsPage/CodeListsPage.tsx, the CodeListsPage component is intentionally an uncontrolled component where the codeLists prop only initializes state on mount and does not synchronize when the prop changes. This is because the createCodeListMap function generates random UUIDs for keys, making it impure. Re-calling it after the first render would reassign keys and significantly disrupt the UI (unmounting/remounting components). The component manages its own state for draft/editing purposes, and the prop is not expected to change to anything that doesn't correspond to the internal state.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-06-16T12:16:47.892Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15659
File: frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/OptionListEditor.tsx:44-47
Timestamp: 2025-06-16T12:16:47.892Z
Learning: In selection components within the Altinn Studio UX editor, the code design anticipates that one of the properties `options` and `optionsId` should always be set, representing manual options vs external option list references respectively. However, this mutual exclusivity is not well reflected in the current type system.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-01-13T12:44:45.751Z
Learnt from: Jondyr
Repo: Altinn/altinn-studio PR: 14379
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/Summary2OverrideEntry.tsx:63-70
Timestamp: 2025-01-13T12:44:45.751Z
Learning: In the Altinn Studio codebase, when using StudioProperty.Button component, it's expected to pass `false` as the value prop when no meaningful value is available (e.g., when componentNameType is undefined). This is the intended behavior and should not be changed to handle undefined cases differently.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
🧬 Code graph analysis (1)
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx (1)
src/Designer/frontend/libs/studio-components/src/components/StudioFormActions/index.ts (1)
StudioFormActions(1-1)
🔇 Additional comments (5)
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx (5)
1-6: LGTM! Import changes align with the controlled editing pattern.The updated imports correctly reflect the new architecture using
StudioFormActionsfor action buttons andStudioProperty.Buttonfor the view mode.
28-41: LGTM! View mode correctly delegates to StudioProperty.Button.The component correctly initializes in view mode and provides a clear entry point to edit mode via the button click. The pattern ensures a consistent edit flow across property editors.
43-51: LGTM! Handlers correctly orchestrate mode transitions.Both
handleSaveandhandleCancelfollow a consistent pattern: exit edit mode, then invoke the parent callback. This ensures the component's UI state is updated before the parent handles data state changes.
56-69: No accessibility concerns—icon-only buttons already have proper ARIA labels and keyboard support.The
StudioFormActionscomponent provides appropriatearia-labelattributes for icon-only buttons and properly hides icons from screen readers witharia-hidden. Keyboard navigation and focus states are handled by the underlying@digdir/designsystemet-reactButton component.
7-16: All call sites provide the required props—verification complete.The prop interface changes have been correctly implemented across all call sites. Three components use
SelectPropertyEditor(ConfigNumberProperties, ConfigStringProperties, ConfigArrayProperties), and each provides:
children(EditNumberValue or EditStringValue components)onSaveandonCancelcallbacks with proper state managementisSaveDisabledcomputed viacomponentComparisonNo breaking change issues remain.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
...tend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts
Fixed
Show fixed
Hide fixed
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts (1)
14-18: Test correctly validates identical component comparison.The test passes the same component reference to both parameters, which correctly verifies that identical components return true. For more robust coverage, consider testing with two separate but structurally identical component objects to ensure deep equality comparison rather than reference equality.
🔎 Optional enhancement for deep equality verification
it('componentComparison should return true for identical components', () => { + const componentB = { ...componentMocks.Input }; expect( - componentComparison({ initialComponent: componentA, currentComponent: componentA }), + componentComparison({ initialComponent: componentA, currentComponent: componentB }), ).toBe(true); });src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx (1)
24-24: State initialisation does not synchronise with prop changes.If
initialComponentchanges from the parent whilst the component is mounted (e.g. external update or navigation),currentComponentwill remain stale. This is acceptable if by design the edited state is intentionally preserved until explicit save/cancel. However, consider adding auseEffectto resetcurrentComponentwheninitialComponentchanges, if external updates should take precedence.useEffect(() => { setCurrentComponent(initialComponent); }, [initialComponent]);If the current behaviour is intentional, a brief comment explaining this design choice would improve clarity.
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx (1)
56-69: Consider makingisLoadingconfigurable for async save operations.The hardcoded
isLoading={false}works for synchronous saves but doesn't support showing a loading indicator during async operations. If any callers perform async saves in the future, this would need updating.🔎 Optional enhancement
export type SelectPropertyEditorProps = { children: React.ReactNode; value?: string | React.ReactNode; property?: string; title?: string; className?: string; onSave: () => void; onCancel: () => void; isSaveDisabled: boolean; + isLoading?: boolean; };Then use
isLoading={isLoading ?? false}inStudioFormActions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.tssrc/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.module.csssrc/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.module.css
🧰 Additional context used
🧠 Learnings (22)
📓 Common learnings
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-10T12:02:06.348Z
Learning: In the Altinn Studio repository, the team convention is to remove irrelevant checkboxes from the PR template description rather than leaving them unmarked. This makes it clear that the author has intentionally considered each item and removed non-applicable ones, helping to avoid merging PRs with incomplete checklists.
Learnt from: framitdavid
Repo: Altinn/altinn-studio PR: 16372
File: src/Designer/frontend/libs/studio-components/src/components/index.ts:34-34
Timestamp: 2025-09-23T08:53:19.508Z
Learning: In the Altinn Studio codebase, when moving components from studio-components-legacy to studio-components, the team prefers to handle the component migration and remaining import updates in separate PRs to maintain focused, atomic changes.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16095
File: src/App/app-template-dotnet/src/App/models/model.cs:10-29
Timestamp: 2025-08-15T05:53:32.529Z
Learning: When moving app templates in Altinn Studio, mirkoSekulic prefers to keep the PR scope strictly limited to the move operation itself, with all content changes (including code quality improvements, naming convention fixes, and attribute cleanup) deferred to separate PRs for better maintainability and atomic changes.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 14575
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/OverrideFields/Summary2OverrideDisplayType.tsx:47-58
Timestamp: 2025-02-07T09:50:33.290Z
Learning: Avoid suggesting StudioCombobox component in future PRs for the Altinn Studio repository. A new replacement component will be introduced later.
Learnt from: mgunnerud
Repo: Altinn/altinn-studio PR: 16187
File: frontend/resourceadm/components/ResourcePageInputs/ResourceSwitchInput.tsx:3-3
Timestamp: 2025-08-25T13:55:27.042Z
Learning: In the Altinn Studio codebase, when migrating from studio/components-legacy to studio/components, PRs should focus only on components that have available replacements in the new package. Components without new implementations should remain in the legacy package until replacements are available, supporting an incremental migration approach.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:36:57.397Z
Learning: When a developer copies a component from `studio-components-legacy` to `studio-components` in the Altinn Studio repository and has not added a deprecation comment for the component in `studio-components-legacy`, suggest adding the deprecation comment in the PR review.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:37:01.381Z
Learning: When reviewing PRs in the Altinn Studio repository where a developer copies a component from `studio-components-legacy` to `studio-components`, suggest adding a `deprecated` JSDoc comment to the component in `studio-components-legacy` if one hasn't been added. The deprecation comment should recommend using the component from `studio/components` instead.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 16123
File: frontend/libs/studio-components/src/types/OverridableComponentProps.ts:3-6
Timestamp: 2025-08-15T08:21:51.564Z
Learning: ErlingHauan prefers to keep PRs that move types from studio-components-legacy to studio-components focused strictly on the move operation, deferring type implementation changes or improvements to separate PRs for better maintainability and atomic changes.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16191
File: .dockerignore:3-3
Timestamp: 2025-08-26T12:35:44.069Z
Learning: In the Altinn Studio repository, mirkoSekulic is implementing a phased directory restructuring approach where backend and testdata are moved first, followed by separate moves for readme, frontend, and development directories into the Designer directory. This ensures atomic changes and prevents path reference issues during the transition.
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15719
File: frontend/dashboard/pages/OrgContentLibraryPage/FeedbackForm/FeedbackForm.tsx:22-39
Timestamp: 2025-06-20T13:41:45.278Z
Learning: In the Altinn Studio project, temporary feedback form components may intentionally use hardcoded text instead of internationalization when they are designed to be removed before language support is implemented.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 14398
File: frontend/libs/studio-components/src/components/StudioCodelistEditor/StudioCodeListEditorRow/StudioCodeListEditorRow.tsx:128-136
Timestamp: 2025-02-10T11:28:12.444Z
Learning: The StudioCodeListEditor component's handling of undefined values (when StudioDecimalField is cleared) is verified through the test case that checks if a numberfield is rendered with inputMode='decimal'.
📚 Learning: 2025-10-25T11:48:49.152Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 16740
File: src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx:63-71
Timestamp: 2025-10-25T11:48:49.152Z
Learning: In src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx, the NameConfig component is intentionally rendered in two places: once in the mainConfig section and once inside the "Text" accordion section. This duplication is by design.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.tssrc/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx
📚 Learning: 2025-11-25T08:21:14.748Z
Learnt from: CR
Repo: Altinn/altinn-studio PR: 0
File: src/App/frontend/CLAUDE.md:0-0
Timestamp: 2025-11-25T08:21:14.748Z
Learning: Applies to src/App/frontend/**/*.test.{ts,tsx} : Use `renderWithProviders` utility from `src/test/renderWithProviders.tsx` when testing components that require form layout context
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts
📚 Learning: 2025-10-28T20:54:18.494Z
Learnt from: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16787
File: src/Designer/frontend/packages/ux-editor-v3/src/components/toolbar/RuleModal.test.tsx:70-70
Timestamp: 2025-10-28T20:54:18.494Z
Learning: Test inconsistencies in the ux-editor-v3 package do not need to be fixed, as indicated by the maintainer for the file `src/Designer/frontend/packages/ux-editor-v3/src/components/toolbar/RuleModal.test.tsx`.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts
📚 Learning: 2025-11-25T08:21:14.748Z
Learnt from: CR
Repo: Altinn/altinn-studio PR: 0
File: src/App/frontend/CLAUDE.md:0-0
Timestamp: 2025-11-25T08:21:14.748Z
Learning: Applies to src/App/frontend/src/layout/**/*.{ts,tsx} : Layout components must have standardized structure with `config.ts`, `Component.tsx`, `index.tsx`, and generated `config.generated.ts` files
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts
📚 Learning: 2025-05-30T14:19:53.714Z
Learnt from: nkylstad
Repo: Altinn/altinn-studio PR: 15550
File: frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/SpecificMainConfig/ImageMainConfig.test.tsx:17-17
Timestamp: 2025-05-30T14:19:53.714Z
Learning: In the SpecificMainConfig test files, the standard pattern is to use 'ComponentMainConfig' as the top-level describe block name, with nested describe blocks named after the specific component type being tested (e.g., 'Image', 'Options', 'Summary2').
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts
📚 Learning: 2025-06-20T13:41:45.278Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15719
File: frontend/dashboard/pages/OrgContentLibraryPage/FeedbackForm/FeedbackForm.tsx:22-39
Timestamp: 2025-06-20T13:41:45.278Z
Learning: In the Altinn Studio project, temporary feedback form components may intentionally use hardcoded text instead of internationalization when they are designed to be removed before language support is implemented.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts
📚 Learning: 2025-02-07T09:50:33.290Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 14575
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/OverrideFields/Summary2OverrideDisplayType.tsx:47-58
Timestamp: 2025-02-07T09:50:33.290Z
Learning: Avoid suggesting StudioCombobox component in future PRs for the Altinn Studio repository. A new replacement component will be introduced later.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts
📚 Learning: 2025-09-23T08:53:19.508Z
Learnt from: framitdavid
Repo: Altinn/altinn-studio PR: 16372
File: src/Designer/frontend/libs/studio-components/src/components/index.ts:34-34
Timestamp: 2025-09-23T08:53:19.508Z
Learning: In the Altinn Studio codebase, when moving components from studio-components-legacy to studio-components, the team prefers to handle the component migration and remaining import updates in separate PRs to maintain focused, atomic changes.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts
📚 Learning: 2025-11-03T08:16:33.782Z
Learnt from: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16865
File: src/Designer/frontend/app-preview/src/components/PreviewControlHeader/PreviewControlHeader.tsx:44-50
Timestamp: 2025-11-03T08:16:33.782Z
Learning: In PreviewControlHeader.tsx (app-preview package), JamalAlabdullah confirmed that the StudioSelect component for layout set selection is acceptable with an empty label prop (label={''}), indicating this is an intentional design decision for that specific component.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-09-25T08:50:58.936Z
Learnt from: Jondyr
Repo: Altinn/altinn-studio PR: 16431
File: src/Designer/frontend/packages/ux-editor/src/converters/formLayoutConverters/externalLayoutToInternal.ts:28-33
Timestamp: 2025-09-25T08:50:58.936Z
Learning: The Altinn Studio codebase contains two parallel UX editor packages: `ux-editor` (current/newer) and `ux-editor-v3` (legacy). When reviewing code changes, need to distinguish between these versions as they have different API implementations, particularly around datamodel binding conversions.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-09-26T12:03:56.908Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-26T12:03:56.908Z
Learning: In the Altinn Studio codebase, when reviewing migration from studio/components-legacy to studio/components for the StudioExpression component, the ExpressionTexts type interface does not require additional keys like `dataModel`, `resource`, or `missingDataModelLabel`. TypeScript compilation will catch any missing required properties, so successful builds indicate proper type compliance.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-07-02T09:02:34.292Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx
📚 Learning: 2025-09-26T12:03:51.850Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-26T12:03:51.850Z
Learning: In PR #16443, TomasEng clarified that the ExpressionTexts type in studio/components does not require additional keys like dataModel, resource, or missingDataModelLabel that were suggested in a review comment. TypeScript compilation would catch any missing required properties, confirming the migration is correctly implemented.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-11-03T18:58:18.996Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 16876
File: src/Designer/frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListsPage/CodeListsPage.tsx:17-23
Timestamp: 2025-11-03T18:58:18.996Z
Learning: In src/Designer/frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListsPage/CodeListsPage.tsx, the CodeListsPage component is intentionally an uncontrolled component where the codeLists prop only initializes state on mount and does not synchronize when the prop changes. This is because the createCodeListMap function generates random UUIDs for keys, making it impure. Re-calling it after the first render would reassign keys and significantly disrupt the UI (unmounting/remounting components). The component manages its own state for draft/editing purposes, and the prop is not expected to change to anything that doesn't correspond to the internal state.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx
📚 Learning: 2025-06-16T12:16:47.892Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15659
File: frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/OptionListEditor.tsx:44-47
Timestamp: 2025-06-16T12:16:47.892Z
Learning: In selection components within the Altinn Studio UX editor, the code design anticipates that one of the properties `options` and `optionsId` should always be set, representing manual options vs external option list references respectively. However, this mutual exclusivity is not well reflected in the current type system.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-01-13T12:44:45.751Z
Learnt from: Jondyr
Repo: Altinn/altinn-studio PR: 14379
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/Summary2OverrideEntry.tsx:63-70
Timestamp: 2025-01-13T12:44:45.751Z
Learning: In the Altinn Studio codebase, when using StudioProperty.Button component, it's expected to pass `false` as the value prop when no meaningful value is available (e.g., when componentNameType is undefined). This is the intended behavior and should not be changed to handle undefined cases differently.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx
📚 Learning: 2025-02-10T11:46:29.320Z
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 14398
File: frontend/libs/studio-components/src/components/StudioCodelistEditor/StudioCodeListEditorRow/StudioCodeListEditorRow.tsx:0-0
Timestamp: 2025-02-10T11:46:29.320Z
Learning: The Numberfield component handles number validation internally and only calls onChange with valid number or undefined values, making additional validation in parent components unnecessary.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx
📚 Learning: 2025-06-16T12:19:07.682Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15495
File: frontend/app-development/layout/VersionDialog/VersionDialogTableRow.tsx:5-13
Timestamp: 2025-06-16T12:19:07.682Z
Learning: In the Altinn Studio codebase, the VersionDialog components work with major version numbers only (e.g., 4, 8) rather than full semantic versions. The latestVersion prop in VersionDialogTableRow should be typed as number since it represents maximum supported major version constants like MAXIMUM_SUPPORTED_FRONTEND_VERSION = 4.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx
📚 Learning: 2025-08-07T13:08:11.913Z
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15981
File: frontend/libs/studio-components/src/components/StudioLabelAsParagraph/StudioLabelAsParagraph.tsx:10-10
Timestamp: 2025-08-07T13:08:11.913Z
Learning: In the Altinn Studio codebase, when using components with the `asChild` prop pattern (like from digdir/designsystemet-react), the ref should be placed on the child DOM element rather than forwarded to the parent component. This pattern was established in PR #15998 where Card asChild was used with StudioFieldset, and the ref was passed to the StudioFieldset (child) not the Card (parent). This approach provides correct TypeScript typing automatically since the ref points directly to the actual DOM element being rendered.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx
📚 Learning: 2025-07-03T14:54:51.389Z
Learnt from: mgunnerud
Repo: Altinn/altinn-studio PR: 15826
File: frontend/resourceadm/components/ResourcePageInputs/ResourceLanguageTextField.tsx:95-95
Timestamp: 2025-07-03T14:54:51.389Z
Learning: The StudioTextfield component at frontend/libs/studio-components/src/components/StudioTextfield/StudioTextfield.tsx has a typing limitation where it only accepts `Ref<HTMLInputElement>` even when rendering textarea elements (when multiline=true). This creates a type mismatch between the ref type and the actual element type, requiring workarounds like removing ref props when both input and textarea elements need to be supported.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx
📚 Learning: 2025-07-02T09:04:49.873Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/PropertiesHeader.tsx:28-30
Timestamp: 2025-07-02T09:04:49.873Z
Learning: In the Altinn Studio UX editor, the PropertiesHeader component can safely access schema.properties directly without null checks because the parent ComponentConfigPanel component handles the loading state and only renders PropertiesHeader after the schema is fetched via useComponentSchemaQuery.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx
🧬 Code graph analysis (3)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts (2)
src/Designer/frontend/packages/ux-editor/src/testing/componentMocks.ts (1)
componentMocks(215-248)src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.ts (1)
componentComparison(8-13)
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx (1)
src/Designer/frontend/libs/studio-components/src/components/StudioFormActions/index.ts (1)
StudioFormActions(1-1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx (4)
src/Designer/frontend/packages/ux-editor/src/hooks/index.ts (1)
useComponentPropertyLabel(9-9)src/Designer/frontend/packages/ux-editor/src/components/config/editModal/EditNumberValue.tsx (1)
EditNumberValue(26-83)src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx (1)
SelectPropertyEditor(18-72)src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.ts (1)
componentComparison(8-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: CodeQL
- GitHub Check: Testing
- GitHub Check: Build environment and run e2e test
🔇 Additional comments (8)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigPropertiesUtils.test.ts (3)
1-2: LGTM!The imports are correct and necessary for testing the
componentComparisonutility.
4-12: LGTM!The test setup is clear and correctly creates test data for both identical and different component scenarios. The describe block name now correctly matches the file name.
20-24: LGTM!The test correctly validates that components with different properties return false. The modification to
dataModelBindings.simpleBindingcreates a realistic difference for testing.src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx (2)
26-36: LGTM – thekeepEditOpenbranch correctly uses direct persistence.When
keepEditOpenis true, the component bypasses local state and passesinitialComponentdirectly alongsidehandleComponentUpdate. This ensures immediate persistence of changes to the parent, with re-renders providing the updated value. The logic is sound for this "always in edit mode" use case.
40-60: Staged edit flow implementation looks correct.The
SelectPropertyEditorwrapper properly orchestrates the edit-mode flow:
currentComponentis passed toEditNumberValuefor display/editingsetCurrentComponentupdates local state on changeonSavecommits viahandleComponentUpdate(currentComponent)onCancelreverts viasetCurrentComponent(initialComponent)isSaveDisabledcorrectly usescomponentComparisonto detect unchanged statesrc/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.tsx (3)
7-16: Clear prop interface for the controlled edit-mode flow.The updated type definition with required
onSave,onCancel, andisSaveDisabledprops establishes a well-defined contract for the staged-edit pattern. Makingchildrenrequired is appropriate since the component now explicitly wraps an editor.
31-41: View mode implementation is clean and straightforward.The non-edit mode correctly renders a
StudioProperty.Buttonthat enters edit mode on click, passing through the display props (property,title,value,className).
43-51: Handler functions toggle mode before invoking callbacks.Note that
setEditMode(false)is called before the callback, so ifonSave()triggers an async operation that fails, the user will have already exited edit mode. Since parent components preservecurrentComponentin state, the user doesn't lose their edits – they can re-enter edit mode and retry. This is acceptable, though worth documenting.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.test.tsx (1)
44-46: Consider adding test coverage for the new props.The new
onSave,onCancel, andisSaveDisabledprops extend the component's API but aren't currently exercised by tests. Consider adding tests to verify:
- The
onCancelcallback is invoked when the cancel button is clicked- The
onSavecallback is invoked when the save button is clicked- The save button is disabled when
isSaveDisabledistrueThis would improve maintainability and ensure the edit-mode flow works as expected.
🔎 Example test cases
it('should call onCancel when the cancel button is clicked', async () => { const user = userEvent.setup(); const onCancel = jest.fn(); renderSelectPropertyEditor({ onCancel }); await user.click(screen.getByText('Test property')); await user.click(screen.getByRole('button', { name: textMock('general.cancel') })); expect(onCancel).toHaveBeenCalledTimes(1); }); it('should call onSave when the save button is clicked', async () => { const user = userEvent.setup(); const onSave = jest.fn(); renderSelectPropertyEditor({ onSave }); await user.click(screen.getByText('Test property')); await user.click(screen.getByRole('button', { name: textMock('general.save') })); expect(onSave).toHaveBeenCalledTimes(1); }); it('should disable the save button when isSaveDisabled is true', async () => { const user = userEvent.setup(); renderSelectPropertyEditor({ isSaveDisabled: true }); await user.click(screen.getByText('Test property')); expect(screen.getByRole('button', { name: textMock('general.save') })).toBeDisabled(); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.test.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-10T12:02:06.348Z
Learning: In the Altinn Studio repository, the team convention is to remove irrelevant checkboxes from the PR template description rather than leaving them unmarked. This makes it clear that the author has intentionally considered each item and removed non-applicable ones, helping to avoid merging PRs with incomplete checklists.
Learnt from: framitdavid
Repo: Altinn/altinn-studio PR: 16372
File: src/Designer/frontend/libs/studio-components/src/components/index.ts:34-34
Timestamp: 2025-09-23T08:53:19.508Z
Learning: In the Altinn Studio codebase, when moving components from studio-components-legacy to studio-components, the team prefers to handle the component migration and remaining import updates in separate PRs to maintain focused, atomic changes.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16095
File: src/App/app-template-dotnet/src/App/models/model.cs:10-29
Timestamp: 2025-08-15T05:53:32.529Z
Learning: When moving app templates in Altinn Studio, mirkoSekulic prefers to keep the PR scope strictly limited to the move operation itself, with all content changes (including code quality improvements, naming convention fixes, and attribute cleanup) deferred to separate PRs for better maintainability and atomic changes.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 14575
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/OverrideFields/Summary2OverrideDisplayType.tsx:47-58
Timestamp: 2025-02-07T09:50:33.290Z
Learning: Avoid suggesting StudioCombobox component in future PRs for the Altinn Studio repository. A new replacement component will be introduced later.
Learnt from: mgunnerud
Repo: Altinn/altinn-studio PR: 16187
File: frontend/resourceadm/components/ResourcePageInputs/ResourceSwitchInput.tsx:3-3
Timestamp: 2025-08-25T13:55:27.042Z
Learning: In the Altinn Studio codebase, when migrating from studio/components-legacy to studio/components, PRs should focus only on components that have available replacements in the new package. Components without new implementations should remain in the legacy package until replacements are available, supporting an incremental migration approach.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:36:57.397Z
Learning: When a developer copies a component from `studio-components-legacy` to `studio-components` in the Altinn Studio repository and has not added a deprecation comment for the component in `studio-components-legacy`, suggest adding the deprecation comment in the PR review.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:37:01.381Z
Learning: When reviewing PRs in the Altinn Studio repository where a developer copies a component from `studio-components-legacy` to `studio-components`, suggest adding a `deprecated` JSDoc comment to the component in `studio-components-legacy` if one hasn't been added. The deprecation comment should recommend using the component from `studio/components` instead.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 16123
File: frontend/libs/studio-components/src/types/OverridableComponentProps.ts:3-6
Timestamp: 2025-08-15T08:21:51.564Z
Learning: ErlingHauan prefers to keep PRs that move types from studio-components-legacy to studio-components focused strictly on the move operation, deferring type implementation changes or improvements to separate PRs for better maintainability and atomic changes.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16191
File: .dockerignore:3-3
Timestamp: 2025-08-26T12:35:44.069Z
Learning: In the Altinn Studio repository, mirkoSekulic is implementing a phased directory restructuring approach where backend and testdata are moved first, followed by separate moves for readme, frontend, and development directories into the Designer directory. This ensures atomic changes and prevents path reference issues during the transition.
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15719
File: frontend/dashboard/pages/OrgContentLibraryPage/FeedbackForm/FeedbackForm.tsx:22-39
Timestamp: 2025-06-20T13:41:45.278Z
Learning: In the Altinn Studio project, temporary feedback form components may intentionally use hardcoded text instead of internationalization when they are designed to be removed before language support is implemented.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
📚 Learning: 2025-06-16T12:41:06.726Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15659
File: frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/CodeListDialog/CodeListDialog.test.tsx:145-151
Timestamp: 2025-06-16T12:41:06.726Z
Learning: In the Altinn Studio codebase, TomasEng has confirmed that using `expect(screen.getByRole('dialog')).toBeVisible` directly with `waitFor` is acceptable and equivalent to wrapping it in a function callback. This pattern is intentionally used in their testing setup.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.test.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: CodeQL
- GitHub Check: Testing
- GitHub Check: Typechecking and linting
- GitHub Check: Build environment and run e2e test
🔇 Additional comments (1)
src/Designer/frontend/packages/ux-editor/src/components/config/SelectPropertyEditor/SelectPropertyEditor.test.tsx (1)
20-27: LGTM! Test updated to reflect cancel terminology.The test correctly verifies that clicking the cancel button hides the children editor. The terminology change from "close" to "cancel" better reflects the edit-mode semantics.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx (1)
114-127: Consider refactoring userEvent usage for consistency.The helper functions create their own
userEvent.setup()instances (lines 115, 130), whilst some tests create a user instance and use it directly (line 15, 21). For better consistency, consider either:
- Always creating a user in each helper function (remove user creation from tests)
- Accepting a user parameter in helpers and creating it once per test
Additionally, closing the dropdown by clicking
document.body(line 126) works but is somewhat indirect. You might consider usinguser.keyboard('{Escape}')for a more explicit interaction pattern.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-10T12:02:06.348Z
Learning: In the Altinn Studio repository, the team convention is to remove irrelevant checkboxes from the PR template description rather than leaving them unmarked. This makes it clear that the author has intentionally considered each item and removed non-applicable ones, helping to avoid merging PRs with incomplete checklists.
Learnt from: framitdavid
Repo: Altinn/altinn-studio PR: 16372
File: src/Designer/frontend/libs/studio-components/src/components/index.ts:34-34
Timestamp: 2025-09-23T08:53:19.508Z
Learning: In the Altinn Studio codebase, when moving components from studio-components-legacy to studio-components, the team prefers to handle the component migration and remaining import updates in separate PRs to maintain focused, atomic changes.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 14575
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/OverrideFields/Summary2OverrideDisplayType.tsx:47-58
Timestamp: 2025-02-07T09:50:33.290Z
Learning: Avoid suggesting StudioCombobox component in future PRs for the Altinn Studio repository. A new replacement component will be introduced later.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16095
File: src/App/app-template-dotnet/src/App/models/model.cs:10-29
Timestamp: 2025-08-15T05:53:32.529Z
Learning: When moving app templates in Altinn Studio, mirkoSekulic prefers to keep the PR scope strictly limited to the move operation itself, with all content changes (including code quality improvements, naming convention fixes, and attribute cleanup) deferred to separate PRs for better maintainability and atomic changes.
Learnt from: mgunnerud
Repo: Altinn/altinn-studio PR: 16187
File: frontend/resourceadm/components/ResourcePageInputs/ResourceSwitchInput.tsx:3-3
Timestamp: 2025-08-25T13:55:27.042Z
Learning: In the Altinn Studio codebase, when migrating from studio/components-legacy to studio/components, PRs should focus only on components that have available replacements in the new package. Components without new implementations should remain in the legacy package until replacements are available, supporting an incremental migration approach.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:36:57.397Z
Learning: When a developer copies a component from `studio-components-legacy` to `studio-components` in the Altinn Studio repository and has not added a deprecation comment for the component in `studio-components-legacy`, suggest adding the deprecation comment in the PR review.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:37:01.381Z
Learning: When reviewing PRs in the Altinn Studio repository where a developer copies a component from `studio-components-legacy` to `studio-components`, suggest adding a `deprecated` JSDoc comment to the component in `studio-components-legacy` if one hasn't been added. The deprecation comment should recommend using the component from `studio/components` instead.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 16123
File: frontend/libs/studio-components/src/types/OverridableComponentProps.ts:3-6
Timestamp: 2025-08-15T08:21:51.564Z
Learning: ErlingHauan prefers to keep PRs that move types from studio-components-legacy to studio-components focused strictly on the move operation, deferring type implementation changes or improvements to separate PRs for better maintainability and atomic changes.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16191
File: .dockerignore:3-3
Timestamp: 2025-08-26T12:35:44.069Z
Learning: In the Altinn Studio repository, mirkoSekulic is implementing a phased directory restructuring approach where backend and testdata are moved first, followed by separate moves for readme, frontend, and development directories into the Designer directory. This ensures atomic changes and prevents path reference issues during the transition.
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15719
File: frontend/dashboard/pages/OrgContentLibraryPage/FeedbackForm/FeedbackForm.tsx:22-39
Timestamp: 2025-06-20T13:41:45.278Z
Learning: In the Altinn Studio project, temporary feedback form components may intentionally use hardcoded text instead of internationalization when they are designed to be removed before language support is implemented.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
📚 Learning: 2025-07-15T06:14:47.860Z
Learnt from: wrt95
Repo: Altinn/altinn-studio PR: 15850
File: frontend/app-development/features/appSettings/components/TabsContent/Tabs/AboutTab/AppConfigForm/StatusRadioGroup/StatusRadioGroup.test.tsx:47-56
Timestamp: 2025-07-15T06:14:47.860Z
Learning: In StatusRadioGroup tests for Altinn Studio, the onChangeStatus callback is called with two parameters: the selected value and the previous value. This is why test assertions like `expect(onChangeStatus).toHaveBeenCalledWith('Completed', '');` use both parameters, even though the TypeScript interface only shows one parameter.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx
📚 Learning: 2025-10-28T20:54:18.494Z
Learnt from: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16787
File: src/Designer/frontend/packages/ux-editor-v3/src/components/toolbar/RuleModal.test.tsx:70-70
Timestamp: 2025-10-28T20:54:18.494Z
Learning: Test inconsistencies in the ux-editor-v3 package do not need to be fixed, as indicated by the maintainer for the file `src/Designer/frontend/packages/ux-editor-v3/src/components/toolbar/RuleModal.test.tsx`.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx
📚 Learning: 2025-10-25T11:48:49.152Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 16740
File: src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx:63-71
Timestamp: 2025-10-25T11:48:49.152Z
Learning: In src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx, the NameConfig component is intentionally rendered in two places: once in the mainConfig section and once inside the "Text" accordion section. This duplication is by design.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx
📚 Learning: 2025-11-25T08:21:14.748Z
Learnt from: CR
Repo: Altinn/altinn-studio PR: 0
File: src/App/frontend/CLAUDE.md:0-0
Timestamp: 2025-11-25T08:21:14.748Z
Learning: Applies to src/App/frontend/**/*.test.{ts,tsx} : Use `renderWithProviders` utility from `src/test/renderWithProviders.tsx` when testing components that require form layout context
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx
📚 Learning: 2025-06-16T12:41:06.726Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15659
File: frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/CodeListDialog/CodeListDialog.test.tsx:145-151
Timestamp: 2025-06-16T12:41:06.726Z
Learning: In the Altinn Studio codebase, TomasEng has confirmed that using `expect(screen.getByRole('dialog')).toBeVisible` directly with `waitFor` is acceptable and equivalent to wrapping it in a function callback. This pattern is intentionally used in their testing setup.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: CodeQL
- GitHub Check: Testing
- GitHub Check: Typechecking and linting
- GitHub Check: Build environment and run e2e test
🔇 Additional comments (4)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx (4)
10-12: LGTM! Proper test cleanup.Adding
afterEachto clear mocks ensures test isolation and prevents state leakage between test cases.
24-37: Test flow correctly reflects the new controlled edit pattern.The updated assertions properly verify that
handleComponentUpdateis only called after the save action, and the DOM querying correctly reflects that the selected value is now displayed within a button element.
129-134: Well-implemented save helper.The
saveChangeshelper properly waits for the save button to be enabled before clicking, which prevents potential race conditions and makes the test more robust.
82-98: keepEditOpen mode test correctly omits save step.The test properly verifies that in
keepEditOpenmode,handleComponentUpdateis called immediately after option selection without requiring an explicit save action, which is the expected behaviour for this mode.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #17316 +/- ##
==========================================
- Coverage 96.26% 96.24% -0.02%
==========================================
Files 2384 2394 +10
Lines 30282 30406 +124
Branches 3529 3540 +11
==========================================
+ Hits 29150 29264 +114
- Misses 820 832 +12
+ Partials 312 310 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx (1)
12-12: Fix typo in parameter name.The parameter name is misspelt as
nemeinstead ofname.🔎 Proposed fix
- validationFn?: (neme: string) => string | undefined; + validationFn?: (name: string) => string | undefined;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.module.csssrc/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: framitdavid
Repo: Altinn/altinn-studio PR: 16372
File: src/Designer/frontend/libs/studio-components/src/components/index.ts:34-34
Timestamp: 2025-09-23T08:53:19.508Z
Learning: In the Altinn Studio codebase, when moving components from studio-components-legacy to studio-components, the team prefers to handle the component migration and remaining import updates in separate PRs to maintain focused, atomic changes.
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-10T12:02:06.348Z
Learning: In the Altinn Studio repository, the team convention is to remove irrelevant checkboxes from the PR template description rather than leaving them unmarked. This makes it clear that the author has intentionally considered each item and removed non-applicable ones, helping to avoid merging PRs with incomplete checklists.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16095
File: src/App/app-template-dotnet/src/App/models/model.cs:10-29
Timestamp: 2025-08-15T05:53:32.529Z
Learning: When moving app templates in Altinn Studio, mirkoSekulic prefers to keep the PR scope strictly limited to the move operation itself, with all content changes (including code quality improvements, naming convention fixes, and attribute cleanup) deferred to separate PRs for better maintainability and atomic changes.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 14575
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/OverrideFields/Summary2OverrideDisplayType.tsx:47-58
Timestamp: 2025-02-07T09:50:33.290Z
Learning: Avoid suggesting StudioCombobox component in future PRs for the Altinn Studio repository. A new replacement component will be introduced later.
Learnt from: mgunnerud
Repo: Altinn/altinn-studio PR: 16187
File: frontend/resourceadm/components/ResourcePageInputs/ResourceSwitchInput.tsx:3-3
Timestamp: 2025-08-25T13:55:27.042Z
Learning: In the Altinn Studio codebase, when migrating from studio/components-legacy to studio/components, PRs should focus only on components that have available replacements in the new package. Components without new implementations should remain in the legacy package until replacements are available, supporting an incremental migration approach.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16191
File: .dockerignore:3-3
Timestamp: 2025-08-26T12:35:44.069Z
Learning: In the Altinn Studio repository, mirkoSekulic is implementing a phased directory restructuring approach where backend and testdata are moved first, followed by separate moves for readme, frontend, and development directories into the Designer directory. This ensures atomic changes and prevents path reference issues during the transition.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:36:57.397Z
Learning: When a developer copies a component from `studio-components-legacy` to `studio-components` in the Altinn Studio repository and has not added a deprecation comment for the component in `studio-components-legacy`, suggest adding the deprecation comment in the PR review.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 16123
File: frontend/libs/studio-components/src/types/OverridableComponentProps.ts:3-6
Timestamp: 2025-08-15T08:21:51.564Z
Learning: ErlingHauan prefers to keep PRs that move types from studio-components-legacy to studio-components focused strictly on the move operation, deferring type implementation changes or improvements to separate PRs for better maintainability and atomic changes.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:37:01.381Z
Learning: When reviewing PRs in the Altinn Studio repository where a developer copies a component from `studio-components-legacy` to `studio-components`, suggest adding a `deprecated` JSDoc comment to the component in `studio-components-legacy` if one hasn't been added. The deprecation comment should recommend using the component from `studio/components` instead.
Learnt from: Jondyr
Repo: Altinn/altinn-studio PR: 16431
File: src/Designer/frontend/packages/ux-editor/src/converters/formLayoutConverters/externalLayoutToInternal.ts:28-33
Timestamp: 2025-09-25T08:50:58.936Z
Learning: The Altinn Studio codebase contains two parallel UX editor packages: `ux-editor` (current/newer) and `ux-editor-v3` (legacy). When reviewing code changes, need to distinguish between these versions as they have different API implementations, particularly around datamodel binding conversions.
📚 Learning: 2025-10-25T11:48:49.152Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 16740
File: src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx:63-71
Timestamp: 2025-10-25T11:48:49.152Z
Learning: In src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx, the NameConfig component is intentionally rendered in two places: once in the mainConfig section and once inside the "Text" accordion section. This duplication is by design.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.module.csssrc/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx
📚 Learning: 2025-10-26T21:09:38.402Z
Learnt from: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16742
File: src/Designer/frontend/packages/schema-editor/src/components/SchemaInspector/ItemDataComponent.module.css:23-25
Timestamp: 2025-10-26T21:09:38.402Z
Learning: In the Altinn Studio codebase, the design system is migrating from `--fds-*` prefixed CSS variables to `--ds-*` prefixed variables. The `--fds-*` prefix will be removed in the future, so usage of `--ds-*` variables (like `--ds-size-4`) alongside existing `--fds-*` variables (like `--fds-spacing-4`) in the same file is intentional during this transition period.
<!--
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.module.css
📚 Learning: 2025-09-25T08:50:58.936Z
Learnt from: Jondyr
Repo: Altinn/altinn-studio PR: 16431
File: src/Designer/frontend/packages/ux-editor/src/converters/formLayoutConverters/externalLayoutToInternal.ts:28-33
Timestamp: 2025-09-25T08:50:58.936Z
Learning: The Altinn Studio codebase contains two parallel UX editor packages: `ux-editor` (current/newer) and `ux-editor-v3` (legacy). When reviewing code changes, need to distinguish between these versions as they have different API implementations, particularly around datamodel binding conversions.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.module.csssrc/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx
📚 Learning: 2025-09-26T12:03:56.908Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-26T12:03:56.908Z
Learning: In the Altinn Studio codebase, when reviewing migration from studio/components-legacy to studio/components for the StudioExpression component, the ExpressionTexts type interface does not require additional keys like `dataModel`, `resource`, or `missingDataModelLabel`. TypeScript compilation will catch any missing required properties, so successful builds indicate proper type compliance.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx
📚 Learning: 2025-09-26T12:03:51.850Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-26T12:03:51.850Z
Learning: In PR #16443, TomasEng clarified that the ExpressionTexts type in studio/components does not require additional keys like dataModel, resource, or missingDataModelLabel that were suggested in a review comment. TypeScript compilation would catch any missing required properties, confirming the migration is correctly implemented.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx
📚 Learning: 2025-11-03T08:16:33.782Z
Learnt from: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16865
File: src/Designer/frontend/app-preview/src/components/PreviewControlHeader/PreviewControlHeader.tsx:44-50
Timestamp: 2025-11-03T08:16:33.782Z
Learning: In PreviewControlHeader.tsx (app-preview package), JamalAlabdullah confirmed that the StudioSelect component for layout set selection is acceptable with an empty label prop (label={''}), indicating this is an intentional design decision for that specific component.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx
📚 Learning: 2025-12-18T12:57:40.385Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 17285
File: src/Designer/frontend/libs/studio-content-library/src/pages/Page.tsx:25-29
Timestamp: 2025-12-18T12:57:40.385Z
Learning: In src/Designer/frontend/libs/studio-content-library/src/pages/Page.tsx, the extractProps method requires a type assertion `as PagePropsMap<Name>` on the return because TypeScript cannot narrow the indexed access `config.pages[this.name].props` to the specific generic type PagePropsMap<Name> even after Guard.againstMissingProperty. Removing this cast results in TS2322 error with a union type instead of the narrowed type. This is a known TypeScript limitation with generics and indexed access types.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx
📚 Learning: 2025-08-12T13:45:39.680Z
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 16081
File: frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListSelector/OptionListSelector.tsx:108-115
Timestamp: 2025-08-12T13:45:39.680Z
Learning: The StudioButton component from studio/components-legacy defaults to type="button", so explicit type attributes are not needed to prevent form submission behavior.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx
📚 Learning: 2025-07-03T14:54:51.389Z
Learnt from: mgunnerud
Repo: Altinn/altinn-studio PR: 15826
File: frontend/resourceadm/components/ResourcePageInputs/ResourceLanguageTextField.tsx:95-95
Timestamp: 2025-07-03T14:54:51.389Z
Learning: The StudioTextfield component at frontend/libs/studio-components/src/components/StudioTextfield/StudioTextfield.tsx has a typing limitation where it only accepts `Ref<HTMLInputElement>` even when rendering textarea elements (when multiline=true). This creates a type mismatch between the ref type and the actual element type, requiring workarounds like removing ref props when both input and textarea elements need to be supported.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx
📚 Learning: 2025-07-03T14:54:51.389Z
Learnt from: mgunnerud
Repo: Altinn/altinn-studio PR: 15826
File: frontend/resourceadm/components/ResourcePageInputs/ResourceLanguageTextField.tsx:95-95
Timestamp: 2025-07-03T14:54:51.389Z
Learning: The StudioTextfield component at frontend/libs/studio-components/src/components/StudioTextfield/StudioTextfield.tsx accepts only HTMLInputElement as ref type, even when the component renders an HTMLTextAreaElement (when multiline is true). This is a typing constraint of the component library.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx
📚 Learning: 2025-01-23T14:49:32.407Z
Learnt from: framitdavid
Repo: Altinn/altinn-studio PR: 14494
File: frontend/app-development/features/appPublish/components/UndeployConsequenceDialog/UndeployConsequenceDialog.tsx:21-27
Timestamp: 2025-01-23T14:49:32.407Z
Learning: The StudioModal component from studio/components already includes built-in support for dialog close handling, keyboard interaction (Esc key), and focus trap for accessibility.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx
📚 Learning: 2025-10-29T07:11:59.228Z
Learnt from: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16787
File: src/Designer/frontend/packages/ux-editor/src/components/config/editModal/EditImage/LocalImage/PreviewImageSummary/DeleteOptionsModal/DeleteOptionsModal.tsx:18-30
Timestamp: 2025-10-29T07:11:59.228Z
Learning: The StudioDialog component from studio/components includes built-in support for dialog close handling, with a close button in the header and keyboard interaction (Esc key) for accessibility.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx
📚 Learning: 2025-07-02T09:02:34.292Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx
📚 Learning: 2025-01-13T12:44:45.751Z
Learnt from: Jondyr
Repo: Altinn/altinn-studio PR: 14379
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/Summary2OverrideEntry.tsx:63-70
Timestamp: 2025-01-13T12:44:45.751Z
Learning: In the Altinn Studio codebase, when using StudioProperty.Button component, it's expected to pass `false` as the value prop when no meaningful value is available (e.g., when componentNameType is undefined). This is the intended behavior and should not be changed to handle undefined cases differently.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx
🧬 Code graph analysis (1)
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx (1)
src/Designer/frontend/libs/studio-components/src/components/StudioFormActions/index.ts (1)
StudioFormActions(1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build environment and run e2e test
- GitHub Check: Testing
- GitHub Check: Typechecking and linting
- GitHub Check: CodeQL
🔇 Additional comments (8)
src/Designer/frontend/packages/ux-editor/src/components/config/EditName.module.css (3)
2-3: LGTM! Flexbox layout is appropriate.The migration from grid to flexbox with gap spacing aligns well with the new component structure using StudioFormActions.
7-7: LGTM! Flexible growth for textfield.Using
flex: 1allows the textfield to appropriately occupy available space within the flex container.
10-12: LGTM! Dedicated actions area with proper spacing.The new
.editingActionsclass provides consistent spacing using the design system variable, supporting the StudioFormActions component layout.src/Designer/frontend/packages/ux-editor/src/components/config/EditName.tsx (5)
1-1: LGTM! Import changes align with refactoring.The new imports (
StudioFormActionsandStudioProperty) support the standardised edit-mode flow pattern being implemented across the config panel.
20-20: LGTM! Appropriate save button disable logic.The button is correctly disabled when validation fails or when the value hasn't changed, preventing unnecessary updates.
23-28: LGTM! Consistent read-mode UI.The
StudioProperty.Buttonprovides a standardised interface for property editing, aligning with the broader config panel refactoring.
53-62: LGTM! Textfield configuration is appropriate.The wrapper div supports the flex layout, and all necessary props (including keyboard shortcuts and error handling) are correctly configured.
63-76: TheiconOnlyprop is fully supported byStudioFormActions.The implementation is correct. The component properly declares
iconOnlyas an optional boolean prop (defaulting tofalse), controls label visibility based on this prop, and maintains accessibility by settingaria-labelattributes when labels are hidden. The hardcodedisLoading={false}is appropriate for this synchronous operation, and the configured primary and secondary actions withdisableSaveButtonare correctly implemented.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx (1)
106-117: Consider documenting the dropdown-close pattern.The
selectOptionhelper has been updated to use shared utilities and now closes the dropdown by clickingdocument.body(line 116). Whilst this works for testing purposes, this implementation detail could benefit from a brief inline comment explaining why the body click is necessary (e.g., to close the dropdown after selection).Suggested comment
await user.click(option); await waitFor(() => expect(option).toHaveAttribute('aria-selected', 'true')); + // Click outside to close the dropdown await user.click(document.body);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigBooleanProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigCustomFileEnding.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigGridProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: framitdavid
Repo: Altinn/altinn-studio PR: 16372
File: src/Designer/frontend/libs/studio-components/src/components/index.ts:34-34
Timestamp: 2025-09-23T08:53:19.508Z
Learning: In the Altinn Studio codebase, when moving components from studio-components-legacy to studio-components, the team prefers to handle the component migration and remaining import updates in separate PRs to maintain focused, atomic changes.
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-10T12:02:06.348Z
Learning: In the Altinn Studio repository, the team convention is to remove irrelevant checkboxes from the PR template description rather than leaving them unmarked. This makes it clear that the author has intentionally considered each item and removed non-applicable ones, helping to avoid merging PRs with incomplete checklists.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16095
File: src/App/app-template-dotnet/src/App/models/model.cs:10-29
Timestamp: 2025-08-15T05:53:32.529Z
Learning: When moving app templates in Altinn Studio, mirkoSekulic prefers to keep the PR scope strictly limited to the move operation itself, with all content changes (including code quality improvements, naming convention fixes, and attribute cleanup) deferred to separate PRs for better maintainability and atomic changes.
Learnt from: mgunnerud
Repo: Altinn/altinn-studio PR: 16187
File: frontend/resourceadm/components/ResourcePageInputs/ResourceSwitchInput.tsx:3-3
Timestamp: 2025-08-25T13:55:27.042Z
Learning: In the Altinn Studio codebase, when migrating from studio/components-legacy to studio/components, PRs should focus only on components that have available replacements in the new package. Components without new implementations should remain in the legacy package until replacements are available, supporting an incremental migration approach.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 14575
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/OverrideFields/Summary2OverrideDisplayType.tsx:47-58
Timestamp: 2025-02-07T09:50:33.290Z
Learning: Avoid suggesting StudioCombobox component in future PRs for the Altinn Studio repository. A new replacement component will be introduced later.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:36:57.397Z
Learning: When a developer copies a component from `studio-components-legacy` to `studio-components` in the Altinn Studio repository and has not added a deprecation comment for the component in `studio-components-legacy`, suggest adding the deprecation comment in the PR review.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16191
File: .dockerignore:3-3
Timestamp: 2025-08-26T12:35:44.069Z
Learning: In the Altinn Studio repository, mirkoSekulic is implementing a phased directory restructuring approach where backend and testdata are moved first, followed by separate moves for readme, frontend, and development directories into the Designer directory. This ensures atomic changes and prevents path reference issues during the transition.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:37:01.381Z
Learning: When reviewing PRs in the Altinn Studio repository where a developer copies a component from `studio-components-legacy` to `studio-components`, suggest adding a `deprecated` JSDoc comment to the component in `studio-components-legacy` if one hasn't been added. The deprecation comment should recommend using the component from `studio/components` instead.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 16123
File: frontend/libs/studio-components/src/types/OverridableComponentProps.ts:3-6
Timestamp: 2025-08-15T08:21:51.564Z
Learning: ErlingHauan prefers to keep PRs that move types from studio-components-legacy to studio-components focused strictly on the move operation, deferring type implementation changes or improvements to separate PRs for better maintainability and atomic changes.
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15719
File: frontend/dashboard/pages/OrgContentLibraryPage/FeedbackForm/FeedbackForm.tsx:22-39
Timestamp: 2025-06-20T13:41:45.278Z
Learning: In the Altinn Studio project, temporary feedback form components may intentionally use hardcoded text instead of internationalization when they are designed to be removed before language support is implemented.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
📚 Learning: 2025-10-28T20:54:18.494Z
Learnt from: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16787
File: src/Designer/frontend/packages/ux-editor-v3/src/components/toolbar/RuleModal.test.tsx:70-70
Timestamp: 2025-10-28T20:54:18.494Z
Learning: Test inconsistencies in the ux-editor-v3 package do not need to be fixed, as indicated by the maintainer for the file `src/Designer/frontend/packages/ux-editor-v3/src/components/toolbar/RuleModal.test.tsx`.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigCustomFileEnding.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigBooleanProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.tssrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigGridProperties.test.tsx
📚 Learning: 2025-10-25T11:48:49.152Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 16740
File: src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx:63-71
Timestamp: 2025-10-25T11:48:49.152Z
Learning: In src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx, the NameConfig component is intentionally rendered in two places: once in the mainConfig section and once inside the "Text" accordion section. This duplication is by design.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigCustomFileEnding.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigBooleanProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.tssrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigGridProperties.test.tsx
📚 Learning: 2025-11-25T08:21:14.748Z
Learnt from: CR
Repo: Altinn/altinn-studio PR: 0
File: src/App/frontend/CLAUDE.md:0-0
Timestamp: 2025-11-25T08:21:14.748Z
Learning: Applies to src/App/frontend/**/*.test.{ts,tsx} : Use `renderWithProviders` utility from `src/test/renderWithProviders.tsx` when testing components that require form layout context
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigCustomFileEnding.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigBooleanProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigGridProperties.test.tsx
📚 Learning: 2025-06-16T12:41:06.726Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15659
File: frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/CodeListDialog/CodeListDialog.test.tsx:145-151
Timestamp: 2025-06-16T12:41:06.726Z
Learning: In the Altinn Studio codebase, TomasEng has confirmed that using `expect(screen.getByRole('dialog')).toBeVisible` directly with `waitFor` is acceptable and equivalent to wrapping it in a function callback. This pattern is intentionally used in their testing setup.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigCustomFileEnding.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigBooleanProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts
📚 Learning: 2025-01-13T12:44:45.751Z
Learnt from: Jondyr
Repo: Altinn/altinn-studio PR: 14379
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/Summary2OverrideEntry.tsx:63-70
Timestamp: 2025-01-13T12:44:45.751Z
Learning: In the Altinn Studio codebase, when using StudioProperty.Button component, it's expected to pass `false` as the value prop when no meaningful value is available (e.g., when componentNameType is undefined). This is the intended behavior and should not be changed to handle undefined cases differently.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigBooleanProperties.test.tsx
📚 Learning: 2025-06-16T12:39:14.951Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15659
File: frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/CodeListDialog/CodeListDialog.test.tsx:146-149
Timestamp: 2025-06-16T12:39:14.951Z
Learning: The Altinn Studio project has HTMLDialogElement polyfills already configured in frontend/testing/setupTests.ts that mock showModal() to set this.open = true and close() to set this.open = false, so showModal() works correctly in Jest/JSDOM tests without needing additional polyfills.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts
📚 Learning: 2025-08-08T13:24:30.117Z
Learnt from: nkylstad
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-08-08T13:24:30.117Z
Learning: In Altinn/altinn-studio PR reviews, when unit tests need React Query/TanStack Query data, prefer seeding with QueryClient.setQueryData over rendering/mocking use…Query hooks and waiting for isSuccess. Flag patterns like renderHookWithMockStore(() => use…Query(...)) and waitFor(() => expect(result.current.isSuccess).toBe(true)) and recommend the simpler setQueryData approach. Cite the internal Confluence guideline on component tests and the best-practice example in useAddItemToLayoutMutation.test.ts (lines updated 08.08.2025).
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts
📚 Learning: 2025-02-10T11:28:12.444Z
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 14398
File: frontend/libs/studio-components/src/components/StudioCodelistEditor/StudioCodeListEditorRow/StudioCodeListEditorRow.tsx:128-136
Timestamp: 2025-02-10T11:28:12.444Z
Learning: The StudioCodeListEditor component's handling of undefined values (when StudioDecimalField is cleared) is verified through the test case that checks if a numberfield is rendered with inputMode='decimal'.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsx
📚 Learning: 2025-06-16T12:16:47.892Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15659
File: frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/OptionListEditor.tsx:44-47
Timestamp: 2025-06-16T12:16:47.892Z
Learning: In selection components within the Altinn Studio UX editor, the code design anticipates that one of the properties `options` and `optionsId` should always be set, representing manual options vs external option list references respectively. However, this mutual exclusivity is not well reflected in the current type system.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx
📚 Learning: 2025-07-02T09:01:58.097Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/Properties/Text.tsx:31-36
Timestamp: 2025-07-02T09:01:58.097Z
Learning: In the Altinn Studio UX editor, schema validation and property existence checks are handled at the ComponentConfigPanel level. The Text component is only rendered when textResourceBindings properties exist in the schema, making direct property access safe without additional null checks within the component itself.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsx
📚 Learning: 2025-07-02T09:02:34.292Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsx
📚 Learning: 2025-07-02T09:04:49.873Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/PropertiesHeader.tsx:28-30
Timestamp: 2025-07-02T09:04:49.873Z
Learning: In the Altinn Studio UX editor, the PropertiesHeader component can safely access schema.properties directly without null checks because the parent ComponentConfigPanel component handles the loading state and only renders PropertiesHeader after the schema is fetched via useComponentSchemaQuery.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsx
🧬 Code graph analysis (6)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigCustomFileEnding.test.tsx (1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts (1)
getPropertyByRole(5-9)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigBooleanProperties.test.tsx (1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts (1)
getPropertyByRole(5-9)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts (2)
src/Designer/frontend/packages/shared/src/mocks/mocks.ts (1)
user(160-167)src/Designer/development/setup.js (1)
waitFor(2-2)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx (1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts (4)
openConfigAndVerify(24-30)saveConfigChanges(11-16)getPropertyByRole(5-9)cancelConfigChanges(18-22)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsx (3)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts (4)
getPropertyByRole(5-9)openConfigAndVerify(24-30)saveConfigChanges(11-16)cancelConfigChanges(18-22)src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.tsx (1)
ConfigStringPropertiesProps(10-14)src/Designer/frontend/packages/ux-editor/src/testing/componentMocks.ts (1)
componentMocks(215-248)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigGridProperties.test.tsx (1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts (1)
getPropertyByRole(5-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Testing
- GitHub Check: CodeQL
- GitHub Check: Typechecking and linting
- GitHub Check: Build environment and run e2e test
🔇 Additional comments (18)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigCustomFileEnding.test.tsx (1)
8-8: LGTM! Consistent migration to test utilities.The migration from direct
screenqueries to the centralisedgetPropertyByRoleutility improves test maintainability and consistency across the test suite.Also applies to: 23-23, 46-46
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigBooleanProperties.test.tsx (1)
14-14: LGTM! Consistent adoption of test utilities.The refactoring to use
getPropertyByRoleacross all boolean property tests improves consistency and maintainability.Also applies to: 25-25, 61-61, 78-78, 91-91
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigGridProperties.test.tsx (3)
10-10: Typo fix in test description.Corrects "grid width text" to "grid with text".
22-26: Improved render helper API.The addition of the
RenderConfigGridPropertiesPropstype and optional props parameter with default value improves the helper's API and makes it more flexible.
35-41: Cleaner helper function.Refactoring
closeCardto create its ownuserEventand usegetPropertyByRoleeliminates the need to passUserEventas a parameter, improving the function's encapsulation.src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx (2)
7-14: Excellent adoption of test utilities.The comprehensive import of test utilities and introduction of the
defaultPropertyconstant reduces duplication and improves test maintainability.
38-68: Enhanced test coverage with save/cancel flows.The new tests for cancelling edits and saving values provide better coverage of the edit mode behaviour. The end-to-end interactions with
openConfigAndVerify,saveConfigChanges, andcancelConfigChangesmake the test suite more robust.src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts (4)
5-9: Well-designed element lookup utility.The
getPropertyByRolehelper centralises the pattern of finding elements by role and translated property name. The return typeHTMLElement | nullprovides flexibility for both existence checks and direct interactions (where tests will fail with clear errors if the element is missing).
11-16: Defensive async interaction helper.The
waitForwrapper ensures the save button is enabled before clicking, preventing flaky tests due to timing issues. Creating a localuserEventinstance is the appropriate pattern for reusable utilities.
18-22: Clean cancellation helper.Simple and focused utility for clicking the cancel button.
24-30: Good verification pattern.The
openConfigAndVerifyhelper encapsulates both the action (opening the config) and the verification (cancel button appears), making tests more declarative and easier to read.src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsx (4)
7-12: LGTM: Shared test utilities improve maintainability.The refactoring to use shared utilities from
testConfigUtilsis a positive change that reduces duplication and ensures consistent test patterns across config property tests.
36-60: Excellent test coverage for save and cancel workflows.The new tests for save and cancel functionality provide valuable coverage for the edit-mode workflow. The tests clearly verify that:
- Saving a changed value calls
handleComponentUpdatewith the updated property- Canceling closes the editor without persisting changes
63-74: LGTM: Simplified render helper with clearer props structure.The updated render helper with direct props merging (rather than nested structure) makes the test setup more straightforward and aligns with the component's refactored interface.
28-34: Replace 'displayMode' with a valid Input component property, as it does not exist in the Input schema.The test passes
'displayMode'as a stringPropertyKey, but this property is not defined in Input.schema.v1.json or componentMocks.Input. ThedisplayModeproperty is specific to FileUpload and FileUploadWithTag components. WhenkeepEditOpenis true, the component attempts to look upschema.properties['displayMode'], which would be undefined, potentially causing unexpected behaviour when the test is expanded.src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx (3)
5-13: LGTM: Appropriate imports for refactored test patterns.The addition of
withinfor scoped queries and the shared test utilities aligns well with the updated test interaction patterns.
16-35: Well-structured test with clear verification of UI state.The refactored test effectively uses the new utilities and adds proper verification of the button content using
within()to ensure the selected value is correctly displayed after saving. This is a good improvement over the previous implementation.
84-90: Excellent addition of cancel workflow test.This test provides valuable coverage for the cancel functionality, ensuring that clicking cancel properly closes the editor without persisting changes. The test follows the same clear pattern established in
ConfigStringProperties.test.tsx.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx (1)
70-78: Schema structure issue: InputSchema properties are being replaced.The
propertiesobject is being defined without spreadingInputSchema.properties, which means all properties from the InputSchema are lost and onlysomeNumberPropertyremains. This should extend rather than replace the original properties.🔎 Proposed fix
schema: { ...InputSchema, properties: { + ...InputSchema.properties, someNumberProperty: { type: 'number', enum: [1, 2, 3], }, }, },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsx
- src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts
- src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: framitdavid
Repo: Altinn/altinn-studio PR: 16372
File: src/Designer/frontend/libs/studio-components/src/components/index.ts:34-34
Timestamp: 2025-09-23T08:53:19.508Z
Learning: In the Altinn Studio codebase, when moving components from studio-components-legacy to studio-components, the team prefers to handle the component migration and remaining import updates in separate PRs to maintain focused, atomic changes.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16095
File: src/App/app-template-dotnet/src/App/models/model.cs:10-29
Timestamp: 2025-08-15T05:53:32.529Z
Learning: When moving app templates in Altinn Studio, mirkoSekulic prefers to keep the PR scope strictly limited to the move operation itself, with all content changes (including code quality improvements, naming convention fixes, and attribute cleanup) deferred to separate PRs for better maintainability and atomic changes.
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-10T12:02:06.348Z
Learning: In the Altinn Studio repository, the team convention is to remove irrelevant checkboxes from the PR template description rather than leaving them unmarked. This makes it clear that the author has intentionally considered each item and removed non-applicable ones, helping to avoid merging PRs with incomplete checklists.
Learnt from: mgunnerud
Repo: Altinn/altinn-studio PR: 16187
File: frontend/resourceadm/components/ResourcePageInputs/ResourceSwitchInput.tsx:3-3
Timestamp: 2025-08-25T13:55:27.042Z
Learning: In the Altinn Studio codebase, when migrating from studio/components-legacy to studio/components, PRs should focus only on components that have available replacements in the new package. Components without new implementations should remain in the legacy package until replacements are available, supporting an incremental migration approach.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16191
File: .dockerignore:3-3
Timestamp: 2025-08-26T12:35:44.069Z
Learning: In the Altinn Studio repository, mirkoSekulic is implementing a phased directory restructuring approach where backend and testdata are moved first, followed by separate moves for readme, frontend, and development directories into the Designer directory. This ensures atomic changes and prevents path reference issues during the transition.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 14575
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/OverrideFields/Summary2OverrideDisplayType.tsx:47-58
Timestamp: 2025-02-07T09:50:33.290Z
Learning: Avoid suggesting StudioCombobox component in future PRs for the Altinn Studio repository. A new replacement component will be introduced later.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15981
File: frontend/libs/studio-components/src/components/StudioLabelAsParagraph/StudioLabelAsParagraph.tsx:10-10
Timestamp: 2025-08-07T13:08:11.913Z
Learning: In the Altinn Studio codebase, when using components with the `asChild` prop pattern (like from digdir/designsystemet-react), the ref should be placed on the child DOM element rather than forwarded to the parent component. This pattern was established in PR #15998 where Card asChild was used with StudioFieldset, and the ref was passed to the StudioFieldset (child) not the Card (parent). This approach provides correct TypeScript typing automatically since the ref points directly to the actual DOM element being rendered.
Learnt from: Konrad-Simso
Repo: Altinn/altinn-studio PR: 14952
File: frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/ManualOptionsEditor/ManualOptionsEditor.test.tsx:57-57
Timestamp: 2025-03-12T14:19:41.785Z
Learning: TODO comments in the codebase mentioning "Todo: Replace 'close modal' with defaultDialogProps.closeButtonTitle" will be addressed when upgrading to Designsystemet v1, which is currently being worked on.
Learnt from: Konrad-Simso
Repo: Altinn/altinn-studio PR: 14952
File: frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/ManualOptionsEditor/ManualOptionsEditor.test.tsx:57-57
Timestamp: 2025-03-12T14:19:43.129Z
Learning: TODO comments in the codebase mentioning "Todo: Replace 'close modal' with defaultDialogProps.closeButtonTitle" will be addressed when upgrading to Designsystemet v1, which is currently being worked on.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 16123
File: frontend/libs/studio-components/src/types/OverridableComponentProps.ts:3-6
Timestamp: 2025-08-15T08:21:51.564Z
Learning: ErlingHauan prefers to keep PRs that move types from studio-components-legacy to studio-components focused strictly on the move operation, deferring type implementation changes or improvements to separate PRs for better maintainability and atomic changes.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
📚 Learning: 2025-02-10T11:28:12.444Z
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 14398
File: frontend/libs/studio-components/src/components/StudioCodelistEditor/StudioCodeListEditorRow/StudioCodeListEditorRow.tsx:128-136
Timestamp: 2025-02-10T11:28:12.444Z
Learning: The StudioCodeListEditor component's handling of undefined values (when StudioDecimalField is cleared) is verified through the test case that checks if a numberfield is rendered with inputMode='decimal'.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
📚 Learning: 2025-10-28T20:54:18.494Z
Learnt from: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16787
File: src/Designer/frontend/packages/ux-editor-v3/src/components/toolbar/RuleModal.test.tsx:70-70
Timestamp: 2025-10-28T20:54:18.494Z
Learning: Test inconsistencies in the ux-editor-v3 package do not need to be fixed, as indicated by the maintainer for the file `src/Designer/frontend/packages/ux-editor-v3/src/components/toolbar/RuleModal.test.tsx`.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
📚 Learning: 2025-10-25T11:48:49.152Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 16740
File: src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx:63-71
Timestamp: 2025-10-25T11:48:49.152Z
Learning: In src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx, the NameConfig component is intentionally rendered in two places: once in the mainConfig section and once inside the "Text" accordion section. This duplication is by design.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
🧬 Code graph analysis (1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx (2)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts (4)
getPropertyByRole(5-9)openConfigAndVerify(25-31)cancelConfigAndVerify(18-23)saveConfigChanges(11-16)src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx (1)
ConfigNumberPropertiesProps(9-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Testing
- GitHub Check: Typechecking and linting
- GitHub Check: CodeQL
- GitHub Check: Build environment and run e2e test
🔇 Additional comments (2)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx (2)
7-14: LGTM! Good use of shared test utilities.The introduction of the test utilities and the
defaultPropertyconstant improves test maintainability and consistency.
31-42: LGTM! Good test coverage for edit and cancel flows.These tests appropriately verify the
keepEditOpenbehaviour and the cancel functionality using the new test utilities.
...nd/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
Outdated
Show resolved
Hide resolved
...nd/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx (1)
24-29: Consider making the test name singular to match what's tested.The test name mentions "components" (plural), but the test only verifies a single combobox for
defaultProperty. Since only one property is being tested, consider updating the name to "should render EditNumberValue component when keepEditOpen is true" for clarity.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: framitdavid
Repo: Altinn/altinn-studio PR: 16372
File: src/Designer/frontend/libs/studio-components/src/components/index.ts:34-34
Timestamp: 2025-09-23T08:53:19.508Z
Learning: In the Altinn Studio codebase, when moving components from studio-components-legacy to studio-components, the team prefers to handle the component migration and remaining import updates in separate PRs to maintain focused, atomic changes.
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-10T12:02:06.348Z
Learning: In the Altinn Studio repository, the team convention is to remove irrelevant checkboxes from the PR template description rather than leaving them unmarked. This makes it clear that the author has intentionally considered each item and removed non-applicable ones, helping to avoid merging PRs with incomplete checklists.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16095
File: src/App/app-template-dotnet/src/App/models/model.cs:10-29
Timestamp: 2025-08-15T05:53:32.529Z
Learning: When moving app templates in Altinn Studio, mirkoSekulic prefers to keep the PR scope strictly limited to the move operation itself, with all content changes (including code quality improvements, naming convention fixes, and attribute cleanup) deferred to separate PRs for better maintainability and atomic changes.
Learnt from: mgunnerud
Repo: Altinn/altinn-studio PR: 16187
File: frontend/resourceadm/components/ResourcePageInputs/ResourceSwitchInput.tsx:3-3
Timestamp: 2025-08-25T13:55:27.042Z
Learning: In the Altinn Studio codebase, when migrating from studio/components-legacy to studio/components, PRs should focus only on components that have available replacements in the new package. Components without new implementations should remain in the legacy package until replacements are available, supporting an incremental migration approach.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 14575
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/OverrideFields/Summary2OverrideDisplayType.tsx:47-58
Timestamp: 2025-02-07T09:50:33.290Z
Learning: Avoid suggesting StudioCombobox component in future PRs for the Altinn Studio repository. A new replacement component will be introduced later.
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15719
File: frontend/dashboard/pages/OrgContentLibraryPage/FeedbackForm/FeedbackForm.tsx:22-39
Timestamp: 2025-06-20T13:41:45.278Z
Learning: In the Altinn Studio project, temporary feedback form components may intentionally use hardcoded text instead of internationalization when they are designed to be removed before language support is implemented.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16191
File: .dockerignore:3-3
Timestamp: 2025-08-26T12:35:44.069Z
Learning: In the Altinn Studio repository, mirkoSekulic is implementing a phased directory restructuring approach where backend and testdata are moved first, followed by separate moves for readme, frontend, and development directories into the Designer directory. This ensures atomic changes and prevents path reference issues during the transition.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 16123
File: frontend/libs/studio-components/src/types/OverridableComponentProps.ts:3-6
Timestamp: 2025-08-15T08:21:51.564Z
Learning: ErlingHauan prefers to keep PRs that move types from studio-components-legacy to studio-components focused strictly on the move operation, deferring type implementation changes or improvements to separate PRs for better maintainability and atomic changes.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15981
File: frontend/libs/studio-components/src/components/StudioLabelAsParagraph/StudioLabelAsParagraph.tsx:10-10
Timestamp: 2025-08-07T13:08:11.913Z
Learning: In the Altinn Studio codebase, when using components with the `asChild` prop pattern (like from digdir/designsystemet-react), the ref should be placed on the child DOM element rather than forwarded to the parent component. This pattern was established in PR #15998 where Card asChild was used with StudioFieldset, and the ref was passed to the StudioFieldset (child) not the Card (parent). This approach provides correct TypeScript typing automatically since the ref points directly to the actual DOM element being rendered.
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 16064
File: frontend/libs/studio-components/src/components/StudioCheckboxGroup/StudioGetCheckboxProps.ts:1-1
Timestamp: 2025-08-12T05:59:20.146Z
Learning: In the Altinn Studio codebase, deep exports from digdir/designsystemet-react should be avoided. The team prefers using shared wrappers for design system imports to prevent breakage from upstream internal path changes, but such refactoring should be done in separate PRs to maintain focused scope.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/PropertiesHeader.tsx:28-30
Timestamp: 2025-07-02T09:04:49.873Z
Learning: In the Altinn Studio UX editor, the PropertiesHeader component can safely access schema.properties directly without null checks because the parent ComponentConfigPanel component handles the loading state and only renders PropertiesHeader after the schema is fetched via useComponentSchemaQuery.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/PropertiesHeader.tsx:28-30
Timestamp: 2025-07-02T09:04:49.873Z
Learning: In the Altinn Studio UX editor, the PropertiesHeader component can safely access schema.properties directly without null checks because the parent ComponentConfigPanel component handles the loading state and only renders PropertiesHeader after the schema is fetched via useComponentSchemaQuery.
📚 Learning: 2025-02-10T11:28:12.444Z
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 14398
File: frontend/libs/studio-components/src/components/StudioCodelistEditor/StudioCodeListEditorRow/StudioCodeListEditorRow.tsx:128-136
Timestamp: 2025-02-10T11:28:12.444Z
Learning: The StudioCodeListEditor component's handling of undefined values (when StudioDecimalField is cleared) is verified through the test case that checks if a numberfield is rendered with inputMode='decimal'.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
📚 Learning: 2025-10-28T20:54:18.494Z
Learnt from: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16787
File: src/Designer/frontend/packages/ux-editor-v3/src/components/toolbar/RuleModal.test.tsx:70-70
Timestamp: 2025-10-28T20:54:18.494Z
Learning: Test inconsistencies in the ux-editor-v3 package do not need to be fixed, as indicated by the maintainer for the file `src/Designer/frontend/packages/ux-editor-v3/src/components/toolbar/RuleModal.test.tsx`.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
📚 Learning: 2025-10-25T11:48:49.152Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 16740
File: src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx:63-71
Timestamp: 2025-10-25T11:48:49.152Z
Learning: In src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx, the NameConfig component is intentionally rendered in two places: once in the mainConfig section and once inside the "Text" accordion section. This duplication is by design.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
📚 Learning: 2025-01-27T14:25:30.012Z
Learnt from: nkylstad
Repo: Altinn/altinn-studio PR: 14516
File: frontend/packages/ux-editor/src/testing/schemas/json/component/PersonLookup.schema.v1.json:50-55
Timestamp: 2025-01-27T14:25:30.012Z
Learning: Schema files in the `frontend/packages/ux-editor/src/testing/schemas/json/` directory are auto-generated via script. Any manual changes to these files will be overwritten when the schema generation script is run. These files should be excluded from refactoring suggestions.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
📚 Learning: 2025-02-10T11:46:29.320Z
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 14398
File: frontend/libs/studio-components/src/components/StudioCodelistEditor/StudioCodeListEditorRow/StudioCodeListEditorRow.tsx:0-0
Timestamp: 2025-02-10T11:46:29.320Z
Learning: The Numberfield component handles number validation internally and only calls onChange with valid number or undefined values, making additional validation in parent components unnecessary.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
📚 Learning: 2025-09-23T08:53:19.508Z
Learnt from: framitdavid
Repo: Altinn/altinn-studio PR: 16372
File: src/Designer/frontend/libs/studio-components/src/components/index.ts:34-34
Timestamp: 2025-09-23T08:53:19.508Z
Learning: In the Altinn Studio codebase, when moving components from studio-components-legacy to studio-components, the team prefers to handle the component migration and remaining import updates in separate PRs to maintain focused, atomic changes.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
📚 Learning: 2025-08-08T13:24:30.117Z
Learnt from: nkylstad
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-08-08T13:24:30.117Z
Learning: In Altinn/altinn-studio PR reviews, when unit tests need React Query/TanStack Query data, prefer seeding with QueryClient.setQueryData over rendering/mocking use…Query hooks and waiting for isSuccess. Flag patterns like renderHookWithMockStore(() => use…Query(...)) and waitFor(() => expect(result.current.isSuccess).toBe(true)) and recommend the simpler setQueryData approach. Cite the internal Confluence guideline on component tests and the best-practice example in useAddItemToLayoutMutation.test.ts (lines updated 08.08.2025).
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
📚 Learning: 2025-08-07T13:08:11.913Z
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15981
File: frontend/libs/studio-components/src/components/StudioLabelAsParagraph/StudioLabelAsParagraph.tsx:10-10
Timestamp: 2025-08-07T13:08:11.913Z
Learning: In the Altinn Studio codebase, when using components with the `asChild` prop pattern (like from digdir/designsystemet-react), the ref should be placed on the child DOM element rather than forwarded to the parent component. This pattern was established in PR #15998 where Card asChild was used with StudioFieldset, and the ref was passed to the StudioFieldset (child) not the Card (parent). This approach provides correct TypeScript typing automatically since the ref points directly to the actual DOM element being rendered.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
📚 Learning: 2025-08-15T08:21:51.564Z
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 16123
File: frontend/libs/studio-components/src/types/OverridableComponentProps.ts:3-6
Timestamp: 2025-08-15T08:21:51.564Z
Learning: ErlingHauan prefers to keep PRs that move types from studio-components-legacy to studio-components focused strictly on the move operation, deferring type implementation changes or improvements to separate PRs for better maintainability and atomic changes.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
📚 Learning: 2025-08-06T12:37:01.381Z
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:37:01.381Z
Learning: When reviewing PRs in the Altinn Studio repository where a developer copies a component from `studio-components-legacy` to `studio-components`, suggest adding a `deprecated` JSDoc comment to the component in `studio-components-legacy` if one hasn't been added. The deprecation comment should recommend using the component from `studio/components` instead.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
📚 Learning: 2025-08-06T13:04:34.007Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15876
File: frontend/libs/studio-components/src/components/StudioProperty/StudioProperty.test.tsx:6-6
Timestamp: 2025-08-06T13:04:34.007Z
Learning: User TomasEng prefers to keep component copying PRs focused on their specific scope without including additional fixes or improvements that are outside the copying objective, even when those fixes address valid best practices concerns.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
📚 Learning: 2025-09-10T12:02:06.348Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-10T12:02:06.348Z
Learning: In the Altinn Studio repository, the team convention is to remove irrelevant checkboxes from the PR template description rather than leaving them unmarked. This makes it clear that the author has intentionally considered each item and removed non-applicable ones, helping to avoid merging PRs with incomplete checklists.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
📚 Learning: 2025-09-26T12:03:51.850Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-26T12:03:51.850Z
Learning: In PR #16443, TomasEng clarified that the ExpressionTexts type in studio/components does not require additional keys like dataModel, resource, or missingDataModelLabel that were suggested in a review comment. TypeScript compilation would catch any missing required properties, confirming the migration is correctly implemented.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
🧬 Code graph analysis (1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx (2)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts (4)
getPropertyByRole(5-9)openConfigAndVerify(25-31)cancelConfigAndVerify(18-23)saveConfigChanges(11-16)src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.tsx (1)
ConfigNumberPropertiesProps(9-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: CodeQL
- GitHub Check: Typechecking and linting
- GitHub Check: Testing
- GitHub Check: Build environment and run e2e test
🔇 Additional comments (1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx (1)
7-12: Excellent refactoring with shared test utilities!The migration to use
testConfigUtilssignificantly improves test maintainability and consistency across config property tests. The expanded coverage for open, cancel, and save flows provides better confidence in the component behaviour.Also applies to: 31-35, 37-61
...nd/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
Show resolved
Hide resolved
4 tasks
Jondyr
approved these changes
Jan 7, 2026
Member
Jondyr
left a comment
Jondyr
left a comment
There was a problem hiding this comment.
LGTM ✅
Added one comment about testing library userEvent.
...signer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts
Outdated
Show resolved
Hide resolved
…ub.com/Altinn/altinn-studio into replace-inline-actions-in-config-panel
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx:
- Around line 20-35: The test is passing nullable HTMLElements from
getPropertyByRole into user-event/within which can receive null; update the
helper getPropertyByRole so it uses screen.getByRole (or otherwise throws) and
returns HTMLElement instead of HTMLElement | null, then update usages in this
test (and at lines 109-119) to rely on the non-null return (e.g., keep calls to
openConfigAndVerify, selectOption, saveConfigChanges unchanged but remove any
null checks) so interactions like user.click(combobox) and
within(selectedValueDisplay) always receive a real element.
🧹 Nitpick comments (2)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx (1)
88-93: Strengthen the “cancel closes editor” assertion.
cancelConfigAndVerifyonly proves the Cancel button disappeared; it doesn’t explicitly prove the editor closed. Consider asserting the combobox is gone afterwards.Proposed test tightening
it('should close the select editor when clicking cancel button', async () => { const user = userEvent.setup(); renderConfigArrayProperties({}); await openConfigAndVerify({ user, property: supportedKey }); await cancelConfigAndVerify(user); + expect( + screen.queryByRole('combobox', { + name: textMock(`ux_editor.component_properties.${supportedKey}`), + }), + ).not.toBeInTheDocument(); });src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsx (1)
23-26: Make the “renders nothing” and “cancel” tests more specific (and fix wording).
screen.queryByRole('button')is very broad and can become flaky if providers/layout render unrelated buttons.- The cancel test refers to a “select editor” (but this is a text editor).
Proposed adjustments
import InputSchema from '../../../testing/schemas/json/component/Input.schema.v1.json'; import { screen } from '@testing-library/react'; +import { textMock } from '@studio/testing/mocks/i18nMock'; import { cancelConfigAndVerify, getPropertyByRole, openConfigAndVerify, saveConfigChanges, } from './testConfigUtils'; import userEvent from '@testing-library/user-event'; @@ it('should not render anything when stringPropertyKeys is empty', () => { renderConfigStringProperties({ stringPropertyKeys: [] }); - expect(screen.queryByRole('button')).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { + name: textMock(`ux_editor.component_properties.${defaultProperty}`), + }), + ).not.toBeInTheDocument(); }); @@ -it('should close the select editor when clicking cancel button', async () => { +it('should close the text editor when clicking cancel button', async () => { const user = userEvent.setup(); renderConfigStringProperties(); await openConfigAndVerify({ user, property: defaultProperty }); await cancelConfigAndVerify(user); });Also applies to: 54-59
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigNumberProperties.test.tsx
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: framitdavid
Repo: Altinn/altinn-studio PR: 16372
File: src/Designer/frontend/libs/studio-components/src/components/index.ts:34-34
Timestamp: 2025-09-23T08:53:19.508Z
Learning: In the Altinn Studio codebase, when moving components from studio-components-legacy to studio-components, the team prefers to handle the component migration and remaining import updates in separate PRs to maintain focused, atomic changes.
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-09-10T12:02:06.348Z
Learning: In the Altinn Studio repository, the team convention is to remove irrelevant checkboxes from the PR template description rather than leaving them unmarked. This makes it clear that the author has intentionally considered each item and removed non-applicable ones, helping to avoid merging PRs with incomplete checklists.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16095
File: src/App/app-template-dotnet/src/App/models/model.cs:10-29
Timestamp: 2025-08-15T05:53:32.529Z
Learning: When moving app templates in Altinn Studio, mirkoSekulic prefers to keep the PR scope strictly limited to the move operation itself, with all content changes (including code quality improvements, naming convention fixes, and attribute cleanup) deferred to separate PRs for better maintainability and atomic changes.
Learnt from: mgunnerud
Repo: Altinn/altinn-studio PR: 16187
File: frontend/resourceadm/components/ResourcePageInputs/ResourceSwitchInput.tsx:3-3
Timestamp: 2025-08-25T13:55:27.042Z
Learning: In the Altinn Studio codebase, when migrating from studio/components-legacy to studio/components, PRs should focus only on components that have available replacements in the new package. Components without new implementations should remain in the legacy package until replacements are available, supporting an incremental migration approach.
Learnt from: mirkoSekulic
Repo: Altinn/altinn-studio PR: 16191
File: .dockerignore:3-3
Timestamp: 2025-08-26T12:35:44.069Z
Learning: In the Altinn Studio repository, mirkoSekulic is implementing a phased directory restructuring approach where backend and testdata are moved first, followed by separate moves for readme, frontend, and development directories into the Designer directory. This ensures atomic changes and prevents path reference issues during the transition.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 14575
File: frontend/packages/ux-editor/src/components/config/componentSpecificContent/Summary2/Override/OverrideFields/Summary2OverrideDisplayType.tsx:47-58
Timestamp: 2025-02-07T09:50:33.290Z
Learning: Avoid suggesting StudioCombobox component in future PRs for the Altinn Studio repository. A new replacement component will be introduced later.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15977
File: frontend/libs/studio-components-legacy/src/components/StudioCard/index.tsx:2-7
Timestamp: 2025-08-06T12:36:57.397Z
Learning: When a developer copies a component from `studio-components-legacy` to `studio-components` in the Altinn Studio repository and has not added a deprecation comment for the component in `studio-components-legacy`, suggest adding the deprecation comment in the PR review.
Learnt from: ErlingHauan
Repo: Altinn/altinn-studio PR: 15981
File: frontend/libs/studio-components/src/components/StudioLabelAsParagraph/StudioLabelAsParagraph.tsx:10-10
Timestamp: 2025-08-07T13:08:11.913Z
Learning: In the Altinn Studio codebase, when using components with the `asChild` prop pattern (like from digdir/designsystemet-react), the ref should be placed on the child DOM element rather than forwarded to the parent component. This pattern was established in PR #15998 where Card asChild was used with StudioFieldset, and the ref was passed to the StudioFieldset (child) not the Card (parent). This approach provides correct TypeScript typing automatically since the ref points directly to the actual DOM element being rendered.
Learnt from: Konrad-Simso
Repo: Altinn/altinn-studio PR: 14952
File: frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/ManualOptionsEditor/ManualOptionsEditor.test.tsx:57-57
Timestamp: 2025-03-12T14:19:43.129Z
Learning: TODO comments in the codebase mentioning "Todo: Replace 'close modal' with defaultDialogProps.closeButtonTitle" will be addressed when upgrading to Designsystemet v1, which is currently being worked on.
Learnt from: Konrad-Simso
Repo: Altinn/altinn-studio PR: 14952
File: frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/ManualOptionsEditor/ManualOptionsEditor.test.tsx:57-57
Timestamp: 2025-03-12T14:19:41.785Z
Learning: TODO comments in the codebase mentioning "Todo: Replace 'close modal' with defaultDialogProps.closeButtonTitle" will be addressed when upgrading to Designsystemet v1, which is currently being worked on.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 16740
File: src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx:63-71
Timestamp: 2025-10-25T11:48:49.152Z
Learning: In src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx, the NameConfig component is intentionally rendered in two places: once in the mainConfig section and once inside the "Text" accordion section. This duplication is by design.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/PropertiesHeader.tsx:28-30
Timestamp: 2025-07-02T09:04:49.873Z
Learning: In the Altinn Studio UX editor, the PropertiesHeader component can safely access schema.properties directly without null checks because the parent ComponentConfigPanel component handles the loading state and only renders PropertiesHeader after the schema is fetched via useComponentSchemaQuery.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/PropertiesHeader.tsx:28-30
Timestamp: 2025-07-02T09:04:49.873Z
Learning: In the Altinn Studio UX editor, the PropertiesHeader component can safely access schema.properties directly without null checks because the parent ComponentConfigPanel component handles the loading state and only renders PropertiesHeader after the schema is fetched via useComponentSchemaQuery.
Learnt from: nkylstad
Repo: Altinn/altinn-studio PR: 15550
File: frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/SpecificMainConfig/ImageMainConfig.test.tsx:17-17
Timestamp: 2025-05-30T14:19:53.714Z
Learning: In the SpecificMainConfig test files, the standard pattern is to use 'ComponentMainConfig' as the top-level describe block name, with nested describe blocks named after the specific component type being tested (e.g., 'Image', 'Options', 'Summary2').
📚 Learning: 2025-10-25T11:48:49.152Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 16740
File: src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx:63-71
Timestamp: 2025-10-25T11:48:49.152Z
Learning: In src/Designer/frontend/packages/ux-editor/src/components/Properties/PageConfigPanel/PageConfigPanel.tsx, the NameConfig component is intentionally rendered in two places: once in the mainConfig section and once inside the "Text" accordion section. This duplication is by design.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.tssrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx
📚 Learning: 2025-10-28T20:54:18.494Z
Learnt from: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16787
File: src/Designer/frontend/packages/ux-editor-v3/src/components/toolbar/RuleModal.test.tsx:70-70
Timestamp: 2025-10-28T20:54:18.494Z
Learning: Test inconsistencies in the ux-editor-v3 package do not need to be fixed, as indicated by the maintainer for the file `src/Designer/frontend/packages/ux-editor-v3/src/components/toolbar/RuleModal.test.tsx`.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.tssrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx
📚 Learning: 2025-11-25T08:21:14.748Z
Learnt from: CR
Repo: Altinn/altinn-studio PR: 0
File: src/App/frontend/CLAUDE.md:0-0
Timestamp: 2025-11-25T08:21:14.748Z
Learning: Applies to src/App/frontend/**/*.test.{ts,tsx} : Use `renderWithProviders` utility from `src/test/renderWithProviders.tsx` when testing components that require form layout context
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx
📚 Learning: 2025-07-02T09:01:58.097Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/Properties/Text.tsx:31-36
Timestamp: 2025-07-02T09:01:58.097Z
Learning: In the Altinn Studio UX editor, schema validation and property existence checks are handled at the ComponentConfigPanel level. The Text component is only rendered when textResourceBindings properties exist in the schema, making direct property access safe without additional null checks within the component itself.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsx
📚 Learning: 2025-07-02T09:02:34.292Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/config/EditFormComponent.tsx:26-31
Timestamp: 2025-07-02T09:02:34.292Z
Learning: In the UX Editor, ComponentConfigPanel acts as a higher-level component that uses useComponentSchemaQuery and conditionally renders its content. While the schema is loading (isFetchingSchema is true), it shows a StudioSpinner instead of rendering child components like EditFormComponent. This means EditFormComponent is only rendered when the schema is available, eliminating the need for null checks within EditFormComponent itself.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsx
📚 Learning: 2025-07-02T09:04:49.873Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15814
File: frontend/packages/ux-editor/src/components/Properties/PropertiesHeader/PropertiesHeader.tsx:28-30
Timestamp: 2025-07-02T09:04:49.873Z
Learning: In the Altinn Studio UX editor, the PropertiesHeader component can safely access schema.properties directly without null checks because the parent ComponentConfigPanel component handles the loading state and only renders PropertiesHeader after the schema is fetched via useComponentSchemaQuery.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsx
📚 Learning: 2025-08-08T13:24:30.117Z
Learnt from: nkylstad
Repo: Altinn/altinn-studio PR: 0
File: :0-0
Timestamp: 2025-08-08T13:24:30.117Z
Learning: In Altinn/altinn-studio PR reviews, when unit tests need React Query/TanStack Query data, prefer seeding with QueryClient.setQueryData over rendering/mocking use…Query hooks and waiting for isSuccess. Flag patterns like renderHookWithMockStore(() => use…Query(...)) and waitFor(() => expect(result.current.isSuccess).toBe(true)) and recommend the simpler setQueryData approach. Cite the internal Confluence guideline on component tests and the best-practice example in useAddItemToLayoutMutation.test.ts (lines updated 08.08.2025).
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts
📚 Learning: 2025-06-16T12:41:06.726Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15659
File: frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/CodeListDialog/CodeListDialog.test.tsx:145-151
Timestamp: 2025-06-16T12:41:06.726Z
Learning: In the Altinn Studio codebase, TomasEng has confirmed that using `expect(screen.getByRole('dialog')).toBeVisible` directly with `waitFor` is acceptable and equivalent to wrapping it in a function callback. This pattern is intentionally used in their testing setup.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts
📚 Learning: 2025-06-16T12:39:14.951Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 15659
File: frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/CodeListDialog/CodeListDialog.test.tsx:146-149
Timestamp: 2025-06-16T12:39:14.951Z
Learning: The Altinn Studio project has HTMLDialogElement polyfills already configured in frontend/testing/setupTests.ts that mock showModal() to set this.open = true and close() to set this.open = false, so showModal() works correctly in Jest/JSDOM tests without needing additional polyfills.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts
🧬 Code graph analysis (3)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.test.tsx (2)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts (4)
getPropertyByRole(5-9)openConfigAndVerify(28-33)saveConfigChanges(11-15)cancelConfigAndVerify(17-21)src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigStringProperties.tsx (1)
ConfigStringPropertiesProps(10-14)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts (2)
src/Designer/frontend/packages/shared/src/mocks/mocks.ts (1)
user(160-167)src/Designer/development/setup.js (1)
waitFor(2-2)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx (1)
src/Designer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts (4)
openConfigAndVerify(28-33)saveConfigChanges(11-15)getPropertyByRole(5-9)cancelConfigAndVerify(17-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: CodeQL
- GitHub Check: Build environment and run e2e test
- GitHub Check: Typechecking and linting
- GitHub Check: Testing
...end/packages/ux-editor/src/components/config/ConfigProperties/ConfigArrayProperties.test.tsx
Show resolved
Hide resolved
...signer/frontend/packages/ux-editor/src/components/config/ConfigProperties/testConfigUtils.ts
Show resolved
Hide resolved
Contributor
JamalAlabdullah
left a comment
JamalAlabdullah
left a comment
There was a problem hiding this comment.
I tested it , it works fine for every input selv, but it seems like they effect in each other see videos;
Screen.Recording.2026-01-09.at.10.15.12.mov
Screen.Recording.2026-01-09.at.10.16.21.mov
Screen.Recording.2026-01-09.at.10.18.59.mov
Screen.Recording.2026-01-09.at.10.22.05.mov
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
editName(edit id for page and group) to make it similar to the other inline configsImage before
Image after
The buttons is still not 100% inline with the input-element, this needs bigger refactors to solve since the input element is integrated as a parent that contains label, input-element and possible error message, from the Designsystem. A further solution would be to separate the error message from the component
Verification
Summary by CodeRabbit
New Features
Improved UX
Style
Tests
✏️ Tip: You can customize this high-level summary in your review settings.