refactor: move Panel to app component library#18880
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughThis PR extracts and reimplements the Panel component in libs/form-component (types, implementation, tests, Storybook) and migrates frontend consumers to import the shared Panel and use FullWidthWrapper for layout control instead of Panel-specific layout props. ChangesPanel component migration to shared library
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
github-actions
Bot
added
the
skip-releasenotes
… created a dependency between a dumb component and a layout component config.
…n/altinn-studio into refactor/move-Panel-to-app
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/App/frontend/src/layout/Panel/config.ts (1)
4-6: ⚡ Quick winConsider adding a verification mechanism to ensure sync with the library.
The inlined
PANEL_VARIANTSconstant addresses the codegen CSS import issue, but it creates a maintenance risk: ifPanelVariantin@app/form-componentis updated, this constant may drift out of sync. Consider adding a verification script or a test that imports both and compares them to catch discrepancies.🔍 Example verification approach
Add a test file (e.g.,
config.test.ts) that runs outside codegen context:import { PANEL_VARIANTS } from './config'; import type { PanelVariant } from '`@app/form-component`'; // This will fail to compile if the types don't match const _typeCheck: readonly PanelVariant[] = PANEL_VARIANTS; test('PANEL_VARIANTS matches library PanelVariant', () => { // Runtime verification could go here if needed expect(PANEL_VARIANTS).toEqual(['info', 'warning', 'error', 'success']); });Alternatively, add a prominent comment reminding maintainers to keep them synchronized.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App/frontend/src/layout/Panel/config.ts` around lines 4 - 6, Add a verification step to ensure the inlined PANEL_VARIANTS stays in sync with the library PanelVariant: create a test or script that imports PANEL_VARIANTS from this module and the PanelVariant type from `@app/form-component` (reference PANEL_VARIANTS and PanelVariant) and fails if the arrays/types diverge; alternatively add a clear TODO comment in config.ts pointing maintainers to update PANEL_VARIANTS when PanelVariant changes and link to the library so future edits catch the drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/App/frontend/src/layout/Panel/config.ts`:
- Around line 4-6: Add a verification step to ensure the inlined PANEL_VARIANTS
stays in sync with the library PanelVariant: create a test or script that
imports PANEL_VARIANTS from this module and the PanelVariant type from
`@app/form-component` (reference PANEL_VARIANTS and PanelVariant) and fails if the
arrays/types diverge; alternatively add a clear TODO comment in config.ts
pointing maintainers to update PANEL_VARIANTS when PanelVariant changes and link
to the library so future edits catch the drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a78d695b-059a-42fd-9340-0d417bae7ddd
📒 Files selected for processing (17)
libs/form-component/src/app-components/Panel/Panel.module.csslibs/form-component/src/app-components/Panel/Panel.stories.tsxlibs/form-component/src/app-components/Panel/Panel.test.tsxlibs/form-component/src/app-components/Panel/Panel.tsxlibs/form-component/src/app-components/Panel/index.tslibs/form-component/src/app-components/index.tssrc/App/frontend/monorepo-changed-paths.txtsrc/App/frontend/src/app-components/Panel/Panel.test.tsxsrc/App/frontend/src/app-components/Panel/Panel.tsxsrc/App/frontend/src/app-components/Panel/constants.tssrc/App/frontend/src/layout/Group/GroupComponent.tsxsrc/App/frontend/src/layout/IFrame/IFrameComponent.tsxsrc/App/frontend/src/layout/Panel/PanelComponent.tsxsrc/App/frontend/src/layout/Panel/config.tssrc/App/frontend/src/layout/SigningActions/PanelAwaitingCurrentUserSignature.tsxsrc/App/frontend/src/layout/SigningActions/PanelSigning.tsxsrc/App/frontend/src/layout/SigningActions/SigningActionsComponent.tsx
💤 Files with no reviewable changes (3)
- src/App/frontend/src/app-components/Panel/constants.ts
- src/App/frontend/src/app-components/Panel/Panel.test.tsx
- src/App/frontend/src/app-components/Panel/Panel.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #18880 +/- ##
=======================================
Coverage 95.84% 95.84%
=======================================
Files 3018 3018
Lines 39575 39585 +10
Branches 4849 4857 +8
=======================================
+ Hits 37931 37941 +10
Misses 1230 1230
Partials 414 414 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Description
Moves
Panelfromsrc/App/frontend/src/app-components/Paneltolibs/form-component/src/app-components/Paneland exposes it through@app/form-component.The migrated Panel is a dumb UI component:
titleis nowReact.ReactNode— callers pass already-translated content (e.g.<Lang id=… />) instead of aTranslationKey. This drops the dependency onAppComponentsProvider/useTranslationinside the lib.fullWidth/isOnBottom/isOnTopprops (which wrapped the panel inFullWidthWrapper) are removed. The two call sites that need full-width framing (PanelComponent, signing panels) now wrap withFullWidthWrapperthemselves, keepingFullWidthWrapperandConditionalWrapperin app-frontend.All Panel call sites in app-frontend were updated to import from
@app/form-componentand to do their own wrapping/translation. Unit tests and a Storybook story were added in the lib, andscripts/compare-frontend-repos.ts updatewas run.Verification
Summary by CodeRabbit