-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(new button in form): Add possibility to render ServicePortal button in form footer #17433
Conversation
WalkthroughThis pull request introduces a new feature for rendering a service portal button in the application's form flow. The changes span multiple libraries, adding a new message entry, extending the Changes
Sequence DiagramsequenceDiagram
participant FormShell
participant Screen
participant ScreenFooter
FormShell->>Screen: Pass renderLastScreenServicePortalButton
Screen->>ScreenFooter: Forward renderLastScreenServicePortalButton
alt renderLastScreenServicePortalButton is true
ScreenFooter->>ScreenFooter: Render Service Portal Button
end
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17433 +/- ##
=======================================
Coverage 35.64% 35.64%
=======================================
Files 6939 6939
Lines 149134 149134
Branches 42647 42647
=======================================
Hits 53165 53165
Misses 95969 95969 Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
libs/application/ui-shell/src/components/ScreenFooter.tsx (2)
199-211
: Consider extracting duplicated button logic.The service portal button implementation is duplicated between desktop and mobile views. Consider extracting this into a reusable component or shared function.
+ const ServicePortalButton = ({ isMobile, ...props }) => ( + <Button + {...props} + variant="ghost" + data-testid="step-back" + onClick={() => { + window.open(servicePortalUrl, '_self') + }} + disabled={!canProceed || loading} + {...(isMobile ? { + circle: true, + icon: "arrowBack" + } : {})} + > + {!isMobile && formatMessage(coreMessages.buttonServicePortal)} + </Button> + ) {showOpenServicePortal && ( - <Button - variant="ghost" - data-testid="step-back" - onClick={() => { - window.open(servicePortalUrl, '_self') - }} - disabled={!canProceed || loading} - > - {formatMessage(coreMessages.buttonServicePortal)} - </Button> + <ServicePortalButton isMobile={false} /> )} // Mobile view {showOpenServicePortal && ( - <Button - circle - data-testid="step-back" - variant="ghost" - icon="arrowBack" - onClick={() => { - window.open(servicePortalUrl, '_self') - }} - disabled={!canProceed || loading} - /> + <ServicePortalButton isMobile={true} /> )}Also applies to: 224-236
84-84
: Consider making the service portal URL configurable.The URL construction could be made more configurable by moving it to a configuration file or accepting it as a prop.
+ interface FooterProps { + // ... existing props + servicePortalBaseUrl?: string + } - const servicePortalUrl = `/minarsidur/umsoknir#${application.id}` + const servicePortalUrl = `${props.servicePortalBaseUrl ?? '/minarsidur/umsoknir'}#${application.id}`libs/application/core/src/lib/messages.ts (1)
14-18
: Consider enhancing the description for better reusability.The implementation follows the established pattern and naming conventions. However, to improve reusability across different NextJS apps, consider making the description more detailed to help other developers understand the specific use case and context.
Apply this diff to enhance the description:
buttonServicePortal: { id: 'application.system:button.buttonServicePortal', defaultMessage: 'Til baka á mínar síður', - description: 'Service portal', + description: 'Button to navigate back to the service portal (mínar síður) from the overview page', },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
libs/application/core/src/lib/messages.ts
(1 hunks)libs/application/types/src/lib/Form.ts
(1 hunks)libs/application/ui-shell/src/components/Screen.tsx
(3 hunks)libs/application/ui-shell/src/components/ScreenFooter.tsx
(5 hunks)libs/application/ui-shell/src/lib/FormShell.tsx
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
libs/application/types/src/lib/Form.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/ui-shell/src/components/ScreenFooter.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/ui-shell/src/lib/FormShell.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/ui-shell/src/components/Screen.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/core/src/lib/messages.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (4)
libs/application/types/src/lib/Form.ts (1)
85-85
: LGTM! Type definition follows best practices.The new optional boolean property is well-typed and follows the existing naming pattern for similar properties.
libs/application/ui-shell/src/lib/FormShell.tsx (1)
68-68
: LGTM! Clean prop drilling implementation.The property is correctly extracted from state and properly passed down to the Screen component, following the established pattern.
Also applies to: 154-156
libs/application/ui-shell/src/components/ScreenFooter.tsx (1)
81-84
: LGTM! Clean conditional rendering logic.The showOpenServicePortal condition is well-structured and follows the existing pattern.
libs/application/ui-shell/src/components/Screen.tsx (1)
77-77
: LGTM! Clean prop drilling implementation.The property is correctly typed in ScreenProps and properly passed down to the ScreenFooter component.
Also applies to: 419-421
What
In the application SecondarySchool, we have an overview page after application has been submitted to MMS. We need to add a back button on that screen, but since there is no steps to go back to, it should just open minar-sidur. If using the built in buildMessageWithLinkButtonField to display button for minar-sidur, then the page looks weird with both "Breyta umsókn" and this blue box
Screenshots / Gifs
Before (using buildMessageWithLinkButtonField):
After (using renderLastScreenServicePortalButton on form):
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Improvements
The update provides users with an additional navigation option, allowing them to easily return to their personal service portal from the last screen of forms.