Feat/google maps link #10003
Feat/google maps link #10003
Conversation
WalkthroughThe pull request introduces a feature to add a Google Maps link for facilities by extending the Changes
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@rithviknishad Kindly review this. |
![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
🧹 Nitpick comments (1)
src/types/facility/facility.ts (1)
31-32: Consider using number type for geographical coordinates.The latitude and longitude properties are currently typed as string, but they represent numerical values. Consider:
- Using
numbertype instead ofstringfor better type safety and to avoid unnecessary string-to-number conversions.- Adding validation constraints using JSDoc or custom type guards to ensure valid coordinate ranges:
- Latitude: -90 to 90
- Longitude: -180 to 180
- latitude: string; - longitude: string; + /** Latitude in decimal degrees, range: -90 to 90 */ + latitude: number; + /** Longitude in decimal degrees, range: -180 to 180 */ + longitude: number;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
public/locale/en.json(1 hunks)public/locale/hi.json(1 hunks)public/locale/kn.json(1 hunks)public/locale/ml.json(1 hunks)public/locale/mr.json(1 hunks)public/locale/ta.json(1 hunks)src/components/Facility/FacilityHome.tsx(3 hunks)src/types/facility/facility.ts(1 hunks)
🔇 Additional comments (8)
src/components/Facility/FacilityHome.tsx (2)
287-297: LGTM! Well-implemented Google Maps link.The implementation:
- Correctly checks for both latitude and longitude before rendering
- Uses proper security attributes (target="_blank" with rel="noreferrer")
- Follows accessibility best practices with proper text and icon
281-281: LGTM! Margin adjustment.The margin adjustment from mt-2 to mt-1 improves the visual spacing.
public/locale/mr.json (1)
42-42: LGTM! Correct Marathi translation.The translation "Google नकाशे" is appropriate and consistent with Marathi language conventions.
public/locale/hi.json (1)
395-395: LGTM! Correct Hindi translation.The translation "Google मैप्स" is appropriate and consistent with Hindi language conventions.
public/locale/kn.json (1)
397-397: Translation looks good!The Kannada translation "Google ನಕ್ಷೆಗಳು" is accurate and follows proper localization practices.
public/locale/ta.json (1)
396-396: Translation looks good!The Tamil translation "Google வரைபடங்கள்" is accurate and follows proper localization practices.
public/locale/ml.json (1)
998-998: LGTM! Translation looks good.The Malayalam translation for "Google Maps" is accurate and consistent with the translations added in other locale files.
public/locale/en.json (1)
1081-1081: LGTM! The translation key follows the established conventions.The addition of the "google_maps" translation key with value "Google Maps" is consistent with the existing naming patterns and properly handles the capitalization of the proper noun.
|
@rahulharpal1603 Do not add translations for all languages except en.json |
Thanks, I will fix it |
…le log in FacilityHome component
rithviknishad
added
needs testing
needs review
and removed
question
|
LGTM |
I thought from the perspective of the district admin, where he/she can see details about various facilities. In the facilities page for the admin, many facilities were listed instead of only one. |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Demo Video:
2025-01-16.02-17-42.mp4
Merge Checklist
Summary by CodeRabbit
New Features
Documentation