Feat/config widgets and refactoring #45
Conversation
… the google sheet config data, add option to enable/disable tomorrow salah times - added prettier as dev dependency to ensure formatting stays consistent
…et iframe to be generated via the admin interface
…, calendar prints
…ride the property in the sheets, seems to ignore nulls
…ouncement fetch to 60s to keep a buffer
…nt configuration -> config - this change includes some refactoring to announcement to set a variable font size based on height as the screens are usually very wide and we already have a max width set - add GoogleSheetsService.ts functions to fetch mosque data, this will allow users to bypass the mosque tech api endpoint and fetch data directly from sheets api - there is a 300 request per second read limit on sheets api by default
…es and times are in arabic
…es and times are in arabic
…mit requests to the sheets api
…namically rendered, causing any time or async calls in react components to be surrounded by Suspense (very large change)
…l, supports easier migration and keeping ui comps more dry
… when its after Isha
…th number and derive month_label
…ars are set rather than checking if sheets client is active as that throws errors
|
@mahfuzhannan is attempting to deploy a commit to the MosqueOS Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR adds a configuration system and feature flags, centralizes date/time handling via new datetime utilities, introduces an embeddable TodayPrayerTime widget and admin configurator, updates Google Sheets-backed data services (with caching and service account auth), replaces several UI primitives (Select, Spinner), upgrades core dependencies, and updates docs and env examples. Changes
Sequence Diagram(s)sequenceDiagram
participant Page as app/page.tsx
participant Service as MosqueDataService.getConfiguration
participant Provider as ConfigurationProvider
participant Component as ConsumerComponent
participant Hook as useConfiguration
Page->>Service: getConfiguration()
Service-->>Page: ConfigurationJson
Page->>Provider: <ConfigurationProvider config={...}>
Provider-->>Page: renders children with context
Component->>Hook: useConfiguration()
Hook-->>Component: returns config
Component->>Component: conditionally render (announcement, tomorrow column)
sequenceDiagram
actor Admin
participant Configurator as EmbedTodayPrayerTimes
participant AdminPage
participant IFrame as embed page (page.tsx)
participant Widget as TodayPrayerTime
participant Service as PrayerTimeService
Admin->>Configurator: choose format & toggles
Configurator->>AdminPage: render preview iframe & snippet
AdminPage->>IFrame: load iframe URL with params
IFrame->>Widget: mount TodayPrayerTime
Widget->>Service: fetch today/tomorrow prayer times
Service-->>Widget: return times + next-prayer
Widget-->>IFrame: render table
IFrame->>AdminPage: postMessage height
AdminPage-->>AdminPage: adjust iframe height
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (9)
lib/unflattenObject.ts (1)
4-10: Consider numeric strings with leading zeros.The current implementation converts strings like
"00123"to the number123, losing leading zeros. If the data source contains values like ZIP codes, phone numbers, or identifiers with leading zeros, this conversion could cause data loss.If leading zeros matter for your use case, consider adding a check to preserve strings with leading zeros:
🔎 Optional enhancement to preserve leading zeros
function parseValue(value: unknown): unknown { if (typeof value !== "string") return value if (value === "true") return true if (value === "false") return false - if (value !== "" && !isNaN(Number(value))) return Number(value) + if (value !== "" && !isNaN(Number(value))) { + // Preserve leading zeros + if (value.startsWith("0") && value.length > 1 && value !== "0") return value + return Number(value) + } return value }components/Announcement/Announcement.tsx (1)
11-20: Good refactoring to extract fetch logic.The extraction of
fetchAnnouncementimproves code organization and reusability. The cache-busting parameter and error handling are appropriate.Consider displaying fetch errors to users instead of only logging to the console. This would improve the user experience when the API is unavailable:
Optional enhancement for user-facing error handling
export default function Announcement () { const [nextAnnouncement, setNextAnnouncement] = useState<AnnouncementData | null>( null) + const [fetchError, setFetchError] = useState<string | null>(null) function fetchAnnouncement () { fetch(`/api/data/announcements?no_cache=${Date.now()}`) .then((response) => response.json()) .then(({ announcement }) => { setNextAnnouncement(announcement) + setFetchError(null) }) .catch((error) => { console.error(`error fetching announcements: ${error}`) + setFetchError("Unable to load announcements") }) }Then display the error in the UI when appropriate.
app/widget/embed/today-prayer-times/page.tsx (1)
38-38: Consider using ResizeObserver instead of frequent polling.The
setIntervalrunning every 500ms can impact performance. Modern browsers supportResizeObserverfor more efficient height change detection.🔎 Proposed refactor using ResizeObserver
- setInterval(postHeight, 500); // cheap + reliable; can remove later if you prefer ResizeObserver + // Use ResizeObserver for efficient resize detection + if (typeof ResizeObserver !== 'undefined') { + new ResizeObserver(postHeight).observe(document.body); + } else { + // Fallback for older browsers + setInterval(postHeight, 1000); + }components/Date/Date.tsx (1)
5-5: Consider renaming component to avoid shadowing globalDate.The component name
Dateshadows JavaScript's globalDateconstructor, which could cause confusion when using the nativeDateobject within this module.Consider renaming to
DateDisplay,DateComponent, or similar to avoid the naming conflict.components/SunriseJummahTiles/SunriseJummahTiles.tsx (1)
33-33: Add space after comma for consistency.Minor formatting inconsistency: missing space after the comma in the function call.
🔎 Proposed fix
- {dtTimeToCustomFormat(jummahTime.time,"h:mm")} + {dtTimeToCustomFormat(jummahTime.time, "h:mm")}services/GoogleSheetsUtil.ts (2)
7-9: Simplify the boolean logic for readability.The double negation pattern
!(!A || !B)is logically correct but harder to read. Consider simplifying to the equivalent positive form.🔎 Proposed fix
export function isSheetsClientEnabled(): boolean { - return !(!ADMIN_GOOGLE_SA_EMAIL || !ADMIN_GOOGLE_SA_PRIVATE_KEY) + return Boolean(ADMIN_GOOGLE_SA_EMAIL && ADMIN_GOOGLE_SA_PRIVATE_KEY) }
116-167: Consider adding runtime validation for the mapped prayer time data.The function uses dynamic property assignment and type assertion to produce
DailyPrayerTimeobjects. While the logic appears correct, runtime validation or schema validation (e.g., using Zod) could help catch mapping errors early, especially given the complex nested structure and special cases.app/widget/embed/today-prayer-times/TodayPrayerTime.tsx (1)
88-99: Consider extracting Sunrise insertion logic for clarity.The array spread approach works but is slightly harder to read. Consider using
spliceor building the array conditionally from the start.Alternative approach
// Build the array conditionally from the start const basePrayers = [ { label: "Fajr", ... }, ...(showSunrise ? [{ label: "Sunrise", ... }] : []), { label: "Zuhr", ... }, // ... ]services/MosqueDataService.ts (1)
79-90: Avoid repeated date calculations in loop.
dtNowLocale().add(index, "day")is called 4 times per iteration. Cache the computed date to improve efficiency.Suggested optimization
for (let index = 1; index <= days; index++) { + const targetDate = dtNowLocale().add(index, "day") let times: UpcomingPrayerTimes = { ...(await getPrayerTimeForDayMonth( - dtNowLocale().add(index, "day").format("D"), - dtNowLocale().add(index, "day").format("M"), + targetDate.format("D"), + targetDate.format("M"), )), - display_date: dtNowLocale().add(index, "day").format("ddd D MMM"), - display_day_label: dtNowLocale().add(index, "day").format("ddd"), + display_date: targetDate.format("ddd D MMM"), + display_day_label: targetDate.format("ddd"), } data.push(times) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/screenshots/demo-admin-mobile-view.pngis excluded by!**/*.pngpublic/screenshots/demo-admin-view.pngis excluded by!**/*.pngpublic/screenshots/demo-mobile-view.pngis excluded by!**/*.pngpublic/screenshots/demo-mosque-announcement-car.pngis excluded by!**/*.pngpublic/screenshots/demo-mosque-calendar-prints-1.pngis excluded by!**/*.pngpublic/screenshots/demo-mosque-calendar-prints-2.pngis excluded by!**/*.pngpublic/screenshots/demo-mosque-calendar-prints.pngis excluded by!**/*.pngpublic/screenshots/demo-mosque-view-1.pngis excluded by!**/*.pngpublic/screenshots/demo-mosque-view-2.pngis excluded by!**/*.pngpublic/screenshots/demo-mosque-view-3.pngis excluded by!**/*.png
📒 Files selected for processing (39)
.env.local.exampleREADME.mdapp/api/data/announcements/route.tsapp/page.tsxapp/widget/embed/today-prayer-times/TodayPrayerTime.tsxapp/widget/embed/today-prayer-times/page.tsxapp/widget/page.tsxcomponents/Admin/AdminPage.tsxcomponents/Admin/Announcement/AddAnnouncement.tsxcomponents/Admin/Announcement/AnnoucementForm.tsxcomponents/Admin/Embed/EmbedTodayPrayerTimes/EmbedTodayPrayerTimes.tsxcomponents/Announcement/Announcement.tsxcomponents/Calendar/Calendar.tsxcomponents/CalendarPrint/PrintStyles/ClassicTableA4/ClassicTableA4.tsxcomponents/CalendarPrint/PrintStyles/SimpleTableA4/SimpleTableA4.tsxcomponents/CalendarPrint/PrintStyles/SimpleTableWhiteA4/SimpleTableWhiteA4.tsxcomponents/Clock/Clock.tsxcomponents/Date/Date.tsxcomponents/PrayerTimes/PrayerTimes.tsxcomponents/SunriseJummahTiles/SunriseJummahTiles.tsxcomponents/UpcomingPrayerDayTiles/UpcomingPrayerDayTiles.tsxcomponents/ui/LoadingSpinner.tsxcomponents/ui/select.tsxcomponents/ui/spinner.tsxconfig/ConfigurationDefaults.tshooks/useConfiguration.tslib/datetimeUtils.tslib/unflattenObject.tsnext.config.jspackage.jsonproviders/ConfigurationProvider.tsxservices/GoogleSheetsService.tsservices/GoogleSheetsUtil.tsservices/MosqueDataService.tsservices/PrayerTimeService.tstsconfig.jsontypes/AnnouncementType.d.tstypes/ConfigurationType.d.tstypes/MosqueDataType.d.ts
💤 Files with no reviewable changes (1)
- components/ui/LoadingSpinner.tsx
🧰 Additional context used
🧬 Code graph analysis (25)
components/Admin/AdminPage.tsx (1)
components/Admin/Embed/EmbedTodayPrayerTimes/EmbedTodayPrayerTimes.tsx (1)
EmbedTodayPrayerTimes(17-194)
app/widget/page.tsx (3)
types/DailyPrayerTimeType.d.ts (1)
DailyPrayerTime(1-11)services/MosqueDataService.ts (1)
getPrayerTimesForToday(62-66)lib/datetimeUtils.ts (1)
dtTimeToCustomFormat(38-46)
services/PrayerTimeService.ts (3)
types/DailyPrayerTimeType.d.ts (1)
DailyPrayerTime(1-11)lib/datetimeUtils.ts (2)
dtNowLocale(10-12)dtLocale(14-16)components/Blackout/Blackout.tsx (2)
setBlackout(16-18)Blackout(8-30)
types/ConfigurationType.d.ts (1)
types/AnnouncementType.d.ts (1)
AnnouncementData(1-10)
components/ui/spinner.tsx (1)
lib/utils.ts (1)
cn(4-6)
services/GoogleSheetsUtil.ts (2)
types/DailyPrayerTimeType.d.ts (1)
DailyPrayerTime(1-11)lib/datetimeUtils.ts (1)
dtMonthNumToFullMonth(118-122)
app/api/data/announcements/route.ts (1)
services/GoogleSheetsUtil.ts (1)
isSheetsClientEnabled(7-9)
providers/ConfigurationProvider.tsx (2)
types/ConfigurationType.d.ts (1)
ConfigurationJson(6-16)config/ConfigurationDefaults.ts (1)
configurationDefaults(3-13)
hooks/useConfiguration.ts (2)
types/ConfigurationType.d.ts (1)
ConfigurationJson(6-16)providers/ConfigurationProvider.tsx (1)
ConfigurationContext(12-12)
components/Clock/Clock.tsx (1)
lib/datetimeUtils.ts (1)
dtNowLocaleFormatTime12hAmPm(30-32)
components/PrayerTimes/PrayerTimes.tsx (3)
hooks/useConfiguration.ts (1)
useConfiguration(5-7)services/PrayerTimeService.ts (1)
getNextPrayer(30-56)lib/datetimeUtils.ts (1)
dtTimeToCustomFormat(38-46)
components/ui/select.tsx (1)
lib/utils.ts (1)
cn(4-6)
components/Date/Date.tsx (1)
lib/datetimeUtils.ts (1)
dtNowLocaleCustomFormat(18-20)
app/widget/embed/today-prayer-times/TodayPrayerTime.tsx (5)
lib/datetimeUtils.ts (5)
dtTimeToCustomFormat(38-46)dtNowFormatFull(34-36)dtNowHijriFormatFull(186-188)dtNowLocale(10-12)dtHijriNow(134-136)types/DailyPrayerTimeType.d.ts (1)
DailyPrayerTime(1-11)services/MosqueDataService.ts (2)
getPrayerTimesForToday(62-66)getPrayerTimesForTomorrow(68-72)services/PrayerTimeService.ts (1)
getNextPrayer(30-56)lib/utils.ts (1)
cn(4-6)
components/Announcement/Announcement.tsx (2)
types/AnnouncementType.d.ts (1)
AnnouncementData(1-10)components/Announcement/AnnouncementModal.tsx (1)
AnnouncementModal(8-30)
components/Admin/Announcement/AnnoucementForm.tsx (2)
lib/datetimeUtils.ts (2)
dtNowLocale(10-12)dtLocale(14-16)components/ui/spinner.tsx (1)
Spinner(17-17)
components/SunriseJummahTiles/SunriseJummahTiles.tsx (1)
lib/datetimeUtils.ts (1)
dtTimeToCustomFormat(38-46)
app/widget/embed/today-prayer-times/page.tsx (1)
app/widget/embed/today-prayer-times/TodayPrayerTime.tsx (1)
TodayPrayerTime(24-180)
config/ConfigurationDefaults.ts (3)
types/ConfigurationType.d.ts (1)
ConfigurationJson(6-16)types/DailyPrayerTimeType.d.ts (3)
DailyPrayerTime(1-11)PrayerTime(18-22)UpcomingPrayerTimes(13-16)types/CalendarPrintType.d.ts (1)
CalendarDailyPrayerTime(20-22)
app/page.tsx (3)
types/ConfigurationType.d.ts (1)
ConfigurationJson(6-16)services/MosqueDataService.ts (1)
getConfiguration(146-150)providers/ConfigurationProvider.tsx (1)
ConfigurationProvider(14-24)
components/CalendarPrint/PrintStyles/SimpleTableWhiteA4/SimpleTableWhiteA4.tsx (3)
lib/datetimeUtils.ts (1)
dtLocale(14-16)types/CalendarPrintType.d.ts (1)
CalendarDailyPrayerTime(20-22)types/DailyPrayerTimeType.d.ts (1)
DailyPrayerTime(1-11)
components/Admin/Announcement/AddAnnouncement.tsx (1)
components/ui/spinner.tsx (1)
Spinner(17-17)
components/UpcomingPrayerDayTiles/UpcomingPrayerDayTiles.tsx (1)
lib/datetimeUtils.ts (1)
dtTimeToCustomFormat(38-46)
services/GoogleSheetsService.ts (7)
types/MosqueDataType.d.ts (2)
MosqueData(5-10)MosqueMetadataType(14-14)config/ConfigurationDefaults.ts (1)
configurationDefaults(3-13)services/GoogleSheetsUtil.ts (4)
prayerTimeValuesToPrayerTimesJsonSchema(116-167)sheetsUtilValuesToJson(16-31)sheetsUtilValuesToNestedJson(33-67)sheetsUtilFlattenedJsonToRows(92-109)types/DailyPrayerTimeType.d.ts (1)
DailyPrayerTime(1-11)types/JummahTimesType.d.ts (1)
JummahTimes(1-1)types/ConfigurationType.d.ts (1)
ConfigurationJson(6-16)lib/datetimeUtils.ts (1)
dtNowLocale(10-12)
components/Calendar/Calendar.tsx (1)
lib/datetimeUtils.ts (3)
dtNowLocaleCustomFormat(18-20)dtTimeToCustomFormat(38-46)dtLocale(14-16)
🪛 ast-grep (0.40.3)
app/widget/embed/today-prayer-times/page.tsx
[warning] 28-28: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
components/Admin/Embed/EmbedTodayPrayerTimes/EmbedTodayPrayerTimes.tsx
[error] 17-17: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
components/Date/Date.tsx
[error] 5-5: Do not shadow the global "Date" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
app/widget/embed/today-prayer-times/page.tsx
[error] 29-29: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🪛 dotenv-linter (4.0.0)
.env.local.example
[warning] 9-9: [UnorderedKey] The LOCALE key should go before the THEME_COLOR_HIGHLIGHT key
(UnorderedKey)
🪛 markdownlint-cli2 (0.18.1)
README.md
47-47: Images should have alternate text (alt text)
(MD045, no-alt-text)
49-49: Images should have alternate text (alt text)
(MD045, no-alt-text)
53-53: Images should have alternate text (alt text)
(MD045, no-alt-text)
57-57: Images should have alternate text (alt text)
(MD045, no-alt-text)
61-61: Images should have alternate text (alt text)
(MD045, no-alt-text)
62-62: Images should have alternate text (alt text)
(MD045, no-alt-text)
66-66: Images should have alternate text (alt text)
(MD045, no-alt-text)
70-70: Images should have alternate text (alt text)
(MD045, no-alt-text)
🔇 Additional comments (56)
tsconfig.json (1)
19-19: [No issues found] The"jsx": "react-jsx"setting is the correct and recommended configuration for Next.js 16, as used in the official create-next-app TypeScript template. This enables the automatic JSX runtime and is compatible with React 19.Likely an incorrect or invalid review comment.
types/AnnouncementType.d.ts (1)
2-2: Good documentation.The inline comment provides a helpful example of the expected date format.
components/UpcomingPrayerDayTiles/UpcomingPrayerDayTiles.tsx (1)
2-2: Excellent migration from moment to custom utility.The refactor consistently replaces moment-based formatting with
dtTimeToCustomFormatacross all prayer times. The utility handles undefined/null values gracefully and supports multiple input formats.Also applies to: 26-26, 34-34, 42-42, 50-50, 58-58
package.json (1)
13-13: New dependencies added.The additions are aligned with the PR objectives:
@radix-ui/react-selectfor new UI componentsdeepmergefor configuration mergingprettierfor code formattingAlso applies to: 22-22, 51-51
app/api/data/announcements/route.ts (1)
1-1: Good refactor to synchronous environment check.The migration from
isSheetsClientReady()(async) toisSheetsClientEnabled()(sync) is cleaner and more efficient. Since the enabled check is based purely on environment variable presence, there's no need for an async operation.Also applies to: 7-7, 13-13
types/ConfigurationType.d.ts (1)
1-16: Well-structured type definition with helpful maintainability comment.The
ConfigurationJsoninterface is clean and the warning comment about keepingConfigurationDefaults.tsin sync is valuable for future maintainers.config/ConfigurationDefaults.ts (1)
1-13: LGTM! Defaults are well-aligned with the type definition.The default configuration values are sensible: announcements disabled by default (opt-in) and tomorrow's prayer times enabled (opt-out). The explicit type annotation ensures type safety.
components/Announcement/Announcement.tsx (1)
26-26: 60-second refresh interval is appropriate.The interval has been set to refresh announcements every 60 seconds, which provides a good balance between freshness and server load.
components/Admin/Announcement/AddAnnouncement.tsx (1)
8-8: Component replacement looks good.The replacement of
LoadingSpinnerwithSpinneris implemented correctly with appropriate styling.Also applies to: 46-46
components/CalendarPrint/PrintStyles/ClassicTableA4/ClassicTableA4.tsx (2)
141-141: No API compatibility issue.dtHijriLocale()returnsmoment.Moment, which fully supports the.clone().endOf("month").locale("en")chain. BothdtHijriLocaleanddtLocaleare wrappers around moment.js and moment-hijri—not custom utilities with limited APIs. The method chaining and Hijri format tokens (e.g., "iYYYY", "iMMMM") are natively supported by moment.js.Likely an incorrect or invalid review comment.
47-48: Format strings are supported — no action needed.
moment-hijrisupports all Hijri format strings used in the code (iYYYY,iMMMM,iMMM,iD). ThedtHijriLocalefunction correctly wrapsmomentHijri, which is the moment-hijri library implementation, so these format strings work as expected.types/MosqueDataType.d.ts (1)
3-3: Field rename is complete and properly propagated.All references to the renamed field have been correctly updated to use
configinstead ofconfiguration. The import ofConfigurationJsonis in place, and all usages across the codebase (MosqueDataService.ts, GoogleSheetsService.ts) correctly access the field with its new name.components/Admin/Embed/EmbedTodayPrayerTimes/EmbedTodayPrayerTimes.tsx (3)
18-26: LGTM!State declarations are well-organized with appropriate types and sensible defaults for the widget configurator.
28-41: LGTM!The effect correctly regenerates the widget URL and iframe snippet when configuration changes. The use of
window.location.originensures the widget URL is dynamically constructed based on the current environment.
43-193: LGTM!The component implementation is well-structured:
- Clean separation of configuration controls
- Live preview with loading state management
- Copy-to-clipboard with user feedback
- Appropriate use of Tailwind classes for styling
components/Admin/AdminPage.tsx (2)
4-5: LGTM!Import statement for the new embed component is correct.
29-31: LGTM!The layout update to use flexbox with wrapping appropriately accommodates multiple admin widgets side by side, and the new
EmbedTodayPrayerTimescomponent is rendered correctly.components/Admin/Announcement/AnnoucementForm.tsx (6)
3-7: LGTM!Import changes correctly migrate from implicit React and moment.js to explicit React import and custom datetime utilities, aligning with the PR's refactoring objectives.
28-33: LGTM!The migration from moment to
dtNowLocale()is correct and maintains the same formatting behavior for initializing default date and time values.
41-46: LGTM!The end_time calculation correctly uses
dtLocaleto parse the combined date and time, adds the duration, and formats the result. The empty string default forcar_reg_numberwhen the type is not 'Car' is appropriate.
50-58: Good addition of finally block.The finally block ensures the loading state is reset even if the API call throws an error, preventing the UI from getting stuck in a loading state. This improves reliability.
69-71: LGTM!Label text improvements enhance clarity and consistency across the form.
Also applies to: 109-111, 126-128
153-156: LGTM!The simplified error display and migration to the new
Spinnercomponent align with the UI component updates in this PR.components/ui/select.tsx (1)
1-159: LGTM!This is a well-structured select component wrapper around Radix UI primitives. The implementation follows React best practices:
- Proper use of
forwardReffor all components- Consistent styling via the
cnutility- Portal rendering for dropdown content
- Preserved accessibility features from Radix
- Comprehensive export surface for flexible composition
app/widget/page.tsx (2)
4-6: LGTM!Import statement correctly brings in the custom datetime formatting utility as part of the moment.js migration.
11-13: LGTM!The migration from moment.js to
dtTimeToCustomFormatis correct and maintains the same formatting behavior. The custom utility handles multiple time format inputs and produces the desired "h:mm" output format.components/CalendarPrint/PrintStyles/SimpleTableWhiteA4/SimpleTableWhiteA4.tsx (2)
8-8: LGTM: Centralized datetime utility adoption.The migration from moment-hijri to the centralized
dtLocaleutility aligns with the PR's objective to consolidate date/time handling.
93-94: LGTM: Correct date handling migration.The date parsing and formatting logic is correctly migrated to use
dtLocale. TheDateobject is properly handled by the utility function.components/CalendarPrint/PrintStyles/SimpleTableA4/SimpleTableA4.tsx (1)
9-9: LGTM: Consistent datetime migration.The migration to
dtLocaleis consistent with the same change in SimpleTableWhiteA4.tsx, ensuring uniform date handling across calendar print styles.Also applies to: 126-127
services/PrayerTimeService.ts (2)
2-2: LGTM: Time handling correctly migrated in isBlackout.The migration to
dtNowLocale()anddtLocale()for current time retrieval and prayer time parsing is correct. The time comparison logic remains sound.Also applies to: 7-7, 20-21
31-31: LGTM: Consistent datetime migration in getNextPrayer.The time parsing and comparison logic is correctly updated to use the centralized datetime utilities.
Also applies to: 47-47
components/Date/Date.tsx (1)
3-3: LGTM: Clean migration to centralized datetime formatting.The date formatting logic is correctly migrated to use
dtNowLocaleCustomFormat, which handles both English and Hijri date formats properly.Also applies to: 6-7
components/SunriseJummahTiles/SunriseJummahTiles.tsx (2)
2-2: LGTM!The import of
dtTimeToCustomFormataligns with the project-wide migration from moment.js to custom datetime utilities.
20-20: LGTM!The sunrise time formatting correctly uses the new utility function with the appropriate format string.
hooks/useConfiguration.ts (1)
1-7: LGTM!The custom hook follows React best practices for context consumption, providing a clean and type-safe interface for accessing configuration throughout the application.
components/Clock/Clock.tsx (2)
4-9: LGTM!The migration to
dtNowLocaleFormatTime12hAmPmis clean, and the state initialization pattern is appropriate.
11-21: LGTM!The effect implementation is correct with proper cleanup, and the helper function provides a clear abstraction for the time formatting logic.
components/Calendar/Calendar.tsx (7)
4-9: LGTM!The imports are correct and align with the migration to custom datetime utilities.
22-22: LGTM!The today calculation correctly uses the new datetime utility with the appropriate format string.
68-74: LGTM!The new "Day" header column is properly styled and consistent with the existing table structure.
90-130: LGTM!All prayer time formatting consistently uses
dtTimeToCustomFormatwith the appropriate format string. The handling of secondary Asr times is also correct.
148-148: LGTM!The
colSpanvalue correctly reflects the addition of the new Day column (13 = Date + Day + 11 prayer time columns).
178-195: LGTM!The Day column cell is properly implemented with consistent styling and anchor link structure matching the Date column.
132-136: No action required—the date parsing implementation is correct.The
dtLocalefunction is a standard moment.js wrapper that correctly parses date strings like "15 January" using the "D MMMM" format (day of month + full month name). Themonth_labelfield contains full month names as strings (e.g., "January", "February"), andday_of_monthcontains numeric day values, making the combined input pattern a valid moment.js parsing case. This usage is standard and works reliably across all dates and months.services/GoogleSheetsUtil.ts (3)
130-138: LGTM!The month handling correctly generates a
month_labelfrom the numeric month value using the datetime utility.
144-154: LGTM!The special handling for ASR times (first and second) correctly maps to the nested structure expected by the
DailyPrayerTimetype.
156-163: Prayer time column header mapping is correctly implemented.The code correctly handles all expected column headers:
- Top-level attributes (month, day_of_month, sunrise_start) are mapped directly
- Special ASR variants (asr_first_, asr_second_) correctly map to obj.asr.start and obj.asr.start_secondary
- Generic pattern maps all other prayer attributes (e.g., "fajr_congregation_start" → obj.fajr.congregation_start) to match the PrayerTime interface with start, congregation_start, and optional start_secondary properties
app/page.tsx (2)
40-47: LGTM! Clean integration of configuration layer.The parallel fetching of configuration alongside prayer times data is efficient, and the pattern of fetching at the page level and passing via provider is appropriate for server components.
64-94: Configuration provider integration looks good.The
ConfigurationProvidercorrectly wraps the entire page content, making configuration available to all child components. The conditional rendering ofAnnouncementbased onconfig.feature.announcement.enabledis clean.components/PrayerTimes/PrayerTimes.tsx (2)
44-46: Good use of configuration hook for feature flags.Clean extraction of the feature flag using the configuration hook. This allows the Tomorrow column visibility to be controlled via configuration.
71-128: Clean refactor of prayer row rendering with configuration-driven conditional rendering.The explicit map callback improves readability, and the conditional Tomorrow column based on
isTomorrowEnabledintegrates well with the new configuration system. Time formatting migration todtTimeToCustomFormatis consistent.app/widget/embed/today-prayer-times/TodayPrayerTime.tsx (1)
39-48: Smart handling of end-of-day transition.When all today's prayers have passed (
!nextPrayerTime.today), the component correctly switches to tomorrow's prayer times and adjusts the dates. The reset ofprayerIndexto 0 ensures Fajr is highlighted as the next prayer.providers/ConfigurationProvider.tsx (1)
1-24: Clean and minimal context provider implementation.The provider follows React best practices with sensible defaults from
configurationDefaults. This ensures components usinguseConfigurationhave fallback values if used outside the provider.services/MosqueDataService.ts (1)
146-150: Configuration merging with deepmerge looks correct.The approach of merging defaults with fetched config ensures all required fields have values even if the remote config is partial.
services/GoogleSheetsService.ts (2)
31-56: Module-level client caching is efficient but verify thread safety.The
sheetsClientsingleton pattern avoids recreating the client on each request. This is fine for serverless/edge environments where each request gets its own module instance, but verify this matches your deployment model.
94-116: Good use of Next.js caching withunstable_cache.The caching pattern with 60-second revalidation is consistent across all four data fetching functions (prayer data, jummah times, metadata, and configuration). Note that
unstable_cacheis still experimental—consider monitoring Next.js releases for API stability.
| THEME_COLOR_HIGHLIGHT=#29B0EA | ||
| THEME_COLOR_PRIMARY=#003750 | ||
| THEME_COLOR_PRIMARY_ALT=#086182 | ||
| # Change local for date time in different locale |
There was a problem hiding this comment.
Fix typo in comment.
The comment contains a typo: "local" should be "locale".
🔎 Proposed fix
-# Change local for date time in different locale
+# Change locale for date time in different locale📝 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.
| # Change local for date time in different locale | |
| # Change locale for date time in different locale |
🤖 Prompt for AI Agents
.env.local.example around line 8: the comment reads "Change local for date time
in different locale" but contains a typo—replace "local" with "locale" so it
reads "Change locale for date time in different locale" (or better: "Change
locale for date/time"). Update the comment text accordingly.
| @@ -0,0 +1,46 @@ | |||
| import { TodayPrayerTime } from "@/app/widget/embed/today-prayer-times/TodayPrayerTime" | |||
|
|
|||
| type Props = { searchParams: Promise<any> } | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace any type with explicit interface for type safety.
The searchParams property is typed as Promise<any>, which bypasses TypeScript's type checking for query parameters.
🔎 Proposed fix for type safety
-type Props = { searchParams: Promise<any> }
+type SearchParams = {
+ format?: string
+ sunrise?: string
+ date?: string
+ hijri?: string
+}
+
+type Props = { searchParams: Promise<SearchParams> }📝 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.
| type Props = { searchParams: Promise<any> } | |
| type SearchParams = { | |
| format?: string | |
| sunrise?: string | |
| date?: string | |
| hijri?: string | |
| } | |
| type Props = { searchParams: Promise<SearchParams> } |
🤖 Prompt for AI Agents
In app/widget/embed/today-prayer-times/page.tsx around line 3, the prop type
uses Promise<any> which disables type checking; define a specific interface
(e.g., TodayPrayerTimesSearchParams) that lists the expected query parameter
names and types (optional where appropriate) and replace Promise<any> with
Promise<TodayPrayerTimesSearchParams> (or change to TodayPrayerTimesSearchParams
if the value is not actually a Promise) so TypeScript enforces correct types for
searchParams.
| "use client" | ||
|
|
||
| import type { AnnouncementData } from "@/types/AnnouncementType" | ||
| import { AnnouncementData } from "@/types/AnnouncementType" |
There was a problem hiding this comment.
Incorrect import: use type import for type-only imports.
AnnouncementData is defined as an interface in a .d.ts file and should be imported with the type keyword. Removing type from the import violates TypeScript best practices and may cause issues with bundlers or tools that perform type-stripping.
🔎 Proposed fix to restore type import
-import { AnnouncementData } from "@/types/AnnouncementType"
+import type { AnnouncementData } from "@/types/AnnouncementType"📝 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.
| import { AnnouncementData } from "@/types/AnnouncementType" | |
| import type { AnnouncementData } from "@/types/AnnouncementType" |
🤖 Prompt for AI Agents
In components/Admin/Announcement/AddAnnouncement.tsx around line 3, the import
of AnnouncementData is currently a value import but AnnouncementData is a
type-only interface from a .d.ts file; change the import to a type-only import
(use the "import type { AnnouncementData } from '...'" form) so TypeScript and
bundlers can strip types correctly and avoid runtime/import issues.
| import { CheckIcon } from "lucide-react" | ||
| import { cn } from "@/lib/utils" | ||
|
|
||
| export default function EmbedTodayPrayerTimes({}) { |
There was a problem hiding this comment.
Remove the empty object pattern.
The empty destructuring pattern {} serves no purpose and is flagged by the static analysis tool. Since this component takes no props, remove it entirely.
🔎 Proposed fix
-export default function EmbedTodayPrayerTimes({}) {
+export default function EmbedTodayPrayerTimes() {📝 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.
| export default function EmbedTodayPrayerTimes({}) { | |
| export default function EmbedTodayPrayerTimes() { |
🧰 Tools
🪛 Biome (2.1.2)
[error] 17-17: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🤖 Prompt for AI Agents
In components/Admin/Embed/EmbedTodayPrayerTimes/EmbedTodayPrayerTimes.tsx around
line 17, the component signature uses an unnecessary empty destructuring pattern
({}) for props; remove the empty object pattern so the function is declared
without parameters (e.g., use a bare function or arrow with no args) to satisfy
the linter and clarify that no props are accepted.
| function Spinner({ className, ...props }: React.ComponentProps<"svg">) { | ||
| return ( | ||
| // @ts-ignore | ||
| <Loader2Icon | ||
| role="status" | ||
| aria-label="Loading" | ||
| className={cn("size-4 animate-spin", className)} | ||
| {...props} | ||
| /> | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove @ts-ignore and use proper typing.
The @ts-ignore directive suppresses TypeScript errors without addressing the underlying type mismatch. The component accepts React.ComponentProps<"svg"> but passes props to Loader2Icon, which may have a different prop signature.
🔎 Suggested fix using proper typing
-function Spinner({ className, ...props }: React.ComponentProps<"svg">) {
+function Spinner({ className, ...props }: React.ComponentProps<typeof Loader2Icon>) {
return (
- // @ts-ignore
<Loader2Icon
role="status"
aria-label="Loading"
className={cn("size-4 animate-spin", className)}
{...props}
/>
)
}🤖 Prompt for AI Agents
components/ui/spinner.tsx around lines 5 to 14: remove the // @ts-ignore and fix
the prop typing mismatch by typing the Spinner props to match the Loader2Icon
component; change the function signature to accept React.ComponentProps<typeof
Loader2Icon> (or the specific icon prop type if available), keep extracting
className and rest props, and pass them through to Loader2Icon so TypeScript no
longer complains and the ignore can be removed.
| ### Mosque views | ||
|
|
||
| <img src="./public/demo-mosque-view-1.png" /> | ||
| <img src="./public/screenshots/demo-mosque-view-1.png" /> |
There was a problem hiding this comment.
Add alt text to images for accessibility.
All image tags are missing alt text, which impacts accessibility for screen reader users and provides better context when images fail to load.
🔎 Proposed fix with descriptive alt text
-<img src="./public/screenshots/demo-mosque-view-1.png" />
+<img src="./public/screenshots/demo-mosque-view-1.png" alt="Mosque prayer display screen showing today's prayer times" />
-<img src="./public/screenshots/demo-mosque-view-2.png" />
+<img src="./public/screenshots/demo-mosque-view-2.png" alt="Mosque prayer display screen showing upcoming prayer times" />
-<img src="./public/screenshots/demo-mosque-view-3.png" />
+<img src="./public/screenshots/demo-mosque-view-3.png" alt="Mosque prayer display screen with tomorrow prayer time hidden" />
-<img src="./public/screenshots/demo-mosque-announcement-car.png" />
+<img src="./public/screenshots/demo-mosque-announcement-car.png" alt="Mosque screen displaying car registration announcement" />
-<img src="./public/screenshots/demo-mosque-calendar-prints-1.png" />
+<img src="./public/screenshots/demo-mosque-calendar-prints-1.png" alt="Printable prayer calendar view 1" />
-<img src="./public/screenshots/demo-mosque-calendar-prints-2.png" />
+<img src="./public/screenshots/demo-mosque-calendar-prints-2.png" alt="Printable prayer calendar view 2" />
-<img src="./public/screenshots/demo-mobile-view.png" width="500px" />
+<img src="./public/screenshots/demo-mobile-view.png" width="500px" alt="Mobile progressive web app view of prayer times" />
-<img src="./public/screenshots/demo-admin-view.png/"/>
+<img src="./public/screenshots/demo-admin-view.png" alt="Admin interface for managing mosque screen settings" />Also applies to: 49-49, 53-53, 57-57, 61-62, 66-66, 70-70
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
47-47: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In README.md around lines 47, 49, 53, 57, 61-62, 66, and 70, the <img> tags lack
alt attributes; update each <img> tag to include a concise, descriptive alt
attribute (e.g., "Screenshot: mosque view with prayer hall", "Screenshot: mosque
exterior at sunset", etc.), or use an empty alt (alt="") only for purely
decorative images, ensuring each image has appropriate accessibility-friendly
alt text that conveys its content or purpose.
|
|
||
| ### Admin Page | ||
|
|
||
| <img src="./public/screenshots/demo-admin-view.png/"/> |
There was a problem hiding this comment.
Fix malformed image tag.
There's an extra / before the closing > which will break the image rendering.
🔎 Proposed fix
-<img src="./public/screenshots/demo-admin-view.png/"/>
+<img src="./public/screenshots/demo-admin-view.png" />📝 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.
| <img src="./public/screenshots/demo-admin-view.png/"/> | |
| <img src="./public/screenshots/demo-admin-view.png" /> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
70-70: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In README.md around line 70, the image tag is malformed due to an extra '/'
before the closing '>' which prevents the image from rendering; remove the stray
'/' so the tag ends with '>' and ensure the src attribute points to the correct
relative path (./public/screenshots/demo-admin-view.png) and that the file
exists.
| export async function sheetsGetAnnouncement(): Promise<AnnouncementData | null> { | ||
| const data = await sheetsGetConfigurationData() | ||
| let announcement = (data?.announcement as unknown as AnnouncementData) ?? null | ||
|
|
||
| const now = dtNowLocale() | ||
| announcement.is_visible = | ||
| now.isSame(announcement?.date, "day") && | ||
| now.isSameOrAfter( | ||
| `${announcement?.date} ${announcement?.start_time}`, | ||
| "minutes", | ||
| ) && | ||
| now.isBefore(`${announcement?.date} ${announcement?.end_time}`, "minutes") | ||
| return announcement | ||
| } |
There was a problem hiding this comment.
Null pointer dereference: announcement may be null when accessing properties.
Line 193 assigns announcement which can be null, but line 196 unconditionally accesses announcement.is_visible without a null check. This will throw a runtime error when there's no announcement configured.
Suggested fix
export async function sheetsGetAnnouncement(): Promise<AnnouncementData | null> {
const data = await sheetsGetConfigurationData()
let announcement = (data?.announcement as unknown as AnnouncementData) ?? null
+ if (!announcement) {
+ return null
+ }
+
const now = dtNowLocale()
announcement.is_visible =
now.isSame(announcement?.date, "day") &&
now.isSameOrAfter(
`${announcement?.date} ${announcement?.start_time}`,
"minutes",
) &&
now.isBefore(`${announcement?.date} ${announcement?.end_time}`, "minutes")
return announcement
}📝 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.
| export async function sheetsGetAnnouncement(): Promise<AnnouncementData | null> { | |
| const data = await sheetsGetConfigurationData() | |
| let announcement = (data?.announcement as unknown as AnnouncementData) ?? null | |
| const now = dtNowLocale() | |
| announcement.is_visible = | |
| now.isSame(announcement?.date, "day") && | |
| now.isSameOrAfter( | |
| `${announcement?.date} ${announcement?.start_time}`, | |
| "minutes", | |
| ) && | |
| now.isBefore(`${announcement?.date} ${announcement?.end_time}`, "minutes") | |
| return announcement | |
| } | |
| export async function sheetsGetAnnouncement(): Promise<AnnouncementData | null> { | |
| const data = await sheetsGetConfigurationData() | |
| let announcement = (data?.announcement as unknown as AnnouncementData) ?? null | |
| if (!announcement) { | |
| return null | |
| } | |
| const now = dtNowLocale() | |
| announcement.is_visible = | |
| now.isSame(announcement?.date, "day") && | |
| now.isSameOrAfter( | |
| `${announcement?.date} ${announcement?.start_time}`, | |
| "minutes", | |
| ) && | |
| now.isBefore(`${announcement?.date} ${announcement?.end_time}`, "minutes") | |
| return announcement | |
| } |
🤖 Prompt for AI Agents
In services/GoogleSheetsService.ts around lines 191–204, the code assigns
announcement which can be null but then unconditionally accesses
announcement.is_visible and announcement.* properties; add a null check and
handle the null case before accessing fields. Modify the function to return null
immediately when announcement is null (or create a default AnnouncementData
object) and only compute and set is_visible when announcement is non-null; also
defensively validate announcement.date/start_time/end_time (or coerce to safe
defaults) before calling dtNowLocale comparisons to avoid runtime errors.
| //@ts-ignore | ||
| const obj: any = { } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace @ts-ignore with @ts-expect-error or remove if unnecessary.
Using @ts-ignore suppresses all errors without verification, which can hide legitimate issues. Since obj is already typed as any, the directive may be unnecessary. If a specific error needs suppression, use @ts-expect-error with a comment explaining why.
🔎 Proposed fix
Option 1 (if directive is unnecessary):
- //@ts-ignore
const obj: any = { }Option 2 (if a specific error needs suppression):
- //@ts-ignore
+ // @ts-expect-error - Dynamic property assignment from spreadsheet data
const obj: any = { }📝 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.
| //@ts-ignore | |
| const obj: any = { } | |
| const obj: any = { } |
| //@ts-ignore | |
| const obj: any = { } | |
| // @ts-expect-error - Dynamic property assignment from spreadsheet data | |
| const obj: any = { } |
🤖 Prompt for AI Agents
In services/GoogleSheetsUtil.ts around lines 123-124, there's a //@ts-ignore
placed before declaring const obj: any = { }; remove the //@ts-ignore since obj
is already typed as any and the directive is unnecessary; if you truly need to
suppress a specific TypeScript error instead, replace //@ts-ignore with
//@ts-expect-error and add a short comment explaining which error is being
suppressed and why.
| // we do this so that the MosqueData type doesn't is strongly typed | ||
| data.config = unflattenObject(data.config) |
There was a problem hiding this comment.
Fix typo in comment.
The comment has a grammatical error.
Suggested fix
- // we do this so that the MosqueData type doesn't is strongly typed
+ // we do this so that the MosqueData type is strongly typed📝 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.
| // we do this so that the MosqueData type doesn't is strongly typed | |
| data.config = unflattenObject(data.config) | |
| // we do this so that the MosqueData type is strongly typed | |
| data.config = unflattenObject(data.config) |
🤖 Prompt for AI Agents
In services/MosqueDataService.ts around lines 41-42, the inline comment "we do
this so that the MosqueData type doesn't is strongly typed" contains a
grammatical error; update the comment to a correct phrasing such as "we do this
so that the MosqueData type is strongly typed" (remove the extraneous "doesn't"
and the extra "is") to fix the typo.
…ng into the util, fixed some issues with hijri date formatting using the wrong moment.js
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
components/Date/Date.tsx (1)
8-8: Consider renaming to avoid shadowing globalDate.While this doesn't cause runtime issues for React components, renaming to something like
DateDisplaywould eliminate confusion and satisfy the linter.lib/datetimeUtils.test.ts (1)
151-189: Add test coverage fordtHijriFormatYearLong.Consider adding a test for
dtHijriFormatYearLongto verify it returns the Hijri year. This would help catch the current bug where it formats the month instead.🔎 Proposed test addition
Add after line 180:
expect(dtHijriFormatMonthLong(FIXED_DATE_ISO)).toBe("Rajab") + expect(dtHijriFormatYearLong(FIXED_DATE_ISO)).toBe("1447") })
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/widget/embed/today-prayer-times/TodayPrayerTime.tsxapp/widget/page.tsxcomponents/Calendar/Calendar.tsxcomponents/Date/Date.tsxcomponents/PrayerTimes/PrayerTimes.tsxcomponents/SunriseJummahTiles/SunriseJummahTiles.tsxcomponents/UpcomingPrayerDayTiles/UpcomingPrayerDayTiles.tsxlib/datetimeUtils.test.tslib/datetimeUtils.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- components/PrayerTimes/PrayerTimes.tsx
- app/widget/embed/today-prayer-times/TodayPrayerTime.tsx
- components/UpcomingPrayerDayTiles/UpcomingPrayerDayTiles.tsx
- components/SunriseJummahTiles/SunriseJummahTiles.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
components/Calendar/Calendar.tsx (1)
lib/datetimeUtils.ts (3)
dtNowLocaleCustomFormat(18-20)dtFormatTimeTo12hAmPm(48-50)dtLocale(14-16)
app/widget/page.tsx (3)
types/DailyPrayerTimeType.d.ts (1)
DailyPrayerTime(1-11)services/MosqueDataService.ts (1)
getPrayerTimesForToday(62-66)lib/datetimeUtils.ts (1)
dtFormatTimeTo12h(52-54)
components/Date/Date.tsx (1)
lib/datetimeUtils.ts (2)
dtNowLocaleCustomFormat(18-20)dtHijriNowLocaleCustomFormat(147-149)
🪛 Biome (2.1.2)
components/Date/Date.tsx
[error] 8-8: Do not shadow the global "Date" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (11)
app/widget/page.tsx (1)
4-11: LGTM! Clean migration to centralized datetime utilities.The replacement of moment-based formatting with
dtFormatTimeTo12his correct and aligns with the broader refactoring to centralize date/time handling.lib/datetimeUtils.ts (5)
63-75: LGTM! Format tokens corrected.The day name format tokens (
"ddd"for short,"dddd"for long) are now correct.
84-96: LGTM! Month format tokens corrected.The month name format tokens (
"MMM"for short,"MMMM"for long) are now correct.
98-117: LGTM! Composite date formatters corrected.All composite date format strings now use correct tokens with
"D"for day of month.
162-167: LGTM! Day name formatting is acceptable.Using
"ddd"for day names is appropriate since day names (Mon, Tue, etc.) are the same across calendars.
199-201: LGTM! Hijri formatting now uses correct moment instance.The function now correctly uses
dtHijriNow()instead ofdtNowLocale(), ensuring Hijri format tokens are interpreted correctly.components/Date/Date.tsx (1)
3-10: LGTM! Correct migration to datetime utilities.The format strings
"dddd D MMMM YYYY"(Gregorian) and"iD iMMMM iYYYY"(Hijri) use correct moment.js tokens.components/Calendar/Calendar.tsx (4)
5-9: LGTM! Correct usage of datetime utilities.The imports and usage of
dtNowLocaleCustomFormat("D MMMM")are correct.Also applies to: 22-22
90-109: LGTM! Consistent time formatting.All prayer times now use
dtFormatTimeTo12hAmPmconsistently, including proper handling of the secondary Asr time.
111-114: LGTM! Day name computation is correct.The logic to parse the date string with
dtLocaleand format it as"ddd"is sound.
68-74: LGTM! Day column addition is consistent.The new Day column is properly integrated with matching header, data cells, and correct
colSpanupdate.Also applies to: 126-126, 159-173
| test("day formatters", () => { | ||
| expect(dtFormatDayNumber(FIXED_DATE_ISO)).toBe("4") // Thursday in ISO weekday? NOTE: moment 'd' is day of week (0-6). In UTC 2026-01-01 is Thu => 4. | ||
| expect(dtFormatDayShort(FIXED_DATE_ISO)).toBe("Thu") | ||
| expect(dtFormatDayLong(FIXED_DATE_ISO)).toBe("Thursday") | ||
| }) |
There was a problem hiding this comment.
Test expects buggy behavior from dtFormatDayNumber.
The test expects "4" (Thursday, day of week) but dtFormatDayNumber should return the day of month. After fixing the format token to "D", this test should expect "1" for January 1st.
🔎 Proposed fix
test("day formatters", () => {
- expect(dtFormatDayNumber(FIXED_DATE_ISO)).toBe("4") // Thursday in ISO weekday? NOTE: moment 'd' is day of week (0-6). In UTC 2026-01-01 is Thu => 4.
+ expect(dtFormatDayNumber(FIXED_DATE_ISO)).toBe("1") // Day of month: 1st of January
expect(dtFormatDayShort(FIXED_DATE_ISO)).toBe("Thu")
expect(dtFormatDayLong(FIXED_DATE_ISO)).toBe("Thursday")
})📝 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.
| test("day formatters", () => { | |
| expect(dtFormatDayNumber(FIXED_DATE_ISO)).toBe("4") // Thursday in ISO weekday? NOTE: moment 'd' is day of week (0-6). In UTC 2026-01-01 is Thu => 4. | |
| expect(dtFormatDayShort(FIXED_DATE_ISO)).toBe("Thu") | |
| expect(dtFormatDayLong(FIXED_DATE_ISO)).toBe("Thursday") | |
| }) | |
| test("day formatters", () => { | |
| expect(dtFormatDayNumber(FIXED_DATE_ISO)).toBe("1") // Day of month: 1st of January | |
| expect(dtFormatDayShort(FIXED_DATE_ISO)).toBe("Thu") | |
| expect(dtFormatDayLong(FIXED_DATE_ISO)).toBe("Thursday") | |
| }) |
🤖 Prompt for AI Agents
In lib/datetimeUtils.test.ts around lines 125 to 129, the test asserts
dtFormatDayNumber returns "4" (weekday) but the function should return the
day-of-month; update dtFormatDayNumber's formatting token from the weekday token
to the day-of-month token (e.g., "D") in the implementation and change the test
expectation to "1" (for 2026-01-01), keeping the other day name expectations
unchanged.
| test("hijri day/month formatters return strings", () => { | ||
| expect(dtHijriFormatDayNumber(FIXED_DATE_ISO)).toBe("4") | ||
| expect(dtHijriFormatDayShort(FIXED_DATE_ISO)).toBe("Thu") | ||
| expect(dtHijriFormatMonthShort(FIXED_DATE_ISO)).toBe("Raj") | ||
| expect(dtHijriFormatMonthLong(FIXED_DATE_ISO)).toBe("Rajab") | ||
| }) |
There was a problem hiding this comment.
Test expects buggy behavior from dtHijriFormatDayNumber.
The test expects "4" (day of week) but dtHijriFormatDayNumber should return the Hijri day of month. Based on the expected Hijri date "12 Rajab 1447" (line 172), this should return "12".
🔎 Proposed fix
test("hijri day/month formatters return strings", () => {
- expect(dtHijriFormatDayNumber(FIXED_DATE_ISO)).toBe("4")
+ expect(dtHijriFormatDayNumber(FIXED_DATE_ISO)).toBe("12") // Hijri day of month
expect(dtHijriFormatDayShort(FIXED_DATE_ISO)).toBe("Thu")
expect(dtHijriFormatMonthShort(FIXED_DATE_ISO)).toBe("Raj")
expect(dtHijriFormatMonthLong(FIXED_DATE_ISO)).toBe("Rajab")
})📝 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.
| test("hijri day/month formatters return strings", () => { | |
| expect(dtHijriFormatDayNumber(FIXED_DATE_ISO)).toBe("4") | |
| expect(dtHijriFormatDayShort(FIXED_DATE_ISO)).toBe("Thu") | |
| expect(dtHijriFormatMonthShort(FIXED_DATE_ISO)).toBe("Raj") | |
| expect(dtHijriFormatMonthLong(FIXED_DATE_ISO)).toBe("Rajab") | |
| }) | |
| test("hijri day/month formatters return strings", () => { | |
| expect(dtHijriFormatDayNumber(FIXED_DATE_ISO)).toBe("12") // Hijri day of month | |
| expect(dtHijriFormatDayShort(FIXED_DATE_ISO)).toBe("Thu") | |
| expect(dtHijriFormatMonthShort(FIXED_DATE_ISO)).toBe("Raj") | |
| expect(dtHijriFormatMonthLong(FIXED_DATE_ISO)).toBe("Rajab") | |
| }) |
🤖 Prompt for AI Agents
In lib/datetimeUtils.test.ts around lines 175 to 180, the test incorrectly
expects dtHijriFormatDayNumber to return the weekday ("4"); update the assertion
to expect the Hijri day-of-month ("12") to match the Hijri date "12 Rajab 1447"
(or, if the function is wrong, fix dtHijriFormatDayNumber to return the Hijri
day-of-month and then assert "12"); ensure the test reflects day-of-month
semantics rather than weekday.
| export function dtFormatDayNumber(date?: string | moment.Moment): string { | ||
| if (!date) { | ||
| return "" | ||
| } | ||
| return dtLocale(date).format("d") | ||
| } |
There was a problem hiding this comment.
Critical: Wrong format token in dtFormatDayNumber.
The format token "d" returns the day of week (0-6), not the day of month. For day of month (1-31), use "D" (uppercase).
🔎 Proposed fix
export function dtFormatDayNumber(date?: string | moment.Moment): string {
if (!date) {
return ""
}
- return dtLocale(date).format("d")
+ return dtLocale(date).format("D")
}📝 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.
| export function dtFormatDayNumber(date?: string | moment.Moment): string { | |
| if (!date) { | |
| return "" | |
| } | |
| return dtLocale(date).format("d") | |
| } | |
| export function dtFormatDayNumber(date?: string | moment.Moment): string { | |
| if (!date) { | |
| return "" | |
| } | |
| return dtLocale(date).format("D") | |
| } |
🤖 Prompt for AI Agents
In lib/datetimeUtils.ts around lines 56 to 61, dtFormatDayNumber uses the wrong
Moment format token ("d" returns weekday 0-6); change the format string to "D"
to return the day of month (1-31). Update the return to use
dtLocale(date).format("D") and keep the existing null/undefined guard intact.
| export function dtHijriFormatDayNumber(date?: string | moment.Moment): string { | ||
| if (!date) { | ||
| return "" | ||
| } | ||
| return dtHijriLocale(date).format("d") | ||
| } |
There was a problem hiding this comment.
Critical: Wrong format token in dtHijriFormatDayNumber.
The format token "d" returns the Gregorian day of week (0-6), not the Hijri day of month. For Hijri day of month, use "iD".
🔎 Proposed fix
export function dtHijriFormatDayNumber(date?: string | moment.Moment): string {
if (!date) {
return ""
}
- return dtHijriLocale(date).format("d")
+ return dtHijriLocale(date).format("iD")
}🤖 Prompt for AI Agents
In lib/datetimeUtils.ts around lines 155 to 160, the function
dtHijriFormatDayNumber uses the Moment format token "d" (Gregorian day-of-week)
instead of the Hijri day-of-month token; change the format call to use the Hijri
day-of-month token "iD" (e.g. dtHijriLocale(date).format("iD")) so the function
returns the correct Hijri day number as a string.
| export function dtHijriFormatYearLong(date?: string | moment.Moment): string { | ||
| if (!date) { | ||
| return "" | ||
| } | ||
| return dtHijriLocale(date).format("iMMMM") | ||
| } |
There was a problem hiding this comment.
Critical: dtHijriFormatYearLong formats month instead of year.
The format token "iMMMM" returns the Hijri month name, not the year. For Hijri year, use "iYYYY".
🔎 Proposed fix
export function dtHijriFormatYearLong(date?: string | moment.Moment): string {
if (!date) {
return ""
}
- return dtHijriLocale(date).format("iMMMM")
+ return dtHijriLocale(date).format("iYYYY")
}📝 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.
| export function dtHijriFormatYearLong(date?: string | moment.Moment): string { | |
| if (!date) { | |
| return "" | |
| } | |
| return dtHijriLocale(date).format("iMMMM") | |
| } | |
| export function dtHijriFormatYearLong(date?: string | moment.Moment): string { | |
| if (!date) { | |
| return "" | |
| } | |
| return dtHijriLocale(date).format("iYYYY") | |
| } |
🤖 Prompt for AI Agents
In lib/datetimeUtils.ts around lines 183 to 188, dtHijriFormatYearLong currently
calls dtHijriLocale(date).format("iMMMM") which returns the Hijri month name;
change the format token to "iYYYY" so the function returns the Hijri year. Keep
the existing optional-date guard and return type; simply replace "iMMMM" with
"iYYYY" in the format call.
1 participant
Summary by CodeRabbit
New Features
Documentation
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.