chore: replace imageupload card with studioconfigcard #16967
+63
−71
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
lassopicasso
added
the
area/ui-editor
github-actions
bot
added
skip-releasenotes
Contributor
WalkthroughRefactors ImageUploadCard to use StudioConfigCard and inline save/cancel/delete handlers; widens Changes
Sequence DiagramsequenceDiagram
participant User
participant ImageUploadCard
participant StudioConfigCard
participant handleSaveChanges
User->>ImageUploadCard: Open card / edit crop
alt Save
User->>StudioConfigCard: Click "Save"
StudioConfigCard->>ImageUploadCard: invoke handleSave()
ImageUploadCard->>ImageUploadCard: getCropToBeSaved(internalCrop)
ImageUploadCard->>handleSaveChanges: handleSaveChanges(externalCrop)
ImageUploadCard->>ImageUploadCard: setOpenCard(false)
else Cancel
User->>StudioConfigCard: Click "Cancel"
StudioConfigCard->>ImageUploadCard: invoke handleCancel()
ImageUploadCard->>ImageUploadCard: setOpenCard(false)
else Delete
User->>StudioConfigCard: Click "Delete"
StudioConfigCard->>User: window.confirm()
User-->>StudioConfigCard: Confirm
StudioConfigCard->>ImageUploadCard: invoke handleDelete()
ImageUploadCard->>handleSaveChanges: handleSaveChanges(null)
ImageUploadCard->>ImageUploadCard: setOpenCard(false)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-10-28T20:54:18.494ZApplied to files:
📚 Learning: 2025-10-26T21:09:38.402ZApplied to files:
📚 Learning: 2025-11-25T08:21:14.748ZApplied to files:
⏰ 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). (5)
🔇 Additional comments (1)
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: 1
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/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.tsx (1)
17-17: Update the type signature to accept null.The
handleSaveChangesparameter type should be(CropToBeSaved: ExternalCrop | null) => voidto match the delete functionality on line 47, where it's called withnull. This type mismatch could cause TypeScript errors or unexpected behaviour.Apply this diff to fix the type:
- handleSaveChanges: (CropToBeSaved: ExternalCrop) => void; + handleSaveChanges: (CropToBeSaved: ExternalCrop | null) => void;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/Designer/frontend/language/src/nb.json(1 hunks)src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadActions.module.css(0 hunks)src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadActions.tsx(0 hunks)src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.test.tsx(1 hunks)src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.tsx(2 hunks)src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadComponent.tsx(1 hunks)
💤 Files with no reviewable changes (2)
- src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadActions.module.css
- src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadActions.tsx
🧰 Additional context used
🧠 Learnings (9)
📓 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: 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: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16240
File: src/Designer/frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/ColumnElement/EditColumnElement/EditColumnElement.tsx:85-116
Timestamp: 2025-09-02T10:47:19.978Z
Learning: In the Altinn Studio codebase, when migrating StudioCard usage to designsystemet v1, avoid using StudioCard.Block if StudioCard already has a border, as this creates unwanted double border effects. The team prioritizes clean visual design over strict adherence to component patterns when there are visual conflicts.
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: 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: 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: 16662
File: src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadUtils.ts:44-54
Timestamp: 2025-10-20T07:58:25.550Z
Learning: In the ImageUpload component in `src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/`, validation errors for all crop size fields (width, height, diameter) should persist when users switch between shapes to provide better UX feedback when they switch back.
📚 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/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.test.tsx
📚 Learning: 2025-10-20T07:58:25.550Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 16662
File: src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadUtils.ts:44-54
Timestamp: 2025-10-20T07:58:25.550Z
Learning: In the ImageUpload component in `src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/`, validation errors for all crop size fields (width, height, diameter) should persist when users switch between shapes to provide better UX feedback when they switch back.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.tsxsrc/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadComponent.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/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.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/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.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/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.tsx
📚 Learning: 2025-06-03T10:54:59.508Z
Learnt from: wrt95
Repo: Altinn/altinn-studio PR: 15542
File: frontend/app-development/features/appSettings/components/TabsContent/Tabs/AboutTab/utils/appResourceValidationUtils.ts:64-95
Timestamp: 2025-06-03T10:54:59.508Z
Learning: In the app settings validation logic for appResourceValidationUtils.ts, the Norwegian Bokmål ('nb') language uses a comprehensive error message via getMissingInputLanguageString that can describe multiple missing languages, while Norwegian Nynorsk ('nn') and English ('en') use simpler, direct translation keys for missing translations. This pattern should be preserved when refactoring validation logic.
Applied to files:
src/Designer/frontend/language/src/nb.json
📚 Learning: 2025-06-02T08:50:48.884Z
Learnt from: Jondyr
Repo: Altinn/altinn-studio PR: 15537
File: frontend/language/src/nb.json:1932-1932
Timestamp: 2025-06-02T08:50:48.884Z
Learning: In frontend/language/src/nb.json, the localization keys use hierarchical dot notation with multiple segments after the namespace (e.g., ux_editor.add_item.add_component, ux_editor.component_category.advanced). Keys with 2-3 segments after ux_editor are both common and accepted patterns.
Applied to files:
src/Designer/frontend/language/src/nb.json
📚 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/language/src/nb.json
🧬 Code graph analysis (2)
src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.tsx (2)
src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadUtils.ts (3)
getDisabledState(62-68)validateCrop(44-54)getCropToBeSaved(36-42)src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadTypes.ts (1)
InternalCrop(15-20)
src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadComponent.tsx (1)
src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadTypes.ts (1)
ExternalCrop(5-5)
⏰ 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: Typechecking and linting
- GitHub Check: CodeQL
- GitHub Check: Testing
🔇 Additional comments (1)
src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.tsx (1)
52-79: LGTM! Well-structured migration to StudioConfigCard.The refactoring successfully migrates from StudioCard to StudioConfigCard with proper composition:
- Header with delete functionality and confirmation
- Body with description and form controls
- Footer with save/cancel actions
The disable logic is correctly implemented, and all existing functionality is preserved.
...nts/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.test.tsx
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.tsx (2)
17-17: Normalise prop parameter casing inhandleSaveChangesMinor naming nit: the parameter in the prop type is
CropToBeSaved, which is unusual PascalCase for a function argument. Consider renaming tocropToBeSavedfor consistency with local variable naming conventions.
26-35: Consider initialisingerrorsfrom the initial crop to align disabled state & validationRight now
errorsstarts as{}, andisSaveDisabledis derived fromgetDisabledState({ errors, internalCrop, externalCrop }). IfgetInitialValues(externalCrop)can yield an “empty”/invalid crop (e.g. new config with no width/height/diameter), the initial render may treat the form as error‑free, andgetDisabledStatemight enable “Save” before any validation has run.To keep the disabled state and error messages in sync from the first render (especially when there is no existing
externalCrop), it would be safer to seederrorsfrom the same initial crop:- const [internalCrop, setInternalCrop] = useState<InternalCrop>(getInitialValues(externalCrop)); - const [errors, setErrors] = useState<ErrorProps>({}); + const [internalCrop, setInternalCrop] = useState<InternalCrop>(() => getInitialValues(externalCrop)); + const [errors, setErrors] = useState<ErrorProps>(() => validateCrop(internalCrop));This also keeps the “persist validation across shapes” behaviour intact, since
handleNewCropstill runsvalidateCropon every change. Based on learnings.
📜 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/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.tsx(2 hunks)
🧰 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: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16240
File: src/Designer/frontend/packages/ux-editor/src/components/Properties/EditSubformTableColumns/ColumnElement/EditColumnElement/EditColumnElement.tsx:85-116
Timestamp: 2025-09-02T10:47:19.978Z
Learning: In the Altinn Studio codebase, when migrating StudioCard usage to designsystemet v1, avoid using StudioCard.Block if StudioCard already has a border, as this creates unwanted double border effects. The team prioritizes clean visual design over strict adherence to component patterns when there are visual conflicts.
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: 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: 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: 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: 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: 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.
📚 Learning: 2025-10-20T07:58:25.550Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 16662
File: src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadUtils.ts:44-54
Timestamp: 2025-10-20T07:58:25.550Z
Learning: In the ImageUpload component in `src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/`, validation errors for all crop size fields (width, height, diameter) should persist when users switch between shapes to provide better UX feedback when they switch back.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.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/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.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/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.tsx
🧬 Code graph analysis (1)
src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.tsx (4)
src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadTypes.ts (2)
ExternalCrop(5-5)InternalCrop(15-20)src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadUtils.ts (3)
getDisabledState(62-68)validateCrop(44-54)getCropToBeSaved(36-42)src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadShape.tsx (1)
ImageUploadShape(11-38)src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadSize.tsx (1)
ImageUploadSize(14-57)
⏰ 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 (3)
src/Designer/frontend/packages/ux-editor/src/components/config/componentSpecificContent/ImageUpload/ImageUploadCard.tsx/ImageUploadCard.tsx (3)
2-9: Config card & utility imports look consistent with the new patternImporting
StudioConfigCard/StudioParagraphand grouping the crop utilities here reads cleanly and matches the rest of the ImageUpload module. No issues from my side.
37-49: Save/cancel/delete handlers are clear; just ensure upstream null handling is in placeThe three handlers are straightforward and centralise the “close card” behaviour nicely. Using
handleSaveChanges(null)inhandleDeletematches the updated prop contract and the new delete capability.Please just double‑check that all callers of
ImageUploadCardand any reducers/store logic behindhandleSaveChangescorrectly treatnullas “remove crop configuration”, so you don’t end up with an unhandled case or lingering stale state.
52-79: StudioConfigCard composition and delete wiring look solidThe move to
StudioConfigCardwith Header/Body/Footer appears well‑structured:
- Header: localised
cardLabel,deleteAriaLabel, andconfirmDeleteMessage, withisDeleteDisabled={!externalCrop}so delete is only available when there is something to remove.- Body: keeps
ImageUploadShape+ImageUploadSizewired throughhandleNewCrop, which continues to runvalidateCropon every change, so validation state should still persist when switching shapes. This aligns with the previous UX requirement for crop size validation. Based on learnings.- Footer: uses
isDisabled={isSaveDisabled}tied to validation/dirty state and localised save/cancel labels.Overall this matches the desired StudioConfigCard UX pattern and keeps the existing crop logic intact.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16967 +/- ##
==========================================
- Coverage 96.19% 96.17% -0.02%
==========================================
Files 2447 2377 -70
Lines 30790 30259 -531
Branches 3544 3487 -57
==========================================
- Hits 29618 29102 -516
+ Misses 864 844 -20
- Partials 308 313 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
|
Works fine, i just changed margen for card to 0: to match the widht with other fieldes when all open ; |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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
As part of the ux-cleanup.
Replaced the config card for image upload component. Now user can also remove the config.
Til info: til vedkommende som skal reviewe/teste denne, kan utføre endringer selv hvis det trengs, siden jeg drar på ferie i 2 uker 🙂
Verification
Summary by CodeRabbit
New Features
Refactor
Style
Tests
✏️ Tip: You can customize this high-level summary in your review settings.