feat(web): add info card #16931
feat(web): add info card #16931
Conversation
WalkthroughThe pull request introduces several changes across multiple files, primarily focusing on the restructuring and enhancement of the InfoCard component suite. It includes the addition of new components such as 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
|
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 (5) |
|
Affected services are: air-discount-scheme-api,api,application-system-api,judicial-system-api,judicial-system-backend,services-search-indexer,services-user-notification,web, Deployed services: application-system-api,api,web,air-discount-scheme-api,application-system-api-worker,consultation-portal,services-bff-portals-admin,services-bff-portals-my-pages,air-discount-scheme-web. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (6)
apps/web/components/InfoCard/DetailedInfoCard.tsx (1)
69-69: Ensurealtattribute for images is providedThe
imgelement should have a meaningfulaltattribute for accessibility. SincelogoAltis optional, there is a possibility of thealtattribute beingundefined. Consider providing a defaultalttext or makinglogoAlta required prop to improve accessibility.Apply this diff to set a default
alttext:return ( <Box style={{ flex: '0 0 40px' }}> - <img height={40} src={logo} alt={logoAlt} /> + <img height={40} src={logo} alt={logoAlt || 'Logo'} /> </Box> )Alternatively, make
logoAlta required prop:export type DetailedProps = BaseProps & { logo?: string - logoAlt?: string + logoAlt: string subEyebrow?: string //max 5 lines detailLines?: Array<{ icon: IconMapIcon text: string }> tags?: Array<ActionCardProps['tag']> }apps/web/screens/Grants/utils.ts (1)
Removing
GrantStatus.SeeDescriptioncase requires additional changes in the codebaseThe verification reveals that
GrantStatus.SEE_DESCRIPTIONis still actively used in the codebase:
- It's defined in
libs/cms/src/lib/models/grant.model.ts- It's referenced in
libs/cms/src/lib/search/importers/grants.service.ts- The status is assigned in the grant model when certain conditions are met
The removal of this case from
generateStatusTagwill causeundefinedto be returned whenSEE_DESCRIPTIONstatus is encountered, which could lead to UI issues in:
apps/web/screens/Grants/Grant/GrantSidebar.tsxapps/web/screens/Grants/SearchResults/SearchResultsContent.tsx🔗 Analysis chain
Line range hint
4-18: Verify the impact of removingGrantStatus.SeeDescriptioncaseBy removing the
GrantStatus.SeeDescriptioncase fromgenerateStatusTag, any instances where this status is used will now returnundefined. Ensure that this change does not cause unintended behavior in the application.Run the following script to check for usages of
GrantStatus.SeeDescription:This will help identify if there are any remaining references that need to be addressed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all usages of GrantStatus.SeeDescription # Search for any occurrences of GrantStatus.SeeDescription in the codebase rg 'GrantStatus\.SeeDescription'Length of output: 34
Script:
#!/bin/bash # Let's first check the GrantStatus enum definition to understand all possible values rg -A 5 "enum GrantStatus" # Also search for any imports or references to GrantStatus rg "GrantStatus" # Let's also check the actual usage of generateStatusTag function rg "generateStatusTag"Length of output: 2671
apps/web/components/InfoCard/InfoCard.tsx (1)
17-24: Consider making thevariantprop required for clarityCurrently, the
variantprop is optional for the simple variant and required for the detailed variant. This could lead to confusion and potential bugs if the prop is omitted unintentionally. Makingvarianta required prop improves type safety and clarity.Apply this diff to adjust the
InfoCardPropstype:export type InfoCardProps = - | (BaseProps & { - variant?: 'simple' + | (BaseProps & ({ + variant: 'simple' }) | (DetailedProps & { variant: 'detailed' })Alternatively, set a default value for the
variantprop in theInfoCardcomponent:export const InfoCard = (props: InfoCardProps) => { + const { variant = 'simple' } = props - if (props.variant === 'detailed') { + if (variant === 'detailed') { return <DetailedInfoCard {...props} /> } else { return <SimpleInfoCard {...props} /> } }This ensures that the component behaves predictably even if
variantis not explicitly provided.apps/web/components/InfoCard/InfoCardWrapper.tsx (1)
21-26: Consider memoizing the width checkThe window size check could be optimized using useMemo to prevent unnecessary re-renders.
+ const isMobileWidth = useMemo( + () => width < theme.breakpoints.md, + [width] + ) useEffect(() => { - if (width < theme.breakpoints.md) { + if (isMobileWidth) { return setIsMobile(true) } setIsMobile(false) - }, [width]) + }, [isMobileWidth])apps/web/screens/Grants/SearchResults/SearchResultsContent.tsx (2)
106-107: Address the TODO comment for application deadline text.The hardcoded deadline text needs to be replaced with the actual deadline from the data.
Would you like me to help implement the dynamic deadline text using the grant data?
58-123: Consider extracting the card data transformation logic.The current implementation has complex data transformation logic inline. Consider extracting this into a separate utility function for better maintainability and reusability.
+const transformGrantToCardData = (grant: Grant, formatMessage: FormatMessage, locale: Locale, linkResolver: LinkResolver) => { + if (!grant || !grant.applicationId) { + return null + } + + return { + eyebrow: grant.fund?.title ?? grant.name ?? '', + subEyebrow: grant.fund?.parentOrganization?.title, + // ... rest of the transformation + } +} return ( <> {grants?.length ? ( <InfoCardWrapper columns={!isGridLayout ? 1 : 2} variant="detailed" - cards={grants?.map((grant) => { - if (!grant || !grant.applicationId) { - return null - } - return { ... } - }).filter(isDefined) ?? []} + cards={grants?.map(grant => + transformGrantToCardData(grant, formatMessage, locale, linkResolver) + ).filter(isDefined) ?? []} /> ) : undefined}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (12)
.github/CODEOWNERS(1 hunks)apps/web/components/InfoCard/DetailedInfoCard.tsx(1 hunks)apps/web/components/InfoCard/InfoCard.css.ts(1 hunks)apps/web/components/InfoCard/InfoCard.tsx(1 hunks)apps/web/components/InfoCard/InfoCardWrapper.tsx(1 hunks)apps/web/components/InfoCard/SimpleInfoCard.tsx(1 hunks)apps/web/components/PlazaCard/PlazaCard.css.ts(0 hunks)apps/web/components/PlazaCard/PlazaCard.tsx(0 hunks)apps/web/components/real.ts(1 hunks)apps/web/screens/Grants/SearchResults/SearchResultsContent.tsx(2 hunks)apps/web/screens/Grants/messages.ts(1 hunks)apps/web/screens/Grants/utils.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/web/components/PlazaCard/PlazaCard.css.ts
- apps/web/components/PlazaCard/PlazaCard.tsx
🧰 Additional context used
📓 Path-based instructions (9)
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/screens/Grants/messages.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/screens/Grants/utils.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/InfoCard/InfoCard.css.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/InfoCard/InfoCard.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/InfoCard/DetailedInfoCard.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/InfoCard/SimpleInfoCard.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/Grants/SearchResults/SearchResultsContent.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/InfoCard/InfoCardWrapper.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."
🪛 Biome (1.9.4)
apps/web/components/InfoCard/DetailedInfoCard.tsx
[error] 119-119: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
apps/web/components/InfoCard/InfoCardWrapper.tsx
[error] 32-32: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 41-45: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (5)
apps/web/components/InfoCard/InfoCard.css.ts (1)
1-17: Styles are appropriately defined
The CSS styles are correctly defined using @vanilla-extract/css, providing clear and maintainable styling for the InfoCard components.
apps/web/components/real.ts (1)
15-16: Verify the removal of PlazaCard export
The PlazaCard export has been removed and replaced with InfoCard exports. Let's verify there are no remaining usages of PlazaCard in the codebase.
✅ Verification successful
PlazaCard removal appears safe
No references to PlazaCard were found in the codebase, suggesting that its removal from the exports is safe and won't cause any breaking changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to PlazaCard
rg "PlazaCard" --type ts --type tsx
# Search for any imports from the old path
rg "from ['\"]@island\.is/web/components['\"]" --type ts --type tsx -A 5 | grep -i plaza
Length of output: 186
Script:
#!/bin/bash
# Search for any remaining references to PlazaCard without file type restrictions
rg "PlazaCard"
# Search for any imports containing Plaza
rg "import.*Plaza"
# List all TypeScript and JavaScript files that might reference PlazaCard
fd "\.(ts|tsx|js|jsx)$" -x grep -l "Plaza"
Length of output: 175
apps/web/screens/Grants/SearchResults/SearchResultsContent.tsx (1)
31-54: LGTM: Layout toggle implementation is clean and follows best practices.
The implementation:
- Uses appropriate state management with useState
- Implements responsive design by hiding toggle on mobile
- Follows accessibility best practices with Button component
- Uses localized messages for button text
apps/web/screens/Grants/messages.ts (1)
9-16: LGTM: Message definitions follow established patterns.
The new messages for grid/list display options are:
- Properly structured with unique IDs
- Follow the existing naming convention
- Include appropriate default messages in Icelandic
.github/CODEOWNERS (1)
135-135: LGTM: Ownership assignment is correct.
The InfoCard component ownership is properly assigned to the @island-is/hugsmidjan team, consistent with their ownership of other web components in the grants domain.
| useEffect(() => { | ||
| if (width < theme.breakpoints.lg) { | ||
| return setIsTablet(true) | ||
| } | ||
| setIsTablet(false) | ||
| }, [width]) |
There was a problem hiding this comment.
Fix incorrect use of return in useEffect
The useEffect hook should not return the result of setIsTablet(true) as it is not a cleanup function. Returning a value from useEffect other than a cleanup function can lead to unexpected behavior.
Apply this diff to fix the issue:
useEffect(() => {
if (width < theme.breakpoints.lg) {
- return setIsTablet(true)
+ setIsTablet(true)
} else {
setIsTablet(false)
}
- }, [width])
+}, [width])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (width < theme.breakpoints.lg) { | |
| return setIsTablet(true) | |
| } | |
| setIsTablet(false) | |
| }, [width]) | |
| useEffect(() => { | |
| if (width < theme.breakpoints.lg) { | |
| setIsTablet(true) | |
| } else { | |
| setIsTablet(false) | |
| } | |
| }, [width]) |
| .map((tag) => { | ||
| if (!tag) { | ||
| return null | ||
| } | ||
| return ( | ||
| <Tag disabled variant={tag.variant}> | ||
| {tag.label} | ||
| </Tag> | ||
| ) | ||
| }) | ||
| .filter(isDefined)} | ||
| </Inline> |
There was a problem hiding this comment.
Add a unique key prop to elements in list rendering
When rendering a list of items in React using map, each element should have a unique key prop. In the tags.map() function, the Tag components are missing a key prop, which can lead to rendering issues and warnings in the console.
Apply this diff to fix the issue:
{tags
- .map((tag) => {
+ .map((tag, index) => {
if (!tag) {
return null
}
return (
- <Tag disabled variant={tag.variant}>
+ <Tag key={tag.label} disabled variant={tag.variant}>
{tag.label}
</Tag>
)
})
.filter(isDefined)}Ensure that tag.label is unique. If it is not guaranteed to be unique, consider using an index or another unique identifier.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .map((tag) => { | |
| if (!tag) { | |
| return null | |
| } | |
| return ( | |
| <Tag disabled variant={tag.variant}> | |
| {tag.label} | |
| </Tag> | |
| ) | |
| }) | |
| .filter(isDefined)} | |
| </Inline> | |
| .map((tag, index) => { | |
| if (!tag) { | |
| return null | |
| } | |
| return ( | |
| <Tag key={tag.label} disabled variant={tag.variant}> | |
| {tag.label} | |
| </Tag> | |
| ) | |
| }) | |
| .filter(isDefined)} | |
| </Inline> |
🧰 Tools
🪛 Biome (1.9.4)
[error] 119-119: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
| {cards.map((c) => ( | ||
| <InfoCard | ||
| variant={variant} | ||
| size={columns === 3 ? 'small' : 'medium'} | ||
| {...c} | ||
| /> | ||
| ))} |
There was a problem hiding this comment.
Add missing key prop to mapped InfoCard components
Similar to the previous issue, add a unique key prop here as well.
<InfoCard
+ key={c.id ?? c.title}
variant={variant}
size={columns === 3 ? 'small' : 'medium'}
{...c}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {cards.map((c) => ( | |
| <InfoCard | |
| variant={variant} | |
| size={columns === 3 ? 'small' : 'medium'} | |
| {...c} | |
| /> | |
| ))} | |
| {cards.map((c) => ( | |
| <InfoCard | |
| key={c.id ?? c.title} | |
| variant={variant} | |
| size={columns === 3 ? 'small' : 'medium'} | |
| {...c} | |
| /> | |
| ))} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 41-45: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
| {cards.map((c) => ( | ||
| <InfoCard variant={variant} size={'large'} {...c} /> | ||
| ))} |
There was a problem hiding this comment.
Add missing key prop to mapped InfoCard components
React requires a unique key prop when rendering elements in an array to optimize rendering and maintain component state correctly.
- <InfoCard variant={variant} size={'large'} {...c} />
+ <InfoCard key={c.id ?? c.title} variant={variant} size={'large'} {...c} />Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 32-32: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
| const [isTablet, setIsTablet] = useState(false) | ||
| const { width } = useWindowSize() | ||
|
|
||
| useEffect(() => { | ||
| if (width < theme.breakpoints.lg) { | ||
| return setIsTablet(true) | ||
| } | ||
| setIsTablet(false) | ||
| }, [width]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract window size logic into a custom hook
The window size check logic is duplicated from InfoCardWrapper. Consider extracting it into a shared custom hook.
Create a new hook file useDeviceSize.ts:
import { useState, useEffect } from 'react'
import { useWindowSize } from 'react-use'
import { theme } from '@island.is/island-ui/theme'
export const useDeviceSize = () => {
const [isTablet, setIsTablet] = useState(false)
const { width } = useWindowSize()
useEffect(() => {
setIsTablet(width < theme.breakpoints.lg)
}, [width])
return { isTablet }
}Then update the component:
- const [isTablet, setIsTablet] = useState(false)
- const { width } = useWindowSize()
-
- useEffect(() => {
- if (width < theme.breakpoints.lg) {
- return setIsTablet(true)
- }
- setIsTablet(false)
- }, [width])
+ const { isTablet } = useDeviceSize()| return ( | ||
| <FocusableBox | ||
| className={ | ||
| size === 'large' | ||
| ? styles.infoCardWide | ||
| : size === 'small' | ||
| ? styles.infoCardSmall | ||
| : styles.infoCard | ||
| } | ||
| component={LinkV2} | ||
| href={link.href} | ||
| background="white" | ||
| borderColor="white" | ||
| borderWidth="standard" | ||
| width="full" | ||
| borderRadius="standard" | ||
| > | ||
| <Box width="full" paddingX={4} paddingY={3}> | ||
| {renderHeader()} | ||
| {renderContent()} | ||
| </Box> | ||
| </FocusableBox> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance accessibility with ARIA labels
The card acts as a clickable link but lacks proper accessibility attributes.
<FocusableBox
+ aria-label={`${eyebrow}: ${title}`}
+ role="article"
className={
size === 'large'
? styles.infoCardWideCommittable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (8)
apps/web/components/InfoCard/DetailedInfoCard.tsx (3)
28-37: LGTM! Consider enhancing the type documentation.The type definition is well-structured. However, the inline comment about max lines could be more descriptive.
Consider using JSDoc format for better documentation:
- //max 5 lines + /** Maximum of 5 detail lines will be displayed */ detailLines?: Array<{ icon: IconMapIcon text: string }>
52-60: Consider extracting breakpoint logic to a constantThe breakpoint comparison could be more maintainable by extracting the value to a named constant.
+ const TABLET_BREAKPOINT = theme.breakpoints.lg const [isTablet, setIsTablet] = useState(false) const { width } = useWindowSize() useEffect(() => { - if (width < theme.breakpoints.lg) { + if (width < TABLET_BREAKPOINT) { setIsTablet(true) } else { setIsTablet(false) } }, [width])
156-189: Consider memoizing render functions for performanceThe render functions could benefit from memoization to prevent unnecessary re-renders.
+ const MemoizedContent = React.useMemo( + () => ( <> <Text variant="h3" color="blue400"> {title} </Text> {description && ( <Box marginTop={1}> <Text>{description}</Text> </Box> )} {renderDetails()} </> + ), + [title, description, renderDetails] + ) const renderContent = () => { if (size === 'large' && !isTablet) { return ( <GridContainer> <GridRow direction="row"> <GridColumn span="8/12"> {MemoizedContent} </GridColumn> <GridColumn span="4/12">{renderDetails()}</GridColumn> </GridRow> </GridContainer> ) } - return ( - <> - <Text variant="h3" color="blue400"> - {title} - </Text> - {description && ( - <Box marginTop={1}> - <Text>{description}</Text> - </Box> - )} - {renderDetails()} - </> - ) + return MemoizedContent }apps/web/pages/en/grants-plaza/index.ts (1)
7-7: Consider Dynamic Locale HandlingHardcoding the locale to
'en'may limit internationalization. Consider making the locale dynamic to support multiple languages and enhance user experience.apps/web/pages/en/grants-plaza/grant/[id].ts (1)
8-8: Consider Dynamic Locale for InternationalizationHardcoding
'en'as the locale may restrict access for users in other regions. Implementing dynamic locale support can enhance inclusivity.apps/web/screens/Grants/utils.ts (1)
28-116: Consider simplifying status parsing logicThe
parseStatusfunction handles multiple cases well but could be simplified by extracting common patterns.Consider:
- Extracting the deadline status formatting logic into a separate function
- Creating a mapping object for status messages to reduce repetition
- Combining similar cases like
ClosedOpeningSoonandClosedOpeningSoonWithEstimationExample of status message mapping:
const STATUS_MESSAGES = { [GrantStatus.Closed]: (grant: Grant, locale: Locale) => grant.dateTo ? formatMessage( containsTimePart(grant.dateTo) ? m.search.applicationWasOpenToAndWith : m.search.applicationWasOpenTo, { arg: formatDate(new Date(grant.dateTo), locale) } ) : formatMessage(m.search.applicationClosed), // ... other status messages }apps/web/screens/Grants/SearchResults/SearchResultsContent.tsx (1)
111-112: Address the TODO comment for deadline textThe comment indicates that the deadline text implementation needs to be completed.
Would you like me to help implement the deadline text logic or create a GitHub issue to track this task?
libs/cms/src/lib/cms.elasticsearch.service.ts (1)
Line range hint
486-580: LGTM: Well-structured grant search implementationThe getGrants method:
- Properly handles search relevance scoring
- Maintains efficient pagination
- Uses appropriate field boosting for title matches
- Implements proper sorting fallback
Consider adding caching for frequently accessed grants to improve performance, especially for popular search queries.
📜 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 (16)
apps/web/components/InfoCard/DetailedInfoCard.tsx(4 hunks)apps/web/components/InfoCard/InfoCard.tsx(1 hunks)apps/web/components/InfoCard/SimpleInfoCard.tsx(1 hunks)apps/web/pages/en/grants-plaza/grant/[id].ts(1 hunks)apps/web/pages/en/grants-plaza/grants/index.ts(1 hunks)apps/web/pages/en/grants-plaza/index.ts(1 hunks)apps/web/screens/Grants/Grant/Grant.tsx(1 hunks)apps/web/screens/Grants/Grant/GrantSidebar.tsx(5 hunks)apps/web/screens/Grants/Home/GrantsHome.tsx(1 hunks)apps/web/screens/Grants/SearchResults/SearchResultsContent.tsx(2 hunks)apps/web/screens/Grants/messages.ts(3 hunks)apps/web/screens/Grants/utils.ts(1 hunks)apps/web/screens/queries/Grants.ts(2 hunks)libs/cms/src/lib/cms.elasticsearch.service.ts(1 hunks)libs/cms/src/lib/models/grant.model.ts(4 hunks)libs/cms/src/lib/search/importers/grants.service.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/components/InfoCard/InfoCard.tsx
- apps/web/components/InfoCard/SimpleInfoCard.tsx
🧰 Additional context used
📓 Path-based instructions (14)
apps/web/pages/en/grants-plaza/grants/index.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/pages/en/grants-plaza/grant/[id].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/pages/en/grants-plaza/index.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/screens/Grants/Home/GrantsHome.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/Grants/Grant/Grant.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/Grants/SearchResults/SearchResultsContent.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/InfoCard/DetailedInfoCard.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/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."
apps/web/screens/Grants/utils.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/cms.elasticsearch.service.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/search/importers/grants.service.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/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."
apps/web/screens/Grants/messages.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."
🪛 Biome (1.9.4)
apps/web/components/InfoCard/DetailedInfoCard.tsx
[error] 119-119: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (33)
apps/web/components/InfoCard/DetailedInfoCard.tsx (2)
55-60: Fix incorrect use of return in useEffect
The useEffect hook should not return the result of setIsTablet(true) as it is not a cleanup function.
114-125:
Add missing key prop to Tag component
React requires a unique key prop when rendering lists of elements.
- <Tag disabled variant={tag.variant}>
+ <Tag key={tag.label} disabled variant={tag.variant}>
{tag.label}
</Tag>Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 119-119: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
apps/web/screens/Grants/Grant/GrantSidebar.tsx (8)
2-2: Good Use of useIntl Hook
Replacing useLocale with useIntl improves internationalization handling and aligns with best practices using react-intl.
19-19: Imported Utilities Enhance Status Parsing
Importing generateStatusTag and parseStatus consolidates status-related logic, promoting modularity and reusability.
44-44: Condition Updated for Clarity
Checking !data.length explicitly handles empty arrays, improving readability over a generic falsy check.
56-57: Destructuring formatMessage from useIntl
This aligns with react-intl usage patterns and ensures consistent access to internationalization functions.
58-61: Proper Memoization with Correct Dependencies
Using useMemo for status calculation optimizes performance, and including grant, formatMessage, and locale in dependencies ensures accuracy.
103-104: Displaying deadlineStatus Appropriately
Conditional rendering of status.deadlineStatus maintains robustness by handling cases where the deadline status may be undefined.
111-114: Correct Usage of generateStatusTag
Generating the status label using status.applicationStatus and formatMessage ensures accurate status representation.
119-119: Updated Dependencies in useMemo Hook
Including status in the dependency array of detailPanelData ensures that memoization updates when the status changes.
libs/cms/src/lib/models/grant.model.ts (7)
3-3: Importing IGrantFields for Type Safety
Adding IGrantFields import enhances type safety when accessing grant fields, preventing potential runtime errors.
12-14: Importing Date Utility Functions
Incorporating format, addHours, and isValidDate improves date handling and formatting within the model.
18-25: Expanded GrantStatus Enum
Adding new statuses to GrantStatus allows for more granular representation of grant states, improving clarity across the application.
68-69: Added statusText Field to Grant Model
Including statusText provides additional context for grant statuses, enabling display of custom status notes.
87-119: Effective Implementation of parseStatus Function
The parseStatus function correctly interprets grant statuses based on provided fields and dates, enhancing the accuracy of status classification.
121-131: Creation of parseDate Function for Consistent Date Parsing
The parseDate utility ensures consistent date formatting and correctly adjusts for provided times, improving date data integrity.
133-169: Refactored mapGrant Function with Enhanced Logic
Utilizing parseDate and parseStatus within mapGrant improves data mapping accuracy and maintains consistency across the grant model.
apps/web/pages/en/grants-plaza/index.ts (1)
4-4: Proper Use of getServerSidePropsWrapper
Implementing getServerSidePropsWrapper ensures server-side rendering functions are correctly wrapped, aligning with Next.js best practices.
apps/web/pages/en/grants-plaza/grant/[id].ts (1)
4-4: Effective Use of Server-Side Rendering Wrapper
Employing getServerSidePropsWrapper facilitates proper data fetching on the server side, adhering to Next.js server-side rendering techniques.
apps/web/pages/en/grants-plaza/grants/index.ts (1)
1-12: LGTM - Clean NextJS page implementation
The implementation follows NextJS best practices:
- Proper use of HOCs for Apollo and localization
- Correct setup of getServerSideProps
- Clean separation of concerns
apps/web/screens/queries/Grants.ts (2)
70-72: LGTM - Clean field additions
The addition of statusText, dateFrom, and dateTo fields in GET_GRANT_QUERY aligns with the new status handling approach.
19-19: Verify the removal of applicationDeadlineStatus field
The statusText field has replaced applicationDeadlineStatus and isOpen. Please ensure all components consuming this query have been updated to use the new field structure.
✅ Verification successful
Let me search for more specific information about the GraphQL query usage.
Let me check the actual implementation of the components consuming this query.
Let me check how the status fields are being used in the components.
Let me check the Grant type definition to understand the status fields.
Based on the evidence gathered, I can now provide the final response:
The removal of applicationDeadlineStatus field appears safe
The codebase search shows no direct usage of the removed fields applicationDeadlineStatus or isOpen in relation to the grants query. The only isOpen occurrences found are related to UI components like accordions, menus, and modals. The new statusText field is part of the schema change, and since GraphQL enforces schema compliance at build time, any breaking changes would have been caught during compilation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining usage of old fields
rg "applicationDeadlineStatus|isOpen" --type ts
Length of output: 11154
Script:
#!/bin/bash
# Search for references to the GET_GRANTS_QUERY
rg "GET_GRANTS_QUERY" -A 5 --type ts
Length of output: 2130
Script:
#!/bin/bash
# Check the SearchResultsContent implementation
rg -A 10 "SearchResultsContent" --type ts
Length of output: 3850
Script:
#!/bin/bash
# Check for status-related field usage in the components
ast-grep --pattern 'statusText'
Length of output: 346
Script:
#!/bin/bash
# Search for Grant type/interface definition
ast-grep --pattern 'type Grant = {
$$$
}'
# Also try interface
ast-grep --pattern 'interface Grant {
$$$
}'
Length of output: 94
apps/web/screens/Grants/utils.ts (2)
11-15: LGTM - Well-defined Status interface
The Status interface provides good type safety for the application status handling.
118-130: LGTM - Clean status tag generation
The generateStatusTag function is well-implemented with good type safety and clear conditional logic.
libs/cms/src/lib/search/importers/grants.service.ts (1)
106-117: LGTM: Status handling is well-structured
The new status cases are logically grouped based on their tag type (open/closed), making the code more maintainable and easier to understand.
apps/web/screens/Grants/SearchResults/SearchResultsContent.tsx (4)
1-1: LGTM: Clean implementation of grid layout state
The imports and state management are well-organized, following React best practices for implementing the new grid layout feature.
Also applies to: 6-6, 10-10, 15-15, 31-32
36-54: LGTM: Well-implemented layout toggle
The layout toggle button is properly implemented with:
- Responsive behavior (hidden on mobile)
- Dynamic icon and label based on the current layout
- Consistent styling using the design system
58-128: LGTM: Clean InfoCardWrapper implementation
The InfoCardWrapper implementation is well-structured with proper data mapping and null checks.
Line range hint 129-165: LGTM: Well-implemented empty state
The empty state implementation follows best practices with:
- Clear messaging
- Responsive image handling
- Proper accessibility
apps/web/screens/Grants/Home/GrantsHome.tsx (1)
45-45: LGTM: Clean intl handling
The reorganization of intl handling maintains functionality while improving code organization.
Also applies to: 47-47
apps/web/screens/Grants/messages.ts (2)
9-16: LGTM: Display option messages are well-structured
The new display option messages follow proper i18n patterns and naming conventions.
63-90: LGTM: Application status messages are comprehensive
The new application status messages:
- Follow proper i18n patterns
- Use consistent argument placeholders
- Cover all necessary status scenarios
libs/cms/src/lib/cms.elasticsearch.service.ts (1)
50-50: LGTM: Clean import statement
The removal of unused imports improves code cleanliness.
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore make web strict |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid Suppressing TypeScript Errors
Disabling linting and TypeScript checks with eslint-disable and @ts-ignore can hide potential issues. It's recommended to resolve the TypeScript errors to maintain strict type safety.
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore make web strict |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid Suppressing TypeScript Errors
Using eslint-disable and @ts-ignore suppresses important checks. Addressing the underlying TypeScript issues will improve code reliability.
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore make web strict | ||
| // |
There was a problem hiding this comment.
Remove or properly fix the ts-ignore comment
The @ts-ignore comment suggests underlying TypeScript issues that should be addressed rather than suppressed. The comment mentions "make web strict" which indicates this might be related to strict mode configuration.
Please investigate and fix the underlying TypeScript issue. If you need assistance debugging the type error, I can help analyze it.
| const formatDate = ( | ||
| date: Date, | ||
| locale: Locale, | ||
| stringFormat = 'dd.MMMM yyyy', | ||
| ) => | ||
| format(date, stringFormat, { | ||
| locale: locale === 'is' ? localeIS : localeEn, | ||
| }) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for date formatting
The formatDate function should handle invalid date inputs to prevent runtime errors.
const formatDate = (
date: Date,
locale: Locale,
stringFormat = 'dd.MMMM yyyy',
-) =>
- format(date, stringFormat, {
- locale: locale === 'is' ? localeIS : localeEn,
- })
+) => {
+ try {
+ return format(date, stringFormat, {
+ locale: locale === 'is' ? localeIS : localeEn,
+ })
+ } catch (error) {
+ console.error('Error formatting date:', error)
+ return ''
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const formatDate = ( | |
| date: Date, | |
| locale: Locale, | |
| stringFormat = 'dd.MMMM yyyy', | |
| ) => | |
| format(date, stringFormat, { | |
| locale: locale === 'is' ? localeIS : localeEn, | |
| }) | |
| const formatDate = ( | |
| date: Date, | |
| locale: Locale, | |
| stringFormat = 'dd.MMMM yyyy', | |
| ) => { | |
| try { | |
| return format(date, stringFormat, { | |
| locale: locale === 'is' ? localeIS : localeEn, | |
| }) | |
| } catch (error) { | |
| console.error('Error formatting date:', error) | |
| return '' | |
| } | |
| } |
| @@ -1,6 +1,7 @@ | |||
| import { useIntl } from 'react-intl' | |||
| import NextLink from 'next/link' | |||
| import { useRouter } from 'next/router' | |||
| import { generateStatusTag, parseStatus } from '../utils' | |||
There was a problem hiding this comment.
💡 Codebase verification
Imported utilities are not used in Grant.tsx
While the utilities generateStatusTag and parseStatus are imported in Grant.tsx, they are not actually used within this file. However, from our previous search results, we can see these utilities are properly used in sibling components:
GrantSidebar.tsx: Uses both utilities for status displaySearchResultsContent.tsx: Uses both utilities for search results display
The import in Grant.tsx appears to be unused and should be removed.
🔗 Analysis chain
Verify the integration of status utilities
The newly imported utilities generateStatusTag and parseStatus should be properly integrated with the grant status handling logic.
Let me generate additional verification to check the usage of these utilities in the Grant.tsx file specifically.
Let me try one more verification with a different approach to check the usage in Grant.tsx.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of imported status utilities
# Test: Search for usage of generateStatusTag and parseStatus
rg -A 5 'generateStatusTag|parseStatus' apps/web/screens/Grants/
Length of output: 4245
Script:
#!/bin/bash
# Search for actual usage of these utilities in Grant.tsx
ast-grep --pattern 'parseStatus($$$)' apps/web/screens/Grants/Grant/Grant.tsx
ast-grep --pattern 'generateStatusTag($$$)' apps/web/screens/Grants/Grant/Grant.tsx
Length of output: 165
Script:
#!/bin/bash
# Get the content of Grant.tsx to verify the usage
cat apps/web/screens/Grants/Grant/Grant.tsx
Length of output: 6114
What
New
<InfoCard />component for webScreenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
InfoCard,DetailedInfoCard, andSimpleInfoCardcomponents for enhanced display of grant information.SearchResultsContentcomponent to switch between grid and list views for displaying grants.Bug Fixes
SearchResultsContentcomponent to ensure valid grant entries are processed.Documentation
Chores
PlazaCardcomponent and associated styles.