feat(native-app): add locale to vaccinations query #16813
Conversation
WalkthroughThis pull request introduces several new GraphQL queries related to health data, including 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: 1
🧹 Outside diff range and nitpick comments (4)
apps/native/app/src/screens/vaccinations/vaccinations.tsx (1)
57-59: Consider adding locale-specific error handlingThe locale integration looks good and aligns with the PR objectives. However, consider these improvements:
- Add error handling for cases where locale is undefined
- Consider handling loading states during locale changes
const locale = useLocale() + // Ensure we have a valid locale, fallback to default if needed + const safeLocale = locale || 'is' - const vaccinationsRes = useGetVaccinationsQuery({ variables: { locale } }) + const vaccinationsRes = useGetVaccinationsQuery({ + variables: { locale: safeLocale }, + // Optionally add loading state handling for locale changes + notifyOnNetworkStatusChange: true + })apps/native/app/src/screens/health/health-overview.tsx (3)
Line range hint
292-321: Consider simplifying the date formatting logicWhile the code is functionally correct, the nested ternary operation for date formatting could be more readable.
Consider extracting the date formatting logic:
-value={ - healthInsuranceData?.from - ? intl.formatDate(healthInsuranceData.from) - : null -} +value={formatInsuranceDate(healthInsuranceData?.from, intl)} +// Add this helper function +const formatInsuranceDate = (date: string | null | undefined, intl: IntlShape) => { + return date ? intl.formatDate(date) : null +}
Line range hint
342-365: Extract repeated currency formatting logicThe currency formatting pattern is repeated across multiple inputs. Consider extracting this logic to reduce duplication.
Consider creating a helper function:
+const formatCurrency = (amount: number | null | undefined, intl: IntlShape) => { + return amount ? `${intl.formatNumber(amount)} kr.` : '0 kr.' +} -value={ - paymentStatusData?.maximumMonthlyPayment - ? `${intl.formatNumber( - paymentStatusData?.maximumMonthlyPayment, - )} kr.` - : '0 kr.' -} +value={formatCurrency(paymentStatusData?.maximumMonthlyPayment, intl)}
Line range hint
1-456: Consider breaking down this component for better maintainabilityThe component currently handles multiple concerns (health insurance, payments, medicine purchases). Consider splitting it into smaller, focused components:
- HealthInsuranceSection
- PaymentStatusSection
- MedicinePurchaseSection
This would improve:
- Code organization
- Reusability
- Testing
- Maintenance
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
apps/native/app/src/graphql/queries/health.graphql(1 hunks)apps/native/app/src/screens/health/health-overview.tsx(8 hunks)apps/native/app/src/screens/vaccinations/components/vaccination-card.tsx(3 hunks)apps/native/app/src/screens/vaccinations/vaccinations.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/native/app/src/graphql/queries/health.graphql (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/native/app/src/screens/health/health-overview.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/native/app/src/screens/vaccinations/components/vaccination-card.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/native/app/src/screens/vaccinations/vaccinations.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 (7)
apps/native/app/src/graphql/queries/health.graphql (2)
Line range hint 1-52: Well-structured and properly typed GraphQL queries!
The new queries are well-organized, properly typed, and follow GraphQL best practices. Each query serves a specific purpose and includes all necessary fields for comprehensive health data retrieval.
53-54: Locale parameter implementation looks good!
The addition of the locale parameter is clean and properly typed. This change aligns with the PR objective to support localized vaccination names.
Let's verify the locale handling in the schema:
✅ Verification successful
Locale parameter implementation is consistent with the codebase patterns
The verification shows that:
- The
localeparameter is widely used across multiple GraphQL queries in the codebase - Other health-related queries like
healthDirectorateOrganDonationalso use the same locale pattern - The implementation follows the established pattern of accepting
locale: $localeas a parameter
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the GraphQL schema properly defines the locale parameter
# and that the healthDirectorateVaccinations field accepts it
# Search for the type definition of healthDirectorateVaccinations
ast-grep --pattern 'type Query {
$$$
healthDirectorateVaccinations($$$): $$$
$$$
}'
# Search for any existing locale-aware queries for consistency
rg 'locale: \$locale' -A 2
Length of output: 8933
apps/native/app/src/screens/vaccinations/vaccinations.tsx (2)
17-17: LGTM: Clean import addition
The useLocale hook import follows the established import pattern and is correctly placed with other hook imports.
59-59: Verify vaccine name display implementation
The PR objectives mention ensuring vaccine names display without URLs, but this implementation isn't visible in the current changes. Let's verify the handling in related components.
✅ Verification successful
Vaccine name display implementation is correct
The implementation correctly handles vaccine names both with and without URLs:
- When a URL is present, it's displayed as a clickable link with an external link icon
- When no URL is present, it's displayed as plain text using Typography component
- The name is always shown without the URL being visible to the user
This matches the PR objective of ensuring proper vaccine name display.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for vaccine name rendering logic in related components
echo "Searching for VaccinationsCard component implementation..."
rg -t tsx "VaccinationsCard" ./apps/native/app/src/screens/vaccinations/
Length of output: 227
Script:
#!/bin/bash
# Search for vaccine name rendering logic in related components with correct file extension
echo "Searching for VaccinationsCard component and name rendering..."
rg "VaccinationsCard" ./apps/native/app/src/screens/vaccinations/
rg "vaccine.*name|name.*vaccine" ./apps/native/app/src/screens/vaccinations/
Length of output: 612
Script:
#!/bin/bash
# Let's examine the vaccination-card component implementation
echo "Checking vaccination-card component implementation..."
cat ./apps/native/app/src/screens/vaccinations/components/vaccination-card.tsx
# Also check the query structure to see how vaccine names are handled
echo -e "\nChecking vaccination query structure..."
rg -A 5 "useGetVaccinationsQuery|query GetVaccinations" ./apps/native/app/src/
Length of output: 11457
apps/native/app/src/screens/vaccinations/components/vaccination-card.tsx (1)
173-173: Verify the layout changes and document the rationale.
The maxWidth increase from 20% to 21% for date columns seems arbitrary. Please verify that:
- This change doesn't cause text overflow or alignment issues with different date formats and screen sizes
- The specific value of 21% is necessary and optimal
Also applies to: 244-244
✅ Verification successful
The width adjustment is part of a consistent table layout design
The change from 20% to 21% for the date column is justified as it's part of a well-structured table layout where columns have specific width allocations:
- Number column: 6% width
- Date column: 21% width
- Age column: 25% width
The remaining space (48%) appears to be allocated for the vaccine name column, creating a balanced table layout. The slight increase to 21% for dates ensures proper spacing for formatted dates while maintaining overall table proportions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other width-related changes or discussions
rg -A 2 "maxWidth.*21%"
rg -A 2 "maxWidth.*20%"
Length of output: 814
Script:
#!/bin/bash
# Search for similar Cell components with width constraints to understand the layout pattern
rg -A 5 "<Cell.*style.*maxWidth"
# Search for the entire table structure to understand the column distribution
ast-grep --pattern 'const VaccinationCard = $_ => {
$$$
return (
$$$
<Table>
$$$
</Table>
$$$
)
}'
Length of output: 2246
apps/native/app/src/screens/health/health-overview.tsx (2)
122-127: Well-structured data access refactoring!
The introduction of these variables improves code readability and maintains proper TypeScript type inference from GraphQL queries. This refactoring aligns with React best practices for organizing data access patterns.
Line range hint 376-394: Apply the currency formatting helper here as well
This section would also benefit from the suggested currency formatting helper function to maintain consistency and reduce code duplication.
apps/native/app/src/screens/vaccinations/components/vaccination-card.tsx
Show resolved
Hide resolved
thoreyjona
added
the
deprecated:automerge
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/native/app/src/ui/lib/card/expandable-card.tsx (1)
66-70: LGTM: Well-structured styled componentThe new Message component effectively encapsulates layout properties and spacing, following React Native best practices for component composition.
Consider adding a type annotation for better TypeScript safety:
-const Message = styled(Typography)` +const Message = styled(Typography)<{ variant: string }>`
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
apps/native/app/src/screens/vaccinations/vaccinations.tsx(4 hunks)apps/native/app/src/ui/lib/card/expandable-card.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/native/app/src/screens/vaccinations/vaccinations.tsx
🧰 Additional context used
📓 Path-based instructions (1)
apps/native/app/src/ui/lib/card/expandable-card.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 (2)
apps/native/app/src/ui/lib/card/expandable-card.tsx (2)
57-57: LGTM: Improved layout flexibility
The addition of flex: 1 ensures the IconMessage container properly expands to fill available space, which is a good practice in flex layouts.
192-192: LGTM: Consistent component usage
The replacement of Typography with the new Message component maintains the heading5 variant while benefiting from the improved layout properties.
* feat: add locale to vaccination query * Fix: show vaccination name even if there is no url * feat: move data into variables for health * feat: wrap vaccination in two lines if no space * fix: make sure loading is shown for vaccinations --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Improvements
These updates enhance the overall user experience by providing more relevant and accessible health-related information.