chore: Select first page by default #17084
Open
+95
−2
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
github-actions
bot
added
area/ui-editor
Contributor
WalkthroughAdds logic to auto-select the first page in DesignView when pages load and nothing is selected, plus test coverage and an updated Playwright test call to pass a page name to the UI editor verification method. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
JamalAlabdullah
changed the title
![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: 0
🧹 Nitpick comments (1)
src/Designer/frontend/packages/ux-editor/src/containers/DesignView/DesignView.tsx (1)
25-25: Simplify the import path forItemType.The current import path traverses up four directories and back into the same package. This is unnecessarily convoluted and differs from the test file which uses the simpler relative path.
-import { ItemType } from '../../../../ux-editor/src/components/Properties/ItemType'; +import { ItemType } from '../../components/Properties/ItemType';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Designer/frontend/packages/ux-editor/src/containers/DesignView/DesignView.test.tsx(2 hunks)src/Designer/frontend/packages/ux-editor/src/containers/DesignView/DesignView.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 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: 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: 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: 15721
File: frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/PageAccordion.tsx:72-76
Timestamp: 2025-06-21T07:48:20.949Z
Learning: In the PageAccordion component in `frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/PageAccordion.tsx`, the `groupIndex?: number` prop is optional only to accommodate two different usage patterns: when the component is used for pages in groups (where groupIndex is always provided), and when used for pages not in groups (where groupIndex is not needed). When `isUsingGroups` is true, `groupIndex` will always be present and valid.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15721
File: frontend/packages/ux-editor/src/utils/designViewUtils/designViewUtils.ts:64-78
Timestamp: 2025-06-21T07:44:57.200Z
Learning: In the `excludePageFromGroup` function in `frontend/packages/ux-editor/src/utils/designViewUtils/designViewUtils.ts`, the group and group.order parameters are always guaranteed to be present based on the application's data flow, so defensive validation checks are not needed.
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15721
File: frontend/packages/ux-editor/src/utils/designViewUtils/designViewUtils.ts:47-57
Timestamp: 2025-06-21T07:47:09.403Z
Learning: In the `getUpdatedGroupsExcludingPage` function in `frontend/packages/ux-editor/src/utils/designViewUtils/designViewUtils.ts`, the groupIndex parameter is always guaranteed to be present and valid based on the application's data flow, so defensive validation checks are not needed.
📚 Learning: 2025-06-21T07:47:09.403Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15721
File: frontend/packages/ux-editor/src/utils/designViewUtils/designViewUtils.ts:47-57
Timestamp: 2025-06-21T07:47:09.403Z
Learning: In the `getUpdatedGroupsExcludingPage` function in `frontend/packages/ux-editor/src/utils/designViewUtils/designViewUtils.ts`, the groupIndex parameter is always guaranteed to be present and valid based on the application's data flow, so defensive validation checks are not needed.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/containers/DesignView/DesignView.tsxsrc/Designer/frontend/packages/ux-editor/src/containers/DesignView/DesignView.test.tsx
📚 Learning: 2025-06-21T07:48:20.949Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15721
File: frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/PageAccordion.tsx:72-76
Timestamp: 2025-06-21T07:48:20.949Z
Learning: In the PageAccordion component in `frontend/packages/ux-editor/src/containers/DesignView/PageAccordion/PageAccordion.tsx`, the `groupIndex?: number` prop is optional only to accommodate two different usage patterns: when the component is used for pages in groups (where groupIndex is always provided), and when used for pages not in groups (where groupIndex is not needed). When `isUsingGroups` is true, `groupIndex` will always be present and valid.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/containers/DesignView/DesignView.tsxsrc/Designer/frontend/packages/ux-editor/src/containers/DesignView/DesignView.test.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/containers/DesignView/DesignView.tsx
📚 Learning: 2025-06-21T07:44:57.200Z
Learnt from: lassopicasso
Repo: Altinn/altinn-studio PR: 15721
File: frontend/packages/ux-editor/src/utils/designViewUtils/designViewUtils.ts:64-78
Timestamp: 2025-06-21T07:44:57.200Z
Learning: In the `excludePageFromGroup` function in `frontend/packages/ux-editor/src/utils/designViewUtils/designViewUtils.ts`, the group and group.order parameters are always guaranteed to be present based on the application's data flow, so defensive validation checks are not needed.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/containers/DesignView/DesignView.tsxsrc/Designer/frontend/packages/ux-editor/src/containers/DesignView/DesignView.test.tsx
📚 Learning: 2025-08-12T05:59:20.146Z
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.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/containers/DesignView/DesignView.tsx
📚 Learning: 2025-08-12T06:09:55.275Z
Learnt from: TomasEng
Repo: Altinn/altinn-studio PR: 16064
File: frontend/libs/studio-components/src/components/StudioCheckboxGroup/StudioGetCheckboxProps.ts:1-1
Timestamp: 2025-08-12T06:09:55.275Z
Learning: In the Altinn Studio codebase, the preferred approach for eliminating dependencies on deep imports from digdir/designsystemet-react is to make the code completely independent of deep imports rather than using shared wrappers. This approach would eliminate the need for import path adjustments during future Design System package updates.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/containers/DesignView/DesignView.tsx
📚 Learning: 2025-10-06T17:51:23.307Z
Learnt from: JamalAlabdullah
Repo: Altinn/altinn-studio PR: 16518
File: src/Designer/frontend/libs/studio-components/src/components/StudioToggleableTextfieldSchema/StudioToggleableTextfieldSchema.stories.tsx:2-3
Timestamp: 2025-10-06T17:51:23.307Z
Learning: Altinn/altinn-studio: In Storybook stories (*.stories.tsx) under Designer/frontend libs, the project convention is to import types (Meta, StoryFn/StoryObj) from 'storybook/react-vite' rather than 'storybook/react'. Prefer 'storybook/react-vite' in future reviews for consistency.
Applied to files:
src/Designer/frontend/packages/ux-editor/src/containers/DesignView/DesignView.tsxsrc/Designer/frontend/packages/ux-editor/src/containers/DesignView/DesignView.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/containers/DesignView/DesignView.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/containers/DesignView/DesignView.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/containers/DesignView/DesignView.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/containers/DesignView/DesignView.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/containers/DesignView/DesignView.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: Build environment and run e2e test
- GitHub Check: Testing
- GitHub Check: CodeQL
- GitHub Check: Typechecking and linting
🔇 Additional comments (2)
src/Designer/frontend/packages/ux-editor/src/containers/DesignView/DesignView.tsx (1)
121-144: LGTM!The auto-selection logic is well-implemented:
firstPageIdcorrectly handles both grouped and non-grouped page models with appropriate optional chaining.- The
useEffectconditions ensure auto-selection only occurs when data is loaded, a first page exists, and no page is already selected.- The dependency array is complete.
src/Designer/frontend/packages/ux-editor/src/containers/DesignView/DesignView.test.tsx (1)
185-250: LGTM!The test suite is well-structured:
- The
renderDesignViewWithAutoSelecthelper correctly isolates the auto-selection behaviour by settingselectedFormLayoutName: undefined.- Both scenarios (flat pages and grouped pages) are covered.
- Assertions verify the correct calls to both
setSelectedFormLayoutNameandsetSelectedItemwith the expected arguments.Note: The AI summary mentioned duplicate test blocks, but this appears to be an inconsistency in the summary — only one test suite is present in the code.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #17084 +/- ##
==========================================
- Coverage 96.19% 96.17% -0.02%
==========================================
Files 2447 2378 -69
Lines 30790 30276 -514
Branches 3544 3490 -54
==========================================
- Hits 29618 29118 -500
+ Misses 864 845 -19
- Partials 308 313 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
github-actions
bot
added
the
quality/testing
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Designer/frontend/testing/playwright/tests/ui-editor/ui-editor-data-model-binding-and-gitea.spec.ts (1)
122-124: Consider removing duplicate verification.Lines 122 and 124 both call
verifyUiEditorPage(LAYOUT_SET, pageName)with identical arguments, separated only byclickOnPageAccordion(pageName). This appears redundant unless there's a specific reason to verify the page state both before and after the accordion click.If the intent is to verify that auto-selection persists after user interaction, consider adding a comment to clarify. Otherwise, the first verification on line 122 may be unnecessary.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Designer/frontend/testing/playwright/tests/ui-editor/ui-editor-data-model-binding-and-gitea.spec.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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: 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.
📚 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/testing/playwright/tests/ui-editor/ui-editor-data-model-binding-and-gitea.spec.ts
🔇 Additional comments (2)
src/Designer/frontend/testing/playwright/tests/ui-editor/ui-editor-data-model-binding-and-gitea.spec.ts (2)
112-134: Verify that auto-select behaviour is adequately tested.Given the PR objective is to fix auto-selecting the first page, this test should ideally verify that the page is automatically selected when navigating to the UI editor, before any explicit user interaction (such as clicking the page accordion on line 123).
Consider verifying the auto-selected state immediately after line 121 (
clickOnUxEditorButton()), before the explicitclickOnPageAccordion(pageName)on line 123. This would confirm that the auto-select feature works as intended.
40-40: Parameters are intentionally optional—inconsistent usage is by design, not a defect.The method signature
verifyUiEditorPage(layoutSet?: string, layout?: string | null)defines both parameters as optional. Calls without arguments (lines 40, 179) verify basic UI editor page load without checking specific layout state, while calls with both arguments (51, 93, 122, 124, 182) verify the correct layout is loaded. This pattern is consistent with the method design—not an inconsistency requiring correction.The double verification at lines 122–124 (calling with identical parameters before and after
clickOnPageAccordion) is functionally harmless, as the accordion click only expands/collapses a UI element without changing the URL or state thatverifyUiEditorPagevalidates.
JamalAlabdullah
changed the title
JamalAlabdullah
added
the
squad/utforming
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
CLOSES; #17082
Select first page by default when navigering from utform.
Solution
---Screen.Recording.2025-11-28.at.09.56.10.mov
Verification
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.