feat(web): Featured link slice #17416
Conversation
WalkthroughThe pull request introduces a new Changes
Sequence DiagramsequenceDiagram
participant User
participant SliceMachine
participant FeaturedLinksSlice
participant FeaturedLinks
participant LinkResolver
User->>SliceMachine: Request page with slice
SliceMachine->>FeaturedLinksSlice: Render slice
FeaturedLinksSlice->>FeaturedLinks: Pass featured links
FeaturedLinks->>LinkResolver: Resolve link URLs
LinkResolver-->>FeaturedLinks: Return resolved URLs
FeaturedLinks->>User: Render featured links
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
|
Datadog ReportAll test runs ❌ 10 Total Test Services: 1 Failed, 8 Passed Test ServicesThis report shows up to 10 services
❌ Failed Tests (11)
🔻 Code Coverage Decreases vs Default Branch (2) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17416 +/- ##
=======================================
Coverage 35.65% 35.65%
=======================================
Files 6939 6942 +3
Lines 149153 149171 +18
Branches 42653 42652 -1
=======================================
+ Hits 53182 53191 +9
- Misses 95971 95980 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
apps/web/components/SearchSection/SearchSection.tsx (1)
Line range hint
41-49: Remove ts-ignore comments by fixing type definitions.There are multiple ts-ignore comments in the code. Consider fixing the underlying type issues by:
- Properly typing the page prop
- Adding proper type definitions for featured, heading, image, etc.
-// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-ignore make web strict -featured, +featured?: Featured[],
🧹 Nitpick comments (3)
apps/web/components/Organization/Slice/FeaturedLinksSlice/FeaturedLinksSlice.tsx (1)
9-17: Consider memoizing the component for performance optimization.The component is well-structured, but since it's likely to be rendered frequently in a list, consider wrapping it with React.memo to prevent unnecessary re-renders.
-export const FeaturedLinksSlice = ({ slice }: FeaturedLinksSliceProps) => { +export const FeaturedLinksSlice = React.memo(({ slice }: FeaturedLinksSliceProps) => { if (!slice.featuredLinks?.length) return null return ( <Stack space={2}> <Text variant="h4">{slice.title}</Text> <FeaturedLinks links={slice.featuredLinks} /> </Stack> ) -} +})libs/cms/src/lib/models/featuredLinks.model.ts (1)
19-27: Consider adding input validation in the mapping function.While the mapping function handles null cases, it might benefit from additional validation for the input fields.
export const mapFeaturedLinks = ({ fields, sys, }: IFeaturedLinks): SystemMetadata<FeaturedLinks> => ({ typename: 'FeaturedLinks', id: sys.id, - title: fields.displayedTitle ?? '', + title: fields.displayedTitle?.trim() ?? '', featuredLinks: fields.links?.filter(Boolean).map(mapFeatured) ?? [], })apps/web/components/FeaturedLinks/FeaturedLinks.tsx (1)
22-31: Extract CustomLink component for better maintainability.The inline CustomLink component definition could be moved outside the render method to improve readability and prevent unnecessary recreations.
+const CustomLink = ({ children, title, ...props }) => ( + <LinkV2 {...props} dataTestId="featured-link"> + {children} + </LinkV2> +) export const FeaturedLinks = ({ links }: FeaturedLinksProps) => { // ... rest of the component
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
libs/cms/src/lib/generated/contentfulTypes.d.tsis excluded by!**/generated/**
📒 Files selected for processing (8)
apps/web/components/FeaturedLinks/FeaturedLinks.tsx(1 hunks)apps/web/components/Organization/Slice/FeaturedLinksSlice/FeaturedLinksSlice.tsx(1 hunks)apps/web/components/Organization/Slice/SliceMachine.tsx(2 hunks)apps/web/components/SearchSection/SearchSection.tsx(2 hunks)apps/web/components/real.ts(1 hunks)apps/web/screens/queries/fragments.ts(2 hunks)libs/cms/src/lib/models/featuredLinks.model.ts(1 hunks)libs/cms/src/lib/unions/slice.union.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
apps/web/components/real.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/components/Organization/Slice/SliceMachine.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/screens/queries/fragments.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/components/Organization/Slice/FeaturedLinksSlice/FeaturedLinksSlice.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/components/FeaturedLinks/FeaturedLinks.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/web/components/SearchSection/SearchSection.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/cms/src/lib/models/featuredLinks.model.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/cms/src/lib/unions/slice.union.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 (6)
apps/web/components/Organization/Slice/FeaturedLinksSlice/FeaturedLinksSlice.tsx (1)
5-7: LGTM! Well-defined TypeScript interface.The interface is properly typed with the GraphQL schema type, ensuring type safety throughout the component.
libs/cms/src/lib/models/featuredLinks.model.ts (1)
8-17: LGTM! Well-structured GraphQL object type.The class is properly decorated with GraphQL decorators and includes appropriate field types and nullability checks.
apps/web/components/real.ts (1)
99-100: LGTM! Exports follow the established pattern.The new exports maintain consistency with the existing barrel file structure and documentation guidelines.
libs/cms/src/lib/unions/slice.union.ts (1)
51-51: Integration of FeaturedLinks follows established patterns!The addition of the
FeaturedLinkstype and its mapping function is well-structured and maintains consistency with the existing type system.Also applies to: 197-197, 250-250, 348-349
apps/web/screens/queries/fragments.ts (1)
924-936: Please clarify the purpose of theattentionfield.The
FeaturedLinksFieldsfragment is well-structured, but the purpose of theattentionfield infeaturedLinksis not immediately clear. Consider adding a comment or documentation to explain its usage.apps/web/components/Organization/Slice/SliceMachine.tsx (1)
110-112: Implementation follows NextJS best practices!The dynamic import and slice rendering implementation for
FeaturedLinksfollows the established patterns and NextJS best practices for code splitting.Also applies to: 227-228
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/components/Organization/Slice/SliceMachine.tsx (2)
228-229: Consider improving type safetyWhile the implementation is correct, the function uses multiple
@ts-ignorecomments. Consider improving type safety by:
- Properly typing the
sliceparameter- Using a discriminated union type for different slice types
- Removing the need for
@ts-ignorecommentsExample type improvement:
type SliceUnion = | { __typename: 'FeaturedLinks'; /* ... other props */ } | { __typename: 'HeadingSlice'; /* ... other props */ } // ... other slice types function renderSlice(slice: SliceUnion, namespace: Record<string, string>, slug: string, params: unknown) { switch (slice.__typename) { // ... cases } }
Line range hint
1-290: Consider restructuring for better maintainabilityThe file has grown to handle many slice types and could benefit from restructuring:
- Group related slices into separate modules
- Create a registry pattern for slice types
- Move slice type definitions to a separate file
Example restructure:
// sliceRegistry.ts const sliceRegistry = { content: { HeadingSlice: () => import('./slices/content/HeadingSlice'), OneColumnTextSlice: () => import('./slices/content/OneColumnTextSlice'), // ... }, features: { FeaturedLinksSlice: () => import('./slices/features/FeaturedLinksSlice'), FeaturedArticlesSlice: () => import('./slices/features/FeaturedArticlesSlice'), // ... } }; // This would simplify SliceMachine.tsx and make it more maintainablelibs/cms/src/lib/unions/slice.union.ts (1)
51-51: Well-structured changes maintaining library guidelines.The changes follow best practices for library code:
- Proper TypeScript type definitions ensuring type safety
- Clean integration with the CMS type system
- Maintains reusability across NextJS apps
- Supports tree-shaking through specific exports
Also applies to: 150-150, 203-203, 256-257, 355-356
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
libs/cms/src/lib/generated/contentfulTypes.d.tsis excluded by!**/generated/**
📒 Files selected for processing (4)
apps/web/components/FeaturedLinks/FeaturedLinks.tsx(1 hunks)apps/web/components/Organization/Slice/SliceMachine.tsx(2 hunks)apps/web/screens/queries/fragments.ts(2 hunks)libs/cms/src/lib/unions/slice.union.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/screens/queries/fragments.ts
- apps/web/components/FeaturedLinks/FeaturedLinks.tsx
🧰 Additional context used
📓 Path-based instructions (2)
apps/web/components/Organization/Slice/SliceMachine.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/cms/src/lib/unions/slice.union.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)
apps/web/components/Organization/Slice/SliceMachine.tsx (1)
110-112: LGTM! Dynamic import follows best practicesThe dynamic import implementation follows NextJS best practices for code splitting and maintains consistency with existing patterns.
libs/cms/src/lib/unions/slice.union.ts (3)
51-51: LGTM! Type definitions and imports are well-organized.The new
FeaturedLinksrelated imports and type definitions are properly integrated, maintaining consistency with existing patterns.Also applies to: 150-150, 203-203
256-257: LGTM! Union type integration is correct.The
FeaturedLinkstype is properly added to theSliceUnionarray, maintaining the type system integrity.
355-356: Verify the FeaturedLinks mapping implementation.The mapping case is correctly added. Let's verify the implementation of the imported
mapFeaturedLinksfunction.Run this script to check the mapping function implementation:
✅ Verification successful
✓ FeaturedLinks mapping implementation is correct
The
mapFeaturedLinksfunction properly transformsIFeaturedLinksintoSystemMetadata<FeaturedLinks>with appropriate field mapping and null handling.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the FeaturedLinks mapping implementation # Expected: Find the mapFeaturedLinks function implementation that converts IFeaturedLinks to SystemMetadata<FeaturedLinks> ast-grep --pattern 'export const mapFeaturedLinks = (slice: IFeaturedLinks)'Length of output: 78
Script:
#!/bin/bash # Search for any mentions of mapFeaturedLinks rg "mapFeaturedLinks" -A 2 # Find files related to FeaturedLinks fd -t f "FeaturedLinks" # Look for import statements containing mapFeaturedLinks rg "import.*mapFeaturedLinks"Length of output: 1013
Script:
#!/bin/bash # Get the complete implementation of mapFeaturedLinks cat libs/cms/src/lib/models/featuredLinks.model.tsLength of output: 795
Featured link slice
Organization page slice
Frontpage
Checklist:
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
FeaturedLinkscomponent for displaying featured links.FeaturedLinksSlicecomponent for rendering featured links within slices.SearchSectionto utilize the newFeaturedLinkscomponent.Improvements
FeaturedLinksSlicecomponent.Technical Updates
The changes improve link presentation and provide more flexible content rendering across the application.