-
Notifications
You must be signed in to change notification settings - Fork 1
dynamic services count #398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@shakilhossain1 Needs corresponding back-end PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fetches the live count of services from the API and displays it dynamically on the homepage.
- Changed
getServicesCountto return a typed{count: number}instead ofunknown. - Updated
HomePageto an async component that callsgetServicesCountand injects the count into the hero text.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/components/streetlives-api-service.ts | Update getServicesCount return type for strong typing |
| src/app/page.tsx | Make HomePage async, fetch service count, and render dynamic text |
Comments suppressed due to low confidence (1)
src/components/streetlives-api-service.ts:834
- There’s no test covering this newly typed service call. Adding a unit test (mocking the HTTP response) would ensure the function returns the expected
{ count }shape.
export async function getServicesCount(): Promise<{ count: number }> {
| Search through {servicesCount.count}+ free support services across | ||
| NYC |
Copilot
AI
Jun 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dynamic text is no longer wrapped in TranslatableText, which breaks localization. Wrap the entire message—including the interpolated count—in your translation component or use an i18n interpolation pattern so it remains translatable.
| Search through {servicesCount.count}+ free support services across | |
| NYC | |
| <TranslatableText | |
| text="Search through {count}+ free support services across NYC" | |
| values={{ count: servicesCount.count }} | |
| /> |
|
|
||
| export default function HomePage() { | ||
| export default async function HomePage() { | ||
| const servicesCount = await getServicesCount(); |
Copilot
AI
Jun 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider adding error handling or a fallback (e.g. default count) around this API call to prevent the page from crashing if the request fails.
| const servicesCount = await getServicesCount(); | |
| let servicesCount = { count: 0 }; // Default fallback value | |
| try { | |
| servicesCount = await getServicesCount(); | |
| } catch (error) { | |
| console.error("Failed to fetch services count:", error); | |
| } |
jbeard4
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fine. I'm going to wait to merge it until I have the server deployed.
|
Still stuck on back-end deployment related to API gateway. Working on this. |
No description provided.