feat(portals-admin): Include relevant learn more links to the docs #16248
Conversation
WalkthroughThe changes in this pull request primarily involve the addition of new message definitions for internationalization, modifications to navigation components to include links to external documentation, and updates to various screens to enhance user assistance. New enums for external paths have been introduced, and existing components have been updated to integrate these paths, improving the user interface and navigation experience. Changes
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (8)
libs/portals/admin/ids-admin/src/lib/paths.ts (1)
13-17: LGTM! Consider adding JSDoc comments for better documentation.The new
IDSAdminExternalPathsenum is well-structured and aligns with the PR objectives by providing "learn more" links to the documentation. It follows good practices for reusability and TypeScript usage.To further improve the code, consider adding JSDoc comments to describe the purpose of each enum member. This will enhance code documentation and provide better context for developers using this enum. Here's an example of how you could improve it:
export enum IDSAdminExternalPaths { /** Main documentation for authentication */ Docs = 'https://docs.devland.is/products/auth', /** Documentation for defining clients */ DocsClients = 'https://docs.devland.is/products/auth/configuration#defining-clients', /** Documentation for scope configuration */ DocsPermissions = 'https://docs.devland.is/products/auth/configuration#scope-configuration', }libs/portals/admin/ids-admin/src/lib/navigation.ts (1)
41-53: New navigation item looks good, but consider addressing the TODOThe new navigation item for documentation is well-structured and aligns with the PR objectives of adding "learn more" links. It correctly uses
IDSAdminExternalPaths.Docsfor the external link and includes necessary properties for proper rendering and behavior.However, there's a TODO comment about fixing the icon rendering. Consider addressing this in a follow-up PR or providing more context about why it's left as a TODO.
Would you like assistance in creating a GitHub issue to track the icon rendering fix?
libs/portals/core/src/components/PortalNavigation/PortalNavigation.tsx (1)
92-92: Approved: Good enhancement for external links.This change improves user experience by opening external links in new tabs. It aligns well with the PR objective of enhancing UX with "learn more" links.
Consider adding
rel="noopener noreferrer"for security when opening links in new tabs:- target: href.startsWith('http') ? '_blank' : undefined, + target: href.startsWith('http') ? '_blank' : undefined, + rel: href.startsWith('http') ? 'noopener noreferrer' : undefined,libs/portals/admin/ids-admin/src/screens/Clients/Clients.tsx (1)
100-108: LGTM: New "Learn More" link improves UX as intended.The addition of the
LinkV2component effectively replaces the previous "Learn More" button with a link to external documentation. This change aligns well with the PR objectives to enhance user experience by providing easy access to relevant information.A minor suggestion for improvement:
Consider adding an aria-label to the link for better accessibility. For example:
<LinkV2 color="blue400" underline="normal" underlineVisibility="always" href={IDSAdminExternalPaths.DocsClients} newTab + aria-label={formatMessage(m.learnMoreAboutClients)} > {formatMessage(m.learnMore)} </LinkV2>This assumes you have a message
learnMoreAboutClientsin your localization file. If not, you may need to add it.libs/portals/admin/ids-admin/src/screens/Permissions/Permissions.tsx (1)
85-86: LGTM: Learn more link added successfullyThe modification to the LinkV2 component successfully implements the PR objective of adding a "learn more" link. The use of
IDSAdminExternalPaths.DocsPermissionsfor the href and the addition of thenewTabprop improve user experience by providing easy access to relevant documentation.Suggestion for improvement:
Consider adding an aria-label to the link for better accessibility, e.g.:aria-label={formatMessage(m.learnMoreAboutPermissions)}This would require adding a new message to your localization file.
libs/portals/admin/ids-admin/src/screens/Client/EditClient.tsx (2)
10-18: Remove unused importAlertBanner.The
AlertBannercomponent is imported but not used in the visible changes. Consider removing this import to keep the code clean and avoid potential bundle size increases.Apply this diff to remove the unused import:
import { AlertMessage, Button, Stack, Text, - AlertBanner, Box, LinkV2, } from '@island.is/island-ui/core'
109-124: Approve new help section with a minor suggestion.The new help section with a "Learn More" link to the documentation aligns well with the PR objectives. It enhances the user experience by providing easy access to additional information.
Consider adding an
aria-labelto the LinkV2 component to improve accessibility. For example:- <LinkV2 href={IDSAdminExternalPaths.Docs} newTab> + <LinkV2 href={IDSAdminExternalPaths.Docs} newTab aria-label={formatMessage(m.learnMoreAriaLabel)}>Don't forget to add the corresponding message to your localization file.
libs/portals/admin/ids-admin/src/lib/messages.ts (1)
547-550: LGTM with a minor suggestion: Documentation description message.The
documentationDescriptionmessage is well-defined and consistent with existing patterns. It supports the PR objective of including relevant "learn more" links.Consider clarifying "IDS admin" in the default message for better user understanding. For example:
- defaultMessage: 'Link to external documentation for IDS admin.', + defaultMessage: 'Link to external documentation for Identity Service (IDS) administration.',
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (7)
- libs/portals/admin/ids-admin/src/lib/messages.ts (2 hunks)
- libs/portals/admin/ids-admin/src/lib/navigation.ts (2 hunks)
- libs/portals/admin/ids-admin/src/lib/paths.ts (1 hunks)
- libs/portals/admin/ids-admin/src/screens/Client/EditClient.tsx (3 hunks)
- libs/portals/admin/ids-admin/src/screens/Clients/Clients.tsx (2 hunks)
- libs/portals/admin/ids-admin/src/screens/Permissions/Permissions.tsx (2 hunks)
- libs/portals/core/src/components/PortalNavigation/PortalNavigation.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
libs/portals/admin/ids-admin/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."
libs/portals/admin/ids-admin/src/lib/navigation.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/portals/admin/ids-admin/src/lib/paths.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/portals/admin/ids-admin/src/screens/Client/EditClient.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/portals/admin/ids-admin/src/screens/Clients/Clients.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/portals/admin/ids-admin/src/screens/Permissions/Permissions.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/portals/core/src/components/PortalNavigation/PortalNavigation.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."
🔇 Additional comments (12)
libs/portals/admin/ids-admin/src/lib/navigation.ts (2)
3-3: LGTM: Import statement updated correctlyThe import statement has been appropriately updated to include
IDSAdminExternalPaths, which is consistent with its usage in the new navigation item. This change supports effective tree-shaking and adheres to the TypeScript usage guideline.
Line range hint
1-67: Compliance with coding guidelines confirmedThe code in this file adheres to the specified guidelines for
libs/**/*:
- Reusability: The navigation structure is defined in a modular way that can be easily reused across different NextJS apps.
- TypeScript usage: The code properly uses TypeScript for importing types and defining the navigation structure with the
PortalNavigationItemtype.- Tree-shaking and bundling: The specific imports and exports support effective tree-shaking, allowing for efficient bundling.
libs/portals/core/src/components/PortalNavigation/PortalNavigation.tsx (1)
Line range hint
1-105: Adherence to coding guidelines confirmed.The file adheres to the specified guidelines for
libs/**/*:
- The
PortalNavigationcomponent is reusable across different NextJS apps.- TypeScript is used effectively for defining props and exporting types.
- The code structure supports effective tree-shaking and bundling practices.
libs/portals/admin/ids-admin/src/screens/Clients/Clients.tsx (2)
11-11: LGTM: Import changes are appropriate and align with PR objectives.The addition of
LinkV2andIDSAdminExternalPathsimports are consistent with the existing code style and support the implementation of the new "learn more" link feature.Also applies to: 17-17
Line range hint
1-165: Verify compliance with coding guidelines for libs//* pattern**The code generally adheres to the guidelines for files in the
libsdirectory. However, to ensure full compliance, please verify the following:
Reusability: The
Clientscomponent seems designed for reuse. Confirm that it can be easily integrated into different NextJS apps without modification.TypeScript usage: While the code uses TypeScript, ensure that all props and exported types are properly defined. Consider extracting and exporting interface definitions for better reusability.
Tree-shaking and bundling: The current import statements look good for tree-shaking. Verify that all exported items are named exports to facilitate tree-shaking in consuming applications.
To assist in verifying these points, you can run the following script:
This script will help identify areas where we might improve type definitions and export patterns to better align with the coding guidelines.
libs/portals/admin/ids-admin/src/screens/Permissions/Permissions.tsx (2)
18-18: LGTM: Import statement updated correctlyThe addition of
IDSAdminExternalPathsto the import statement is appropriate and aligns with the PR objective of including "learn more" links. This change follows TypeScript usage guidelines and is necessary for the subsequent use in the component.
Line range hint
1-186: Verify type exports for reusabilityThe changes comply with the coding guidelines for
libs/**/*files. To further enhance reusability across different NextJS apps, please ensure that any new types or interfaces used in this component are properly exported from the file or a central types file.Run the following script to check for type exports:
If no type exports are found, consider adding them to improve reusability.
libs/portals/admin/ids-admin/src/screens/Client/EditClient.tsx (1)
Line range hint
1-180: Summary: Effective implementation of help section with documentation link.The changes in this file successfully implement the PR objective of including relevant "learn more" links to the documentation. The new help section is well-placed and uses reusable components from the
@island.is/island-ui/corelibrary, adhering to the coding guidelines for thelibs/**/*path pattern.Key points:
- Reusable components (Box, AlertMessage, LinkV2) are used effectively.
- TypeScript is used for defining props.
- The changes enhance the user experience without disrupting existing functionality.
Overall, this is a solid implementation that improves the admin portal's usability.
libs/portals/admin/ids-admin/src/lib/messages.ts (4)
80-83: LGTM: New message definition for user assistance.The
needHelpTitlemessage is well-defined and consistent with existing message patterns. It aligns with the PR objective of enhancing user experience by providing additional help-related content.
84-87: LGTM: Complementary description for user assistance.The
needHelpDescriptionmessage is well-defined and complements theneedHelpTitlemessage. It provides a clear direction for users to find help, which aligns with the PR's goal of improving user experience.
543-546: LGTM: New message definition for documentation link.The
documentationmessage is correctly defined and consistent with existing message patterns. It supports the PR objective of including relevant "learn more" links to the documentation.
80-87: Overall assessment: Changes align with guidelines and objectives.The new message definitions (
needHelpTitle,needHelpDescription,documentation, anddocumentationDescription) adhere to the coding guidelines forlibs/**/*files:
- They use TypeScript for defining props and exporting types.
- The messages are reusable across different NextJS apps.
- The structure supports effective tree-shaking and bundling.
These additions enhance the internationalization support for user assistance and documentation links, aligning well with the PR's objective of improving user experience in the portals-admin feature.
Also applies to: 543-550
|
Affected services are: portals-admin,service-portal,system-e2e, Deployed services: service-portal,portals-admin. |
Datadog ReportAll test runs ✅ 70 Total Test Services: 0 Failed, 67 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (2) |
baering
added
deprecated:automerge
What
☝️
Why
To improve UX
Screenshots / Gifs
Navigation
Empty client list
Empty permission list
Edit client
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes