feat(native-app): add useLocale hook #16357
Conversation
WalkthroughA new custom hook named Changes
Possibly related PRs
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 (4)
apps/native/app/src/hooks/use-locale.ts (1)
3-6: Consider adding TypeScript type annotations for better type safety.The
useLocalehook implementation looks good and follows efficient state management practices by usingusePreferencesStore. However, to improve type safety and adhere to TypeScript best practices, consider adding explicit type annotations.Here's a suggested improvement:
export const useLocale = (): 'is' | 'en' => { const locale = usePreferencesStore(({ locale }: { locale: string }) => locale) return locale === 'is-IS' ? 'is' : 'en' }This change:
- Explicitly defines the return type of the hook as
'is' | 'en'.- Adds type annotation for the
localeparameter in the selector function.These additions will enhance type checking and improve code readability.
apps/native/app/src/screens/applications/applications-incomplete.tsx (2)
42-42: Approved: useLocale implementation with a suggestion for type safety.The direct usage of
useLocale()in the query simplifies the locale determination logic and aligns with the PR objectives. It enhances reusability and reduces the likelihood of typographical errors.Consider adding type annotation for better type safety:
locale: useLocale() as string,This ensures that the
localeproperty explicitly receives a string value, which might be expected by the GraphQL query.
Line range hint
1-62: Overall structure adheres to best practices with room for improvement.The component follows NextJS and TypeScript best practices, including proper file structure, use of custom hooks, and correct typing for the component.
To further enhance type safety, consider adding explicit type annotations for the GraphQL query result:
const applicationsRes = useListApplicationsQuery<ListApplicationsQuery, ListApplicationsQueryVariables>({ // ... existing code });This ensures that the query result and variables are properly typed, reducing the likelihood of runtime errors and improving developer experience with better autocomplete suggestions.
apps/native/app/src/screens/applications/applications-completed.tsx (1)
46-46: Approved: Effective use of useLocale hookThe implementation of
useLocale()aligns well with the PR objectives. It simplifies locale handling, reduces the chance of typos, and enhances reusability. This change is consistent with the provided summary and maintains type safety through TypeScript inference.Consider memoizing the locale value to optimize performance:
const locale = useLocale(); const applicationsRes = useListApplicationsQuery({ variables: { input: { status: [ ApplicationResponseDtoStatusEnum.Completed, ApplicationResponseDtoStatusEnum.Rejected, ApplicationResponseDtoStatusEnum.Approved, ], }, locale, }, });This approach can prevent unnecessary re-renders if
useListApplicationsQueryis sensitive to prop changes.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
- apps/native/app/src/hooks/use-locale.ts (1 hunks)
- apps/native/app/src/screens/applications/applications-completed.tsx (2 hunks)
- apps/native/app/src/screens/applications/applications-in-progress.tsx (2 hunks)
- apps/native/app/src/screens/applications/applications-incomplete.tsx (2 hunks)
- apps/native/app/src/screens/applications/applications.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
apps/native/app/src/hooks/use-locale.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/native/app/src/screens/applications/applications-completed.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/applications/applications-in-progress.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/applications/applications-incomplete.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/applications/applications.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 (11)
apps/native/app/src/hooks/use-locale.ts (2)
1-1: LGTM: Import statement is correct and follows best practices.The import statement correctly imports
usePreferencesStorefrom the relative path. This adheres to NextJS best practices for importing custom hooks and stores.
3-6: Verify the usage of 'is-IS' locale code.The implementation assumes that 'is-IS' is the correct locale code for Icelandic. It's important to ensure this is consistent with the rest of the application and follows ISO language codes.
Run the following script to verify the usage of 'is-IS' locale code across the codebase:
This script will help ensure consistency in locale handling across the application.
✅ Verification successful
Usage of 'is-IS' locale code is correct and consistent with ISO language codes and is uniformly applied across the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of 'is-IS' locale code across the codebase. # Test: Search for 'is-IS' usage in TypeScript and JavaScript files echo "Occurrences of 'is-IS' in TypeScript and JavaScript files:" rg --type-add 'script:*.{ts,js}' --type script "'is-IS'" -n # Test: Search for other potential Icelandic locale codes echo "\nOther potential Icelandic locale codes:" rg --type-add 'script:*.{ts,js}' --type script "'is'|'is-is'|'IS'|'IS-is'" -n # Test: Search for locale-related configurations echo "\nLocale-related configurations:" rg --type json --type yaml "locale.*is" -nLength of output: 64145
apps/native/app/src/screens/applications/applications-incomplete.tsx (1)
10-10: LGTM: Import statement for useLocale hook.The import statement for the new
useLocalehook is correctly added and follows React conventions for custom hooks. This change aligns with the PR objective of introducing theuseLocalehook to reduce typographical errors and enhance reusability.apps/native/app/src/screens/applications/applications-in-progress.tsx (3)
10-10: LGTM: New import for useLocale hookThe import statement for the
useLocalehook is correctly placed and follows the existing import pattern in the file. This addition aligns with the PR objective of introducing theuseLocalehook to reduce typographical errors and enhance reusability.
42-42: Excellent use of the new useLocale hookThe update to use
useLocale()for thelocaleproperty inuseListApplicationsQueryis a great improvement. It simplifies locale handling, eliminates the need for conditional checks, and adheres to React best practices by utilizing a custom hook for shared logic. This change effectively meets the PR objectives of reducing typographical errors and enhancing code reusability.
Line range hint
1-58: Overall assessment: Excellent adherence to best practicesThe changes in this file maintain and improve upon existing best practices:
- NextJS best practices: The file structure and component organization remain consistent with NextJS conventions.
- TypeScript usage: The code continues to leverage TypeScript for type safety, particularly in component props and GraphQL query results.
- State management: The introduction of the
useLocalehook simplifies state management by centralizing locale logic.- Code organization: The component remains focused on its primary responsibility, with the new changes enhancing its clarity and maintainability.
These improvements align well with the PR objectives and contribute to a more robust and maintainable codebase.
apps/native/app/src/screens/applications/applications-completed.tsx (2)
10-10: LGTM: Import statement for useLocaleThe import statement for the
useLocalehook follows React naming conventions and uses a correct relative path. This adheres to NextJS best practices for organizing custom hooks.
Line range hint
1-63: Overall: Excellent adherence to best practicesThe file
applications-completed.tsxdemonstrates strong adherence to NextJS best practices, efficient state management, and optimal use of TypeScript for type safety. The introduction of theuseLocalehook enhances code reusability and maintainability.Key observations:
- Proper file structure and naming conventions for a NextJS application.
- Effective use of React hooks (
useState,useListApplicationsQuery,useLocale).- Clear separation of concerns with custom hooks (
useNavigationOptions,useConnectivityIndicator).- Consistent use of TypeScript for improved type safety.
The changes introduced in this PR are focused and improve the overall code quality without disrupting the existing functionality.
apps/native/app/src/screens/applications/applications.tsx (3)
17-17: LGTM: Import statement for useLocale hook.The import statement for the
useLocalehook is correctly added and follows the expected relative path structure. This change aligns with the PR objective of introducing theuseLocalehook to reduce typographical errors and enhance reusability.
Line range hint
1-207: Summary: Successful implementation of useLocale hookThe changes in this file successfully introduce the
useLocalehook, aligning with the PR objectives. The modifications improve locale management by removing direct dependency on the preferences store, which should reduce the likelihood of typographical errors and enhance reusability.Key points:
- The
useLocalehook is correctly imported and used.- The changes are minimal and focused, maintaining the overall structure and logic of the
ApplicationsScreencomponent.- The implementation adheres to NextJS best practices and makes efficient use of TypeScript.
These changes contribute to a more maintainable and consistent codebase. Great job!
107-107: LGTM: Usage of useLocale hook in query variables.The
useLocalehook is correctly implemented in theuseListApplicationsQueryvariables. This change simplifies the code and aligns with the PR objective of using the new hook for locale information in queries.To ensure consistency, let's verify the usage of
useLocaleacross the codebase:✅ Verification successful
LGTM: Consistent usage of
useLocalehook across the codebase.All instances of locale handling have been updated to use the
useLocalehook, and there are no remaining direct accesses to locale outside the hook's implementation. This ensures consistency and aligns with the PR's objective.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent usage of useLocale hook # Test 1: Search for useLocale import statements echo "Checking useLocale import statements:" rg --type typescript --type tsx "import.*useLocale.*from.*hooks/use-locale" # Test 2: Search for useLocale usage in query variables echo "\nChecking useLocale usage in query variables:" rg --type typescript --type tsx "variables:.*\{.*locale:.*useLocale\(\).*\}" # Test 3: Search for any remaining direct access to locale in preferences store echo "\nChecking for any remaining direct access to locale in preferences store:" rg --type typescript --type tsx "usePreferencesStore.*locale"Length of output: 672
Script:
#!/bin/bash # Description: Check for consistent usage of useLocale hook # Test 1: Search for useLocale import statements echo "Checking useLocale import statements:" rg --type=ts --type=tsx "import.*useLocale.*from.*hooks/use-locale" # Test 2: Search for useLocale usage in query variables echo "\nChecking useLocale usage in query variables:" rg --type=ts --type=tsx "variables:\s*\{\s*locale:\s*useLocale\(\)\s*\}" # Test 3: Search for any remaining direct access to locale in preferences store echo "\nChecking for any remaining direct access to locale in preferences store:" rg --type=ts --type=tsx "usePreferencesStore.*locale"Length of output: 631
Script:
#!/bin/bash # Description: Check for consistent usage of useLocale hook # Test 1: Search for useLocale import statements echo "Checking useLocale import statements:" rg -g "*.ts" -g "*.tsx" "import.*useLocale.*from.*hooks/use-locale" # Test 2: Search for useLocale usage in query variables echo "\nChecking useLocale usage in query variables:" rg -g "*.ts" -g "*.tsx" "variables:\s*\{\s*locale:\s*useLocale\(\)\s*\}" # Test 3: Search for any remaining direct access to locale in preferences store echo "\nChecking for any remaining direct access to locale in preferences store:" rg -g "*.ts" -g "*.tsx" "usePreferencesStore.*locale"Length of output: 1215
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16357 +/- ##
==========================================
- Coverage 36.73% 36.73% -0.01%
==========================================
Files 6808 6808
Lines 141061 141049 -12
Branches 40217 40213 -4
==========================================
- Hits 51817 51808 -9
+ Misses 89244 89241 -3
Flags with carried forward coverage won't be shown. Click here to find out more. see 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportBranch report: ✅ 0 Failed, 84 Passed, 0 Skipped, 30.74s Total Time |
thoreyjona
force-pushed
the
feat/use-locale-hook
branch
from
beeebf7 to
8e7a41c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/native/app/src/screens/document-detail/document-detail.tsx (1)
295-295: LGTM: Usage of useLocale hookThe replacement of
usePreferencesStorewithuseLocale()for obtaining the locale is a good improvement. It aligns with the PR objective and should help reduce typographical errors.For enhanced type safety, consider explicitly typing the
localeparameter:locale: useLocale() as string,This ensures that the
localevalue matches the expected type in the query variables.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
- apps/native/app/src/hooks/use-locale.ts (1 hunks)
- apps/native/app/src/screens/applications/applications-completed.tsx (2 hunks)
- apps/native/app/src/screens/applications/applications-in-progress.tsx (2 hunks)
- apps/native/app/src/screens/applications/applications-incomplete.tsx (2 hunks)
- apps/native/app/src/screens/applications/applications.tsx (2 hunks)
- apps/native/app/src/screens/document-detail/document-detail.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/native/app/src/hooks/use-locale.ts
- apps/native/app/src/screens/applications/applications-completed.tsx
- apps/native/app/src/screens/applications/applications-in-progress.tsx
- apps/native/app/src/screens/applications/applications-incomplete.tsx
- apps/native/app/src/screens/applications/applications.tsx
🧰 Additional context used
📓 Path-based instructions (1)
apps/native/app/src/screens/document-detail/document-detail.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/screens/document-detail/document-detail.tsx (2)
33-33: LGTM: Import of useLocale hookThe addition of the
useLocalehook import aligns with the PR objective and follows proper import conventions. This change will help reduce typographical errors and enhance reusability across the codebase.
Line range hint
1-524: Overall assessment: Positive changes with minor improvement suggestionThe introduction of the
useLocalehook and its usage in theDocumentDetailScreencomponent aligns well with the PR objectives. These changes should improve code consistency and reduce the likelihood of typographical errors when dealing with locale information.The implementation adheres to NextJS and TypeScript best practices, and the changes are minimal with no apparent breaking changes or performance issues. The suggestion for explicit typing of the
localeparameter is a minor improvement for enhanced type safety.Consider adding unit tests for the
useLocalehook and updating the documentation to reflect these changes, as mentioned in the PR checklist.To ensure that the
useLocalehook is used consistently across the codebase, run the following script:✅ Verification successful
Locale Hook Migration Verified
The verification confirms that there are no remaining instances of
usePreferencesStorefor locale handling. All locale-related functionalities have been successfully migrated touseLocale, ensuring consistency and adherence to the PR objectives.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining uses of usePreferencesStore for locale and compare with useLocale usage echo "Checking for usePreferencesStore usage for locale:" rg "usePreferencesStore\(\).*locale" --type typescript echo "\nChecking for useLocale usage:" rg "useLocale\(\)" --type typescript echo "\nNote: Compare the outputs to ensure useLocale is being used consistently where appropriate."Length of output: 546
Script:
#!/bin/bash # Description: Check for any remaining uses of usePreferencesStore for locale and compare with useLocale usage echo "Checking for usePreferencesStore usage for locale:" rg "usePreferencesStore\(\).*locale" -g "*.ts" -g "*.tsx" echo "\nChecking for useLocale usage:" rg "useLocale\(\)" -g "*.ts" -g "*.tsx" echo "\nNote: Compare the outputs to ensure useLocale is being used consistently where appropriate."Length of output: 135703
thoreyjona
added
automerge
What
Add useLocale hook to use when sending locale into queries.
Why
Prevent typos like happened with applications and to be reusable
Checklist:
Summary by CodeRabbit
New Features
useLocalefor simplified locale management based on user preferences.Improvements
useLocalehook, enhancing the clarity and efficiency of locale handling.DocumentDetailScreento improve document fetching logic and rendering based on the new locale management approach.