feat(web): add support links to grants #17189
Conversation
WalkthroughThe changes involve modifications to the Changes
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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17189 +/- ##
==========================================
- Coverage 35.77% 35.76% -0.01%
==========================================
Files 6931 6931
Lines 147962 147974 +12
Branches 42169 42171 +2
==========================================
- Hits 52928 52926 -2
- Misses 95034 95048 +14 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files 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: 0
🧹 Outside diff range and nitpick comments (2)
apps/web/screens/Grants/Grant/GrantSidebar.tsx (2)
138-165: Consider adding loading state and error handlingWhile the implementation is solid, consider these improvements:
- Add loading state for async link rendering
- Handle potential errors when links are invalid
const supportLinksPanelData = useMemo( () => grant.supportLinks ?.map((link) => { if (!link.url || !link.text || !link.id) { + console.warn(`Invalid support link found: ${JSON.stringify(link)}`); return null; } + try { + const url = new URL(link.url); return ( <LinkV2 newTab key={link.id} - href={link.url} + href={url.toString()} underlineVisibility="hover" > <Button size="medium" icon="link" iconType="outline" variant="text" > {link.text} </Button> </LinkV2> ) + } catch (error) { + console.error(`Invalid URL in support link: ${link.url}`); + return null; + } }) .filter(isDefined) ?? [], [grant.supportLinks], )
3-10: LGTM: Follows NextJS and TypeScript best practicesThe component implementation:
- Uses proper TypeScript types with BoxProps
- Implements efficient state management with useMemo
- Follows island-ui/core component patterns
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
libs/cms/src/lib/generated/contentfulTypes.d.tsis excluded by!**/generated/**
📒 Files selected for processing (3)
apps/web/screens/Grants/Grant/GrantSidebar.tsx(5 hunks)apps/web/screens/queries/Grants.ts(1 hunks)libs/cms/src/lib/models/grant.model.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
apps/web/screens/queries/Grants.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."
libs/cms/src/lib/models/grant.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."
apps/web/screens/Grants/Grant/GrantSidebar.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."
🔇 Additional comments (6)
libs/cms/src/lib/models/grant.model.ts (3)
10-10: LGTM: Import statement is correctly placed
The Link model import is appropriately placed with other model imports.
70-72: LGTM: Field definition follows best practices
The supportLinks field is properly decorated with @CacheField and correctly typed as an optional array of Link.
123-124: LGTM: Mapping function handles edge cases
The mapping function correctly handles null/undefined cases using the nullish coalescing operator.
apps/web/screens/queries/Grants.ts (1)
80-85: Verify usage of the date field
While the query structure is correct, the date field is retrieved but not used in the UI component.
Consider removing the date field from the query if it's not being used to optimize the response payload.
apps/web/screens/Grants/Grant/GrantSidebar.tsx (2)
40-52: LGTM: Well-structured helper function
The generateSidebarPanel helper function improves code reusability and maintains consistent styling across panels.
178-180: LGTM: Consistent panel implementation
The implementation maintains visual consistency with other panels through proper background colors and spacing.
Datadog ReportAll test runs ✅ 11 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
thorkellmani
added
automerge
oddsson
added
automerge
What
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation