load calendar data #100
load calendar data #100
Conversation
WalkthroughThe changes in this pull request involve significant updates across various components and configurations in the application. Key modifications include a complete overhaul of the Changes
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 59
🧹 Outside diff range and nitpick comments (23)
apps/vite/vite.config.ts (1)
1-1: Use the node: protocol for Node.js builtin modules.For better clarity and to follow modern best practices, update the path import to use the node: protocol.
-import path from "path" +import path from "node:path"🧰 Tools
🪛 Biome
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
apps/vite/src/hooks/useMediaQuery.tsx (1)
21-21: Consider using named exports instead of default exportNamed exports are generally preferred in React applications as they enable better tree-shaking and make imports more explicit.
- export default useMediaQuery + export { useMediaQuery }apps/vite/src/components/calendar/DayDialog.tsx (2)
1-17: Optimize imports for better type safetyThe React import should be marked as a type import since it's only used for type definitions.
-import * as React from "react" +import type * as React from "react"🧰 Tools
🪛 Biome
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
43-53: Optimize drawer implementationA few suggestions for the mobile drawer implementation:
- The empty DrawerFooter with padding seems unnecessary
- Use self-closing tag for empty elements
<DrawerContent> <DrawerHeader className="text-left"> <DrawerTitle>आजको विवरण</DrawerTitle> </DrawerHeader> {children} - <DrawerFooter className="pt-2"></DrawerFooter> + <DrawerFooter className="pt-2" /> </DrawerContent>🧰 Tools
🪛 Biome
[error] 50-52: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
apps/vite/src/Body.tsx (1)
18-18: Use template literals for better readabilityReplace string concatenation with template literals for cleaner code.
- <div className={(darkMode ? "dark" : "") + " flex min-h-screen flex-col"}> + <div className={`${darkMode ? "dark" : ""} flex min-h-screen flex-col`}>🧰 Tools
🪛 Biome
[error] 18-18: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
apps/vite/src/index.css (2)
41-43: Consider consolidating redundant grid row variables.The variables
--grid-auto-rows-lg,--grid-auto-rows-md, and--grid-auto-rows-small share the same value1fr. If there's no plan to have different values for different breakpoints, consider using a single variable instead.- --grid-auto-rows-lg: 1fr; - --grid-auto-rows-md: 1fr; - --grid-auto-rows-sm: 1fr; + --grid-auto-rows: 1fr;
16-39: Consider adding documentation for the color system.The color system is well-structured using HSL values, but adding comments to document the intended usage of each color variable would improve maintainability.
+ /* Main background and text colors */ --background: 0 0% 100%; --foreground: 0 0% 3.9%; + + /* Card component colors */ --card: 0 0% 100%; --card-foreground: 0 0% 3.9%;apps/vite/src/components/calendar/DayDetails.tsx (1)
1-3: Useimport typefor type-only imports.The
NewCalendarDataimport is only used as a type. Usingimport typehelps the compiler optimize the bundle size.import { cn } from "@/lib/utils" -import { NewCalendarData } from "@miti/types" +import type { NewCalendarData } from "@miti/types" import Panchang from "./Panchang"🧰 Tools
🪛 Biome
[error] 2-2: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
apps/vite/src/components/ui/drawer.tsx (1)
1-16: LGTM! Consider adding prop documentation.The base Drawer component implementation is clean and type-safe.
Consider adding JSDoc comments to document the
shouldScaleBackgroundprop:+/** + * Root Drawer component that wraps Vaul's DrawerPrimitive.Root + * @param shouldScaleBackground - When true, scales the background content when drawer opens + */ const Drawer = ({ shouldScaleBackground = true, ...props }: React.ComponentProps<typeof DrawerPrimitive.Root>)apps/vite/src/components/YearMonthPicker.tsx (1)
40-42: Optimize disabled state styling classesThe disabled state styling contains redundant classes that can be simplified.
- "mx-3 flex flex-none items-center justify-center rounded-lg bg-indigo-600 p-1.5 text-white hover:bg-indigo-700 disabled:cursor-not-allowed disabled:bg-blue-600 disabled:text-white disabled:opacity-20 disabled:hover:cursor-not-allowed disabled:hover:bg-blue-600 disabled:hover:text-white" + "mx-3 flex flex-none items-center justify-center rounded-lg bg-indigo-600 p-1.5 text-white hover:bg-indigo-700 disabled:cursor-not-allowed disabled:bg-blue-600 disabled:text-white disabled:opacity-20"apps/vite/src/components/UserSettings.tsx (1)
Line range hint
39-44: Consider adding type safety for status values.The class name handling looks good, but consider defining an enum or union type for the status prop to prevent potential typos and improve maintainability.
type UserStatus = 'LOGGED_IN' | 'NOT_LOGGED_IN' | 'OFFLINE'; interface UserSettingsProps { photoUrl?: string | null; status: UserStatus; }🧰 Tools
🪛 Biome
[error] 36-36: Avoid the words "image", "picture", or "photo" in img element alt text.
Screen readers announce img elements as "images", so it is not necessary to redeclare this in alternative text.
(lint/a11y/noRedundantAlt)
[error] 41-41: Use === instead of ==. == is only allowed when comparing against
null== is only allowed when comparing against null
Using == may be unsafe if you are relying on type coercion
Unsafe fix: Use ===(lint/suspicious/noDoubleEquals)
[error] 42-42: Use === instead of ==. == is only allowed when comparing against
null== is only allowed when comparing against null
Using == may be unsafe if you are relying on type coercion
Unsafe fix: Use ===(lint/suspicious/noDoubleEquals)
apps/vite/src/components/Navbar.tsx (1)
Line range hint
115-120: Consider cleaning up or restoring commented code.There's commented out code for the InstallPWA component in the mobile menu. Should this be removed or restored?
If this feature is planned for future implementation, consider adding a TODO comment with the ticket reference, or remove if no longer needed.
apps/vite/src/helper/dates.ts (2)
Line range hint
29-52: Consider enhancing type safety and validation.The implementation looks good and correctly mirrors the Nepali version. Consider these improvements:
- Add input validation to handle negative indices
- Define an enum or constant for the valid Tithi indices to improve maintainability
+// Define valid Tithi indices +const VALID_TITHI_INDICES = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 30] as const; +type TithiIndex = typeof VALID_TITHI_INDICES[number]; + export function getTithiEnglish(index: number): string { + if (index < 0) return ""; + const tithiName: { - [key: number]: string + [key in TithiIndex]: string } = { // ... rest of the mapping }
Line range hint
88-117: Refactor to improve robustness and reduce duplication.The implementation correctly mirrors the Nepali version but has some areas for improvement:
- Missing bounds checking could cause undefined access
- Duplicated "Adhik" logic could be extracted
- Repeated
Math.floor(index - 1)calculationConsider this safer implementation:
+const isAdhikMonth = (index: number): boolean => Math.floor(index - 1) < index - 1; + export function getChandramaEnglish(index: number): string { const chandraNames = [ "Kartik Shuklapaksha", // ... rest of the names ] + const adjustedIndex = Math.floor(index - 1); + + if (adjustedIndex < 0 || adjustedIndex >= chandraNames.length) { + return ""; + } + return ( - (Math.floor(index - 1) < index - 1 ? "Adhik " : "") + - chandraNames[Math.floor(index - 1)] + (isAdhikMonth(index) ? "Adhik " : "") + + chandraNames[adjustedIndex] ) }This refactoring:
- Adds bounds checking to prevent undefined access
- Extracts the "Adhik" logic to a reusable function
- Reduces repeated calculations
- Returns empty string for invalid indices
Consider applying similar changes to
getChandramaNepalifor consistency.apps/vite/src/components/MonthCalendar.tsx (3)
Line range hint
104-112: Consider extracting complex className logic.The className conditions are becoming complex and harder to maintain. Consider extracting this logic into a separate function for better readability.
const getDayClassName = (isSelectedDay: boolean, isToday: boolean, day: DayData) => { return cn( 'p-1 font-mukta leading-3 hover:bg-gray-100 focus:z-10', (isSelectedDay || isToday) && 'font-semibold', isToday && 'bg-indigo-200 font-semibold text-indigo-600', !isSelectedDay && 'bg-white dark:bg-gray-900', isSelectedDay && 'bg-indigo-600 text-white hover:bg-indigo-700', (day.events.find((event) => event.jds?.gh == "1") || day.week_day === 6) && 'text-rose-600' ) }
Line range hint
123-134: Consider improving event color handling.The current approach of using inline styles for event colors could be improved by using CSS variables or a more maintainable styling solution.
Consider:
- Using CSS variables for event colors
- Creating a separate component for event indicators
- Moving colors to a theme configuration
Example approach:
// EventIndicator.tsx interface EventIndicatorProps { colorId: string; } const EventIndicator = ({ colorId }: EventIndicatorProps) => { const colorVar = `var(--event-color-${colorId}, #475569)`; return ( <span className="mx-[1px] inline-block h-1 w-1 rounded-full" style={{ backgroundColor: colorVar }} /> ); };🧰 Tools
🪛 Biome
[error] 133-133: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
Line range hint
41-205: Consider breaking down the component for better maintainability.The MonthCalendar component has grown to handle multiple responsibilities:
- Calendar grid rendering
- Event filtering and display
- Date manipulation
- Localization
- UI state management
This could make it harder to maintain and test.
Consider splitting this into smaller, focused components:
- CalendarGrid - Handle the grid layout and day rendering
- EventList - Handle event filtering and display
- DayDetails - Handle the selected day information display
- Use a custom hook for date manipulation logic
This would improve:
- Code organization
- Testing capabilities
- Reusability
- Performance (through better memoization opportunities)
🧰 Tools
🪛 Biome
[error] 133-133: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
apps/vite/src/components/calendar/Panchang.tsx (4)
66-66: Ensure safe access with optional chainingIn
value={data.panchangaDetails?.chandraRashi.time.np ?? "-"}, ifchandraRashiortimecan be undefined, consider adding optional chaining to prevent potential runtime errors.Suggested change:
- value={data.panchangaDetails?.chandraRashi.time.np ?? "-"} + value={data.panchangaDetails?.chandraRashi?.time?.np ?? "-"}
70-70: Ensure safe access with optional chainingIn
value={data.panchangaDetails?.suryaRashi.np ?? "-"}, ifsuryaRashican be undefined, consider adding optional chaining.Suggested change:
- value={data.panchangaDetails?.suryaRashi.np ?? "-"} + value={data.panchangaDetails?.suryaRashi?.np ?? "-"}
75-80: Ensure safe access of nested properties in Karan valuesWhen accessing
karans.first.npandkarans.second.np, consider adding optional chaining to handle cases wherekarans,first, orsecondmight be undefined.Suggested changes:
- value={data.panchangaDetails?.karans.first.np ?? "-"} + value={data.panchangaDetails?.karans?.first?.np ?? "-"} - value={data.panchangaDetails?.karans.second.np ?? "-"} + value={data.panchangaDetails?.karans?.second?.np ?? "-"}
83-83: Ensure safe access with optional chainingIn
value={data.hrituDetails?.title.np ?? "-"}, iftitlecan be undefined, consider adding optional chaining.Suggested change:
- value={data.hrituDetails?.title.np ?? "-"} + value={data.hrituDetails?.title?.np ?? "-"}apps/vite/src/pages/Calendar2.tsx (1)
84-86: SimplifyTodayEventListprop by removing unnecessary optional chainingSince
todayData?.eventDetailsis validated in the condition, you can passtodayData.eventDetailsdirectly without optional chaining.Apply this diff:
{todayData?.eventDetails && ( - <TodayEventList data={todayData?.eventDetails} /> + <TodayEventList data={todayData.eventDetails} /> )}apps/vite/src/components/calendar/CalendarGrid.tsx (1)
Line range hint
95-105: Use dynamic data instead of hardcoded event detailsThe event details for "Developer Meetup" are hardcoded and displayed when
index % 3 === 0. This can lead to inconsistencies and does not reflect actual event data. Consider usingday.eventDetailsto dynamically render events.Apply this refactor:
- {index % 3 === 0 && ( - <div className="absolute bottom-1 left-1 p-1.5 h-max rounded xl:bg-emerald-50 z-10 flex-row"> - <p className="hidden xl:block text-xs font-medium text-emerald-600 mb-px whitespace-nowrap"> - Developer Meetup - </p> - <p className="xl:hidden w-2 h-2 rounded-full bg-emerald-600" /> - <p className="hidden xl:block text-xs font-medium text-emerald-600 mb-px whitespace-nowrap"> - Developer Meetup - </p> - <p className="xl:hidden w-2 h-2 rounded-full bg-emerald-600" /> - </div> - )} + {day.eventDetails.map((event, eventIndex) => ( + <div + key={eventIndex} + className="absolute bottom-1 left-1 p-1.5 h-max rounded xl:bg-emerald-50 z-10 flex-row" + > + <p className="hidden xl:block text-xs font-medium text-emerald-600 mb-px whitespace-nowrap"> + {event.title.np} + </p> + <p className="xl:hidden w-2 h-2 rounded-full bg-emerald-600" /> + </div> + ))}This change dynamically displays event details based on the actual data in
day.eventDetails.🧰 Tools
🪛 Biome
[error] 71-71: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 2-2: The default import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 61-76: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (32)
- apps/api/package.json (1 hunks)
- apps/vite/components.json (1 hunks)
- apps/vite/package.json (1 hunks)
- apps/vite/src/Body.tsx (1 hunks)
- apps/vite/src/components/MonthCalendar.tsx (3 hunks)
- apps/vite/src/components/Navbar.tsx (3 hunks)
- apps/vite/src/components/Spinner.tsx (2 hunks)
- apps/vite/src/components/SplitButton.tsx (2 hunks)
- apps/vite/src/components/UserSettings.tsx (3 hunks)
- apps/vite/src/components/YearMonthPicker.tsx (3 hunks)
- apps/vite/src/components/calendar/CalendarGrid.tsx (2 hunks)
- apps/vite/src/components/calendar/CalendarHeader.tsx (2 hunks)
- apps/vite/src/components/calendar/DayDetails.tsx (1 hunks)
- apps/vite/src/components/calendar/DayDialog.tsx (1 hunks)
- apps/vite/src/components/calendar/EventList.tsx (1 hunks)
- apps/vite/src/components/calendar/Panchang.tsx (2 hunks)
- apps/vite/src/components/calendar/Today.tsx (1 hunks)
- apps/vite/src/components/calendar/TodayEventList.tsx (1 hunks)
- apps/vite/src/components/calendar/UpcomingEvent.tsx (2 hunks)
- apps/vite/src/components/ui/dialog.tsx (1 hunks)
- apps/vite/src/components/ui/drawer.tsx (1 hunks)
- apps/vite/src/helper/dates.ts (2 hunks)
- apps/vite/src/helper/utils.ts (0 hunks)
- apps/vite/src/hooks/useMediaQuery.tsx (1 hunks)
- apps/vite/src/index.css (1 hunks)
- apps/vite/src/lib/utils.ts (1 hunks)
- apps/vite/src/pages/Calendar2.tsx (1 hunks)
- apps/vite/tailwind.config.js (1 hunks)
- apps/vite/tsconfig.json (1 hunks)
- apps/vite/vite.config.ts (1 hunks)
- packages/query/src/calendar/index.ts (1 hunks)
- packages/types/src/calendar.types.ts (6 hunks)
💤 Files with no reviewable changes (1)
- apps/vite/src/helper/utils.ts
✅ Files skipped from review due to trivial changes (2)
- apps/api/package.json
- apps/vite/components.json
🧰 Additional context used
🪛 Biome
apps/vite/src/Body.tsx
[error] 18-18: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
apps/vite/src/components/MonthCalendar.tsx
[error] 133-133: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
apps/vite/src/components/Spinner.tsx
[error] 6-6: The elements with the following roles can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
apps/vite/src/components/SplitButton.tsx
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
apps/vite/src/components/calendar/CalendarGrid.tsx
[error] 71-71: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 2-2: The default import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 61-76: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
[error] 114-114: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
apps/vite/src/components/calendar/CalendarHeader.tsx
[error] 1-1: The default import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 34-41: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
[error] 45-48: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
apps/vite/src/components/calendar/DayDetails.tsx
[error] 2-2: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
apps/vite/src/components/calendar/DayDialog.tsx
[error] 50-52: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
apps/vite/src/components/calendar/EventList.tsx
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 52-52: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.(lint/suspicious/noArrayIndexKey)
[error] 64-64: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.(lint/suspicious/noArrayIndexKey)
apps/vite/src/components/calendar/Panchang.tsx
[error] 2-2: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 30-30: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
[error] 119-119: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
apps/vite/src/components/calendar/Today.tsx
[error] 105-105: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 106-109: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 111-112: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 113-113: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 114-117: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 4-4: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 78-82: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
apps/vite/src/components/calendar/TodayEventList.tsx
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 21-26: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
apps/vite/src/components/calendar/UpcomingEvent.tsx
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
apps/vite/src/pages/Calendar2.tsx
[error] 17-17: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 27-27: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.(lint/style/useNumberNamespace)
[error] 28-28: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.(lint/style/useNumberNamespace)
apps/vite/vite.config.ts
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.(lint/style/useNodejsImportProtocol)
packages/query/src/calendar/index.ts
[error] 2-2: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
🔇 Additional comments (47)
apps/vite/vite.config.ts (1)
7-11: LGTM! Path alias configuration looks good.The alias configuration is well-structured and aligns with the complementary tsconfig.json changes. This will enable cleaner imports using the "@" prefix throughout the project.
Let's verify the path alias configuration is properly set up:
✅ Verification successful
Let me generate another script to check the tsconfig.json paths configuration with a fixed regex:
Path alias configuration is correctly set up and actively used
The verification confirms that:
- The tsconfig.json correctly maps "@/" to "./src/"
- The src directory exists with the expected structure
- Multiple files are already using the @ alias imports correctly (21 imports found across different components)
The path alias configuration in both vite.config.ts and tsconfig.json are properly aligned and working as intended.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the path alias configuration # Check if tsconfig.json exists and contains the corresponding path configuration echo "Checking tsconfig.json configuration..." rg -A 2 '"paths":\s*{' apps/vite/tsconfig.json # Look for any existing imports using the new alias to ensure they'll work echo "Checking current usage of @ imports..." rg "from\s+['\"]@/" apps/vite/srcLength of output: 2144
Script:
#!/bin/bash # Check tsconfig.json paths configuration with a fixed regex echo "Checking tsconfig.json paths configuration..." cat apps/vite/tsconfig.json | grep -A 5 "paths" # Also verify the existence of the src directory echo -e "\nVerifying src directory structure..." ls -la apps/vite/srcLength of output: 1339
apps/vite/src/lib/utils.ts (1)
1-2: LGTM!The imports are appropriate and necessary for the utility functions.
apps/vite/tsconfig.json (2)
10-10: LGTM! Modern module resolution strategy.The "Bundler" moduleResolution is the recommended setting for modern bundlers like Vite when using ESM modules.
11-14: Verify path alias configuration in Vite config.The path alias configuration looks good, but ensure it's properly mirrored in your Vite configuration.
✅ Verification successful
Path alias configuration is correctly set up in both TypeScript and Vite
The path alias
@is properly configured:
- Vite config has the correct alias setup pointing to
./src- Multiple files are actively using the
@/imports- The configuration in both
tsconfig.jsonandvite.config.tsare aligned🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that Vite config has corresponding path alias setup # Test: Search for path alias configuration in vite.config rg --type ts "resolve.*alias.*@" apps/vite/vite.config.ts # Test: Verify actual usage of the alias rg --type typescript "from ['\"]\@\/" apps/vite/src/Length of output: 205
Script:
#!/bin/bash # Let's try different approaches to verify the path alias configuration # Check if vite.config.ts exists and show its content fd "vite.config.ts" apps/vite/ --exec cat {} # Look for any resolve or alias configuration in vite config rg "resolve|alias" apps/vite/vite.config.ts # Check for imports using @ pattern (fixing the escape sequence) rg "from ['\"]@/" apps/vite/src/Length of output: 2263
apps/vite/src/hooks/useMediaQuery.tsx (3)
1-2: LGTM! Clean imports and proper TypeScript usage.
14-17: LGTM! Proper cleanup implementation.The cleanup function correctly removes the event listener, preventing memory leaks.
3-19: Verify the implementation in DayDialog componentLet's verify that the media query being used in the DayDialog component matches the expected breakpoint.
✅ Verification successful
The initial search shows that the hook is being used correctly with a standard breakpoint of 768px for desktop views, but let's verify if this breakpoint is consistent with other media queries in the codebase to ensure design consistency.
Let me try a different approach to search for media query breakpoints without file type restrictions.
Let me check the tailwind configuration since it commonly defines breakpoints for responsive design.
The media query implementation in DayDialog is correctly using Tailwind's default md breakpoint
The
useMediaQueryhook in DayDialog is using(min-width: 768px)which aligns with Tailwind's defaultmdbreakpoint. Since the project is using Tailwind CSS (as evidenced by the tailwind.config.js) and the breakpoint matches Tailwind's standard responsive design system, this implementation is correct and consistent with the framework's conventions.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check how the useMediaQuery hook is being used in the DayDialog component # Search for useMediaQuery usage in DayDialog component rg -A 5 "useMediaQuery" apps/vite/src/components/calendar/DayDialog.tsxLength of output: 383
Script:
#!/bin/bash # Search for all media query breakpoints in the codebase rg -i "min-width:|max-width:" --type css --type tsx --type jsxLength of output: 92
Script:
#!/bin/bash # Search for media query breakpoints in the codebase rg -i "min-width:|max-width:" # Also check if there's a central theme or configuration file that might define breakpoints fd -e ts -e js -e json "theme|config|breakpoint"Length of output: 759
Script:
#!/bin/bash # Check tailwind config for breakpoint definitions cat apps/vite/tailwind.config.jsLength of output: 1894
apps/vite/src/components/SplitButton.tsx (1)
1-2: LGTM! Clean implementation of the utility function change.The migration from
classNamestocnutility is implemented correctly. The conditional class application and styling remain well-structured and maintainable.Also applies to: 18-20
🧰 Tools
🪛 Biome
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
apps/vite/src/components/calendar/DayDialog.tsx (2)
19-27: LGTM! Well-structured component interfaceThe props interface is well-defined with proper TypeScript types and clear naming conventions.
28-41: LGTM! Clean responsive implementationThe desktop view implementation using useMediaQuery is well-structured and follows responsive design best practices.
apps/vite/src/components/calendar/UpcomingEvent.tsx (3)
7-10: LGTM!The component props are well-typed and properly structured.
14-16: LGTM!The conditional styling using the
cnutility is well-implemented and maintains clear visual distinction between holiday and non-holiday events.Also applies to: 25-27
33-33: 🛠️ Refactor suggestionAdd date validation to prevent runtime errors.
The direct construction of Date from
event.enDatecould throw if the date is invalid. Consider adding validation or using a date parsing library.Let's verify the format of enDate in other files:
Consider adding validation:
-{relativeTimeFromDates(new Date(event.enDate))} +{event.enDate ? relativeTimeFromDates(new Date(event.enDate)) : 'Invalid date'}apps/vite/src/Body.tsx (2)
29-32: LGTM: Route formatting improvementThe GoogleApiDisclosure route formatting enhances code readability while maintaining the same functionality.
23-25: Review calendar routing changesThe routing changes make Calendar2 the primary view by:
- Setting it as the root route
- Supporting optional year/month parameters
- Removing the previous Home component route
Please ensure this aligns with the intended user experience and that the Home component's functionality is properly migrated or no longer needed.
#!/bin/bash # Check for any remaining references to the Home component route pattern rg "/:pageType\?/:BSYear\?/:BSMonth\?" # Check if Home component is used elsewhere rg -l "element={<Home"apps/vite/src/components/calendar/TodayEventList.tsx (2)
1-53: Overall implementation looks good!The component is well-structured with:
- Clean separation of concerns between list and single event display
- Proper use of TypeScript for type safety
- Consistent styling with Tailwind CSS
- Good handling of empty states
🧰 Tools
🪛 Biome
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 21-26: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
36-38:⚠️ Potential issueFix date formatting to use event date instead of current date.
The component is using
new NepaliDate()which always returns the current date. This could lead to incorrect date display if the event is for a different date.The date should be derived from the event data instead. Consider adding a date field to the EventDetail type and using that for display:
-{nepaliNumber(new NepaliDate().format("DD"))} +{nepaliNumber(new NepaliDate(event.date).format("DD"))}Likely invalid or redundant comment.
apps/vite/src/components/Spinner.tsx (4)
1-1: LGTM: Import statement update is consistent with codebase changesThe change from
classNamestocnutility aligns with the broader refactoring mentioned in the PR summary.
9-12: LGTM: Proper usage of cn utility for className compositionThe className composition using
cnutility is implemented correctly, maintaining the base styles while allowing for className prop override.
6-6: 🛠️ Refactor suggestionConsider using a semantic status element
Instead of using a
divwithrole="status", consider using the native HTML<status>element for better semantic meaning and accessibility.- <div role="status"> + <status>Likely invalid or redundant comment.
🧰 Tools
🪛 Biome
[error] 6-6: The elements with the following roles can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
Line range hint
1-32: Verify loading state managementThe Spinner component appears to be used for loading states. Ensure proper loading state management in parent components to prevent unnecessary spinner renders.
✅ Verification successful
Let me check the loading state management in these components by examining their implementation.
Let me check the complete loading state logic in these components to ensure they're properly managed.
Loading states are properly managed across all Spinner usages
The verification shows that loading states are correctly implemented in all components using the Spinner:
- Home.tsx: Uses
isLoadingfromuseYearlyDatahook to conditionally render Spinner- EventDetailsDialog.tsx: Uses
isPendingfromuseDeleteEventmutation to show loading state during deletion- AddEventModal.tsx: Uses both
isCalendarListLoadingfor calendar data andisPendingfor event creation- All components properly handle loading states with conditional rendering and disable interactive elements during loading
The implementation follows best practices for loading state management and prevents unnecessary Spinner renders.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Spinner component usage to verify loading state handling rg -A 3 "<Spinner"Length of output: 981
Script:
#!/bin/bash # Check loading state implementation in components using Spinner echo "=== Home.tsx ===" rg -B 5 -A 2 "<Spinner.*className=\"h-5 w-5\"" apps/vite/src/pages/Home.tsx echo -e "\n=== EventDetailsDialog.tsx ===" rg -B 5 -A 2 "<Spinner.*className=\"h-5 w-5 fill-white\"" apps/vite/src/components/EventDetailsDialog.tsx echo -e "\n=== AddEventModal.tsx ===" rg -B 5 -A 2 "<Spinner />" apps/vite/src/components/AddEventModal.tsxLength of output: 1308
Script:
#!/bin/bash # Search for loading state variables and their usage echo "=== Home.tsx loading states ===" rg -B 5 -A 10 "isPending|isLoading" apps/vite/src/pages/Home.tsx echo -e "\n=== EventDetailsDialog.tsx loading states ===" rg -B 5 -A 10 "isPending|isLoading" apps/vite/src/components/EventDetailsDialog.tsx echo -e "\n=== AddEventModal.tsx loading states ===" rg -B 5 -A 10 "isPending|isLoading" apps/vite/src/components/AddEventModal.tsxLength of output: 4538
🧰 Tools
🪛 Biome
[error] 6-6: The elements with the following roles can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
apps/vite/src/index.css (1)
14-71: LGTM! Well-structured theming system.The implementation provides a comprehensive theming solution with:
- Consistent variable naming
- Good separation of light and dark themes
- Complete coverage of UI elements
- Proper use of CSS custom properties
apps/vite/tailwind.config.js (2)
13-17: LGTM! Well-structured border radius system.The border radius system is well-designed with:
- Base variable for consistency
- Logical size progression
- Maintainable calculations
18-59: Verify CSS variable definitions for the color system.The color system implementation is excellent, following best practices with HSL and semantic naming. However, we should ensure all CSS variables are properly defined.
apps/vite/package.json (4)
15-16: LGTM: Calendar data dependencies are correctly configured.The addition of
@miti/queryand@miti/typesas workspace dependencies aligns with the PR objective of loading calendar data.
40-60: Review development tooling version compatibility.Several development dependencies have been upgraded to major new versions:
- ESLint 9.x
- TypeScript 5.5.x
- Vite 5.x
These represent significant version jumps that could introduce breaking changes.
Consider:
- Documenting these major version upgrades in the changelog
- Creating a separate PR for major version upgrades to isolate potential issues
- Adding an engines field to package.json to specify Node.js version requirements
#!/bin/bash # Verify compatibility between TypeScript and other dev dependencies npm info [email protected] peerDependencies --json npm info @vitejs/[email protected] peerDependencies --json
Line range hint
7-8: Verify TypeScript build configuration.The build script
tsc -b && vite buildsuggests a separate TypeScript build step. This might be redundant as Vite can handle TypeScript compilation.#!/bin/bash # Check for tsconfig.json and potential build configuration fd tsconfig.json
17-21: Verify UI component dependencies versions.Several new UI-related dependencies have been added for enhanced functionality:
- Radix UI components for dialogs
- Styling utilities (class-variance-authority, clsx, tailwind-merge)
- Lucide icons
- Vaul for drawer components
While these additions enhance the UI capabilities, some version specifications use
^which could lead to unexpected breaking changes.Also applies to: 27-27, 34-36
✅ Verification successful
UI dependencies are safely scoped to the Vite app
The UI-related dependencies (@radix-ui/react-dialog, class-variance-authority, clsx, and vaul) are only used in the Vite app and not shared across other packages in the monorepo. This localized usage means:
- Version conflicts are unlikely as these packages are only declared once
- The caret (^) versioning is less risky since breaking changes would only impact the Vite app
- The team can manage updates within the scope of a single application
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential breaking changes in the new UI dependencies npm info @radix-ui/react-dialog versions --json | jq -r '.[]' | grep '^1\.1\.' npm info class-variance-authority versions --json | jq -r '.[]' | grep '^0\.7\.' npm info vaul versions --json | jq -r '.[]' | grep '^1\.1\.'Length of output: 875
Script:
#!/bin/bash # Let's try a different approach to check the dependencies # First, let's check if these packages are used elsewhere in the codebase rg "@radix-ui/react-dialog" --type json rg "class-variance-authority" --type json rg "clsx" --type json rg "vaul" --type json # Also check for any potential version conflicts in package.json files fd "package.json" -x cat {} \; | grep -A 1 -B 1 "@radix-ui/react-dialog\|class-variance-authority\|clsx\|vaul"Length of output: 785
apps/vite/src/components/calendar/DayDetails.tsx (3)
5-5: LGTM! Props are well-typed.The component follows TypeScript best practices with proper type annotations for the props.
5-70: Overall implementation looks good!The component is well-structured with:
- Semantic HTML
- Responsive layout
- Proper TypeScript usage
- Clear separation of concerns
The suggested improvements are mostly refinements to an already solid implementation.
66-66: Verify Panchang component's required props.Consider being explicit about which properties from
dayDataare actually needed by the Panchang component. This would make the component contract clearer and potentially improve performance.apps/vite/src/components/ui/drawer.tsx (4)
17-21: LGTM!Clean re-export of primitive components.
56-77: LGTM!Clean implementation of header and footer components with proper type safety.
105-116: LGTM!Clean and complete exports of all drawer components.
23-34: Verify z-index consistency across overlay components.The z-index value of 50 needs to be verified against other overlay components (like modals, dialogs) to ensure proper stacking order.
✅ Verification successful
Z-index values are consistent across overlay components
The z-index value of 50 is consistently used across all overlay components in the UI system:
- Dialog overlay: z-50
- Dialog content: z-50
- Drawer overlay: z-50
- Drawer content: z-50
This maintains a uniform stacking context for all overlay components, ensuring proper layering behavior.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other z-index values in overlay components rg -g '*.tsx' -g '*.css' 'z-\d+|z-\[.*\]' apps/vite/src/components/ui/Length of output: 1126
apps/vite/src/components/YearMonthPicker.tsx (1)
4-4: Verify the import path alias configurationThe
@alias in the import path needs proper configuration in your build setup.✅ Verification successful
The import path alias
@is properly configured in both TypeScript and Vite configurations:
- TypeScript config has
"@/*": ["./src/*"]in paths- Vite config has
"@": path.resolve(__dirname, "./src")in aliasThis means the import statement
import { cn } from "@/lib/utils"will correctly resolve tosrc/lib/utils.Let's verify if the utility file exists and contains the
cnexport:
Let's try searching for the utils file in the entire repository since the
src/libpath wasn't found:
Import path and utility function are correctly configured
The
cnutility function is properly exported fromapps/vite/src/lib/utils.tsand the@path alias is correctly configured in both TypeScript and Vite configs. The import statement will work as expected.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the alias is properly configured in the relevant config files echo "Checking for alias configuration..." fd -t f "tsconfig.*json|vite.config.*" -x cat {} \; | grep -A 5 "paths\|alias"Length of output: 523
Script:
#!/bin/bash # Check if the utils file exists and contains the cn export echo "Checking for utils file..." fd -t f "utils\.(ts|js|tsx|jsx)$" src/lib echo -e "\nChecking for cn export..." rg -U "export.*\bcn\b" -A 3 src/lib/utilsLength of output: 366
Script:
#!/bin/bash # Search for utils file in the entire repository echo "Searching for utils file..." fd -t f "utils\.(ts|js|tsx|jsx)$" echo -e "\nSearching for cn export in any utils file..." rg -U "export.*\bcn\b" -A 3Length of output: 450
apps/vite/src/components/ui/dialog.tsx (5)
1-14: LGTM! Clean imports and component definitions.The imports are well-organized and the basic component definitions follow best practices for Radix UI integration.
54-80: LGTM! Well-implemented responsive layouts.The header and footer components have clean implementations with good responsive behavior and semantic class names.
82-107: LGTM! Accessible and well-styled text components.The title and description components are properly implemented with appropriate typography and accessibility considerations.
109-120: LGTM! Clean and organized exports.All components are properly exported in a logical order.
1-120: Verify theme system integration.Since this is a new component, please ensure it works correctly with the application's theme system, especially for dark mode support.
✅ Verification successful
Theme integration is properly implemented ✓
The dialog component correctly integrates with the application's theme system:
- Uses CSS variables defined in the dark/light themes (
index.css)- Implements theme-aware classes like
bg-background,text-muted-foreground- Works with the app's
DarkModeProvidercontext which handles theme switching🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for theme-related utilities and configurations echo "Checking for theme configuration..." rg -l "theme|dark|light" apps/vite/src/Length of output: 1007
Script:
#!/bin/bash # Let's check the theme implementation and configuration echo "Checking DarkModeProvider implementation..." cat apps/vite/src/components/DarkModeProvider.tsx echo -e "\nChecking index.css for theme variables..." cat apps/vite/src/index.css echo -e "\nChecking for theme-related class usage in dialog component..." rg -n "bg-background|text-muted-foreground|ring-ring|ring-offset-background" apps/vite/src/components/ui/dialog.tsxLength of output: 4295
apps/vite/src/components/UserSettings.tsx (3)
6-6: LGTM: Import change aligns with codebase refactoring.The replacement of classNames with cn utility is consistent with the broader refactoring effort across components.
Line range hint
75-79: LGTM: Clean implementation of menu item styling.The conditional styling with cn utility is well-implemented, maintaining proper dark mode support and active state handling.
Line range hint
71-74: Verify authentication flow security.The authentication redirect uses window.location.origin as a parameter. Let's verify that the API properly validates this redirect URL to prevent open redirect vulnerabilities.
✅ Verification successful
Let me search for more context about the authentication implementation.
Let me check the authentication controller implementation to verify the redirect handling.
Redirect URL validation is properly implemented and secure
The authentication flow has proper redirect URL validation:
- The API enforces a strict whitelist of allowed redirect URLs through Zod validation:
redirect: z.enum(["io.veez.miti://", "http://localhost:5173", "https://miti.bikram.io"])- The redirect URL is stored in an HTTP-only cookie with secure flags
- The callback endpoint validates the state parameter and other OAuth requirements before processing the redirect
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for redirect URL validation in the API codebase rg -g '!{*.json,*.md}' -i 'redirect.*validation|validate.*redirect' # Look for other instances of auth endpoint usage rg -g '!{*.json,*.md}' '/auth/google\?redirect'Length of output: 248
Script:
#!/bin/bash # Look for auth-related code and URL handling rg -g '!{*.json,*.md}' -B 3 -A 3 'auth.*google|validateRedirect|allowedOrigins' # Search for environment configuration related to allowed origins/domains rg -g '!{*.json,*.md}' 'ALLOWED_ORIGINS|CORS_ORIGIN|REDIRECT_URL' # Look for OAuth or authentication configuration fd -e ts -e js -e tsx -e jsx -e config --exec grep -l -i "oauth\|auth.*config"Length of output: 2849
Script:
#!/bin/bash # Check auth controller implementation ast-grep --pattern 'export $_ function $_($$_) { $$$ redirect $$$ }' # Look for specific redirect validation in auth files rg -g '*.ts' -B 5 -A 5 'redirect.*=|redirect.*\?' apps/api/src/controller/auth/ # Check OAuth configuration and setup cat apps/api/src/controller/auth/auth.controller.tsLength of output: 8657
apps/vite/src/components/Navbar.tsx (3)
Line range hint
59-64: LGTM! Clean implementation of conditional styling.The change to use
cn()maintains the same functionality while aligning with the new utility convention.
Line range hint
102-106: LGTM! Consistent styling implementation.The mobile menu styling follows the same pattern as the main navigation, maintaining consistency throughout the component.
8-8: Verify path alias configuration for "@/" imports.The change to use the
@/path alias is consistent with the refactoring, but let's ensure the alias is properly configured in the build setup.✅ Verification successful
Path alias
@/is correctly configuredThe
@/path alias is properly set up in both:
vite.config.ts: Configured in resolve.alias mapping to./srcapps/vite/tsconfig.json: Configured in paths mapping to./src/*🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify path alias configuration in vite config # Test: Search for path alias configuration rg -A 5 "alias.*lib" "vite.config" "tsconfig"Length of output: 144
Script:
#!/bin/bash # Search for vite config files recursively fd -t f "vite.config" # Search for tsconfig files recursively fd -t f "tsconfig.json" # Once we find the config files, let's check their content for alias configurationLength of output: 408
Script:
#!/bin/bash # Check vite config for alias configuration cat apps/vite/vite.config.ts # Check relevant tsconfig files for path aliases cat apps/vite/tsconfig.json cat tsconfig.jsonLength of output: 850
| export function cn(...inputs: ClassValue[]) { | ||
| return twMerge(clsx(inputs)) | ||
| } |
There was a problem hiding this comment.
Fix the clsx function call to properly spread arguments.
The current implementation passes the array as a single argument to clsx, which may not work as expected. The array should be spread to pass individual arguments.
Apply this fix:
export function cn(...inputs: ClassValue[]) {
- return twMerge(clsx(inputs))
+ return twMerge(clsx(...inputs))
}📝 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 cn(...inputs: ClassValue[]) { | |
| return twMerge(clsx(inputs)) | |
| } | |
| export function cn(...inputs: ClassValue[]) { | |
| return twMerge(clsx(...inputs)) | |
| } |
| export const padNumber = (num: number, length: number) => { | ||
| return Number.parseInt(num.toString().padStart(length, "0")) | ||
| } |
There was a problem hiding this comment.
Revise the padNumber implementation for better handling of edge cases.
The current implementation has several issues:
- Converting back to number removes leading zeros, defeating the purpose of padding
- No validation for the length parameter
- Negative numbers aren't handled properly
Consider this improved implementation:
-export const padNumber = (num: number, length: number) => {
- return Number.parseInt(num.toString().padStart(length, "0"))
+export const padNumber = (num: number, length: number): string => {
+ if (length < 1) throw new Error("Length must be positive");
+ const absNum = Math.abs(num);
+ const sign = num < 0 ? "-" : "";
+ return sign + absNum.toString().padStart(length, "0");
}This version:
- Returns a string to preserve leading zeros
- Properly handles negative numbers
- Validates the length parameter
- Removes unnecessary number conversion
📝 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 const padNumber = (num: number, length: number) => { | |
| return Number.parseInt(num.toString().padStart(length, "0")) | |
| } | |
| export const padNumber = (num: number, length: number): string => { | |
| if (length < 1) throw new Error("Length must be positive"); | |
| const absNum = Math.abs(num); | |
| const sign = num < 0 ? "-" : ""; | |
| return sign + absNum.toString().padStart(length, "0"); | |
| } |
| function useMediaQuery(query: string): boolean { | ||
| const [matches, setMatches] = useState(false) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider initializing state with the current media query match
The initial state is set to false without checking the current media query match. This could cause a flash of incorrect content on the first render.
function useMediaQuery(query: string): boolean {
- const [matches, setMatches] = useState(false)
+ const [matches, setMatches] = useState(() => window.matchMedia(query).matches)📝 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.
| function useMediaQuery(query: string): boolean { | |
| const [matches, setMatches] = useState(false) | |
| function useMediaQuery(query: string): boolean { | |
| const [matches, setMatches] = useState(() => window.matchMedia(query).matches) |
| useEffect(() => { | ||
| const mediaQueryList = window.matchMedia(query) | ||
| setMatches(mediaQueryList.matches) | ||
| const handleChange = (event: MediaQueryListEvent) => { | ||
| setMatches(event.matches) | ||
| } | ||
| mediaQueryList.addEventListener("change", handleChange) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding browser compatibility checks
While the implementation uses the modern addEventListener API, some older browsers might not support matchMedia. Consider adding a fallback mechanism.
useEffect(() => {
+ if (!window.matchMedia) {
+ console.warn('matchMedia is not supported in this browser');
+ return;
+ }
const mediaQueryList = window.matchMedia(query)
setMatches(mediaQueryList.matches)
const handleChange = (event: MediaQueryListEvent) => {
setMatches(event.matches)
}
mediaQueryList.addEventListener("change", handleChange)Committable suggestion was skipped due to low confidence.
| <Dialog open={open} onOpenChange={setOpen}> | ||
| <DialogContent className="sm:max-w-[425px]"> | ||
| <DialogHeader> | ||
| <DialogTitle>आजको विवरण</DialogTitle> | ||
| </DialogHeader> | ||
| {children} | ||
| </DialogContent> | ||
| </Dialog> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing accessibility
While the base Dialog and Drawer components likely handle basic accessibility, consider adding:
- aria-label or aria-describedby for better screen reader context
- Keyboard navigation handlers for better interaction
<Dialog open={open} onOpenChange={setOpen}>
- <DialogContent className="sm:max-w-[425px]">
+ <DialogContent
+ className="sm:max-w-[425px]"
+ aria-describedby="day-dialog-description"
+ >
<DialogHeader>
<DialogTitle>आजको विवरण</DialogTitle>
</DialogHeader>
- {children}
+ <div id="day-dialog-description">
+ {children}
+ </div>
</DialogContent>
</Dialog>Also applies to: 44-52
| className="text-sm rounded-lg flex-1 items-end text-end transition-all duration-300 text-indigo-500 hover:text-indigo-600" | ||
| onClick={() => setIsOpen(!isOpen)} | ||
| > | ||
| आजको पंचांग{" "} | ||
| {isOpen ? ( |
There was a problem hiding this comment.
Add explicit 'type' prop to the button element
The default type for a <button> is 'submit', which may cause unintended form submissions. Specify the 'type' prop to ensure the button behaves as expected in your React application.
Apply this diff to fix the issue:
<button
+ type="button"
className="text-sm rounded-lg flex-1 items-end text-end transition-all duration-300 text-indigo-500 hover:text-indigo-600"
onClick={() => setIsOpen(!isOpen)}
>📝 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.
| className="text-sm rounded-lg flex-1 items-end text-end transition-all duration-300 text-indigo-500 hover:text-indigo-600" | |
| onClick={() => setIsOpen(!isOpen)} | |
| > | |
| आजको पंचांग{" "} | |
| {isOpen ? ( | |
| type="button" | |
| className="text-sm rounded-lg flex-1 items-end text-end transition-all duration-300 text-indigo-500 hover:text-indigo-600" | |
| onClick={() => setIsOpen(!isOpen)} | |
| > | |
| आजको पंचांग{" "} | |
| {isOpen ? ( |
🧰 Tools
🪛 Biome
[error] 78-82: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
| className={cn( | ||
| "inline-flex items-center transition-colors duration-300 ease-in focus:outline-none hover:text-blue-400 focus:text-blue-400 rounded-md px-2 py-2", | ||
| view === "event" ? "bg-white" : "" | ||
| )} | ||
| id="list" | ||
| onClick={() => setView('event')} | ||
| onClick={() => setView("event")} | ||
| > |
There was a problem hiding this comment.
Add type="button" to prevent unintended form submissions
Specify the type prop to prevent unintended form submissions when the button is within a form element.
Apply this diff:
onClick={() => setView("event")}
+ type="button"
>📝 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.
| className={cn( | |
| "inline-flex items-center transition-colors duration-300 ease-in focus:outline-none hover:text-blue-400 focus:text-blue-400 rounded-md px-2 py-2", | |
| view === "event" ? "bg-white" : "" | |
| )} | |
| id="list" | |
| onClick={() => setView('event')} | |
| onClick={() => setView("event")} | |
| > | |
| className={cn( | |
| "inline-flex items-center transition-colors duration-300 ease-in focus:outline-none hover:text-blue-400 focus:text-blue-400 rounded-md px-2 py-2", | |
| view === "event" ? "bg-white" : "" | |
| )} | |
| id="list" | |
| onClick={() => setView("event")} | |
| type="button" | |
| > |
🧰 Tools
🪛 Biome
[error] 34-41: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
| <button | ||
| className="hidden md:flex py-2 pl-1.5 pr-3 rounded-md bg-gray-50 border border-gray-300 items-center gap-1.5 text-xs font-medium text-gray-900 transition-all duration-500 hover:bg-gray-200" | ||
| onClick={() => setCurrentNepaliDate(new NepaliDate())} | ||
| > |
There was a problem hiding this comment.
Add type="button" to prevent unintended form submissions
Specify the type prop for this button to prevent unintended form submissions when the button is within a form element.
Apply this diff:
onClick={() => setCurrentNepaliDate(new NepaliDate())}
+ type="button"
>📝 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.
| <button | |
| className="hidden md:flex py-2 pl-1.5 pr-3 rounded-md bg-gray-50 border border-gray-300 items-center gap-1.5 text-xs font-medium text-gray-900 transition-all duration-500 hover:bg-gray-200" | |
| onClick={() => setCurrentNepaliDate(new NepaliDate())} | |
| > | |
| <button | |
| className="hidden md:flex py-2 pl-1.5 pr-3 rounded-md bg-gray-50 border border-gray-300 items-center gap-1.5 text-xs font-medium text-gray-900 transition-all duration-500 hover:bg-gray-200" | |
| onClick={() => setCurrentNepaliDate(new NepaliDate())} | |
| type="button" | |
| > |
🧰 Tools
🪛 Biome
[error] 45-48: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
| className={cn( | ||
| "inline-flex items-center transition-colors duration-300 ease-in focus:outline-none hover:text-blue-400 focus:text-blue-400 rounded-md px-2 py-2 active", | ||
| view === "calendar" ? "bg-white" : "" | ||
| )} | ||
| id="grid" | ||
| onClick={() => setView('calendar')} | ||
| onClick={() => setView("calendar")} | ||
| > |
There was a problem hiding this comment.
Add type="button" to prevent unintended form submissions
Without specifying the type prop, the default type of a button is submit, which may cause unintended form submissions when the button is within a form element.
Apply this diff to fix the issue:
onClick={() => setView("calendar")}
+ type="button"
>📝 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.
| className={cn( | |
| "inline-flex items-center transition-colors duration-300 ease-in focus:outline-none hover:text-blue-400 focus:text-blue-400 rounded-md px-2 py-2 active", | |
| view === "calendar" ? "bg-white" : "" | |
| )} | |
| id="grid" | |
| onClick={() => setView('calendar')} | |
| onClick={() => setView("calendar")} | |
| > | |
| className={cn( | |
| "inline-flex items-center transition-colors duration-300 ease-in focus:outline-none hover:text-blue-400 focus:text-blue-400 rounded-md px-2 py-2 active", | |
| view === "calendar" ? "bg-white" : "" | |
| )} | |
| id="grid" | |
| onClick={() => setView("calendar")} | |
| type="button" | |
| > |
🧰 Tools
🪛 Biome
[error] 24-31: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
| import { classNames } from '../../helper/utils' | ||
| import EventIcon from '../icons/EventIcon' | ||
| import ListIcon from '../icons/ListIcon' | ||
| import React, { useState } from "react" |
There was a problem hiding this comment.
Use import type for type-only imports to optimize module loading
The default import React is only used as a type. Using import type ensures that unnecessary modules are not loaded by the compiler.
Apply this diff to fix the issue:
-import React, { useState } from "react"
+import type { FC } from "react"
+import { useState } from "react"Then update the component definition accordingly:
-const CalendarHeader: React.FC<CalendarHeaderProps> = ({
+const CalendarHeader: FC<CalendarHeaderProps> = ({Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 Biome
[error] 1-1: The default import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
Deploying miti with
|
| Latest commit: |
ad921cb
|
| Status: | 🚫 Build failed. |
changed the layout of the container to make calendar more useful on a bigger screen, changed the calendar header layout
There was a problem hiding this comment.
Actionable comments posted: 27
🧹 Outside diff range and nitpick comments (15)
packages/query/src/query-provider.tsx (1)
14-18: Consider documenting recommended QueryClient configurations.
Since the component now supports custom QueryClient instances, it would be valuable to document recommended configurations, especially for calendar-related queries (e.g., caching strategies, stale time, retry policies).
apps/vite/src/hooks/useMediaQuery.tsx (1)
3-3: Consider using a more specific type for the query parameter.
The query parameter could benefit from a more specific type to ensure valid CSS media query strings are passed.
-function useMediaQuery(query: string): boolean {
+type CSSMediaQuery = string;
+function useMediaQuery(query: CSSMediaQuery): boolean {packages/query/src/user/useNavigatorOnline.ts (1)
Line range hint 14-22: Consider enhancing browser environment check
While the window check is good, consider adding a more comprehensive check that aligns with the getOnLineStatus function's checks for better consistency.
React.useEffect(() => {
- if (typeof window === "undefined") return
+ if (typeof window === "undefined" || typeof navigator === "undefined") return
+ if (typeof navigator.onLine !== "boolean") return
+
window.addEventListener("online", setOnline)
window.addEventListener("offline", setOffline)🧰 Tools
🪛 Biome
[error] 8-8: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
apps/vite/src/components/calendar/EventList.tsx (2)
6-13: Add JSDoc documentation for the Event type.
Consider adding documentation to explain the purpose and usage of each field in the Event type. This will improve code maintainability and help other developers understand the data structure better.
+/**
+ * Represents a calendar event with both Nepali and English date information
+ * @property {string} date - The day of the month in Nepali
+ * @property {string} enDate - The full date in English (AD)
+ * @property {boolean} isHoliday - Indicates if this event is a holiday
+ * @property {string} day - The day of week in Nepali
+ * @property {string} title - The event title in Nepali
+ * @property {string} fullDate - The full date in Nepali (BS)
+ */
export type Event = {
date: string
enDate: string
isHoliday: boolean
day: string
title: string
fullDate: string
}66-68: Remove commented out code.
Commented out code should be removed as it can be retrieved from version control if needed.
apps/vite/src/components/calendar/DayDetails.tsx (2)
1-4: Optimize type imports for better compilation.
Use import type for type-only imports to ensure proper tree-shaking and avoid loading unnecessary modules at runtime.
import { cn } from "@/lib/utils"
-import { NewCalendarData } from "@miti/types"
+import type { NewCalendarData } from "@miti/types"
import Panchang from "./Panchang"🧰 Tools
🪛 Biome
[error] 2-2: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
10-32: Enhance accessibility for date information.
While the date display is well-structured, consider adding ARIA labels to improve screen reader support.
-<div className="text-center w-16 h-16 flex flex-col gap-1 items-center justify-center">
+<div
+ className="text-center w-16 h-16 flex flex-col gap-1 items-center justify-center"
+ role="region"
+ aria-label="Selected date information"
+>apps/vite/src/components/calendar/Panchang.tsx (1)
36-44: Add TypeScript interface for props.
Consider adding a proper interface for the component props to improve type safety.
+interface MuhuratItemProps {
+ name: string;
+ time: string;
+}
+
-const MuhuratItem = ({ name, time }: { name: string; time: string }) => {
+const MuhuratItem = ({ name, time }: MuhuratItemProps) => {apps/vite/src/components/YearMonthPicker.tsx (1)
43-45: Optimize button styles by removing redundant disabled states.
The disabled styles contain redundant hover states that can be simplified:
-"flex flex-none items-center justify-center rounded-lg bg-indigo-600 p-1.5 text-white hover:bg-indigo-700 disabled:cursor-not-allowed disabled:bg-blue-600 disabled:text-white disabled:opacity-20 disabled:hover:cursor-not-allowed disabled:hover:bg-blue-600 disabled:hover:text-white"
+"flex flex-none items-center justify-center rounded-lg bg-indigo-600 p-1.5 text-white hover:bg-indigo-700 disabled:cursor-not-allowed disabled:bg-blue-600 disabled:opacity-20"The disabled:text-white and disabled:hover:* classes are unnecessary because:
- Text is already white in disabled state
- hover styles don't apply to disabled elements
Also applies to: 93-95
apps/vite/src/pages/Calendar2.tsx (1)
Line range hint 102-107: Remove hardcoded exchange rate
The CurrencyConverterCard component uses a hardcoded exchange rate of 134.21, which will become outdated.
Consider:
- Moving this to configuration
- Fetching real-time exchange rates
- Adding a last-updated timestamp
Example implementation:
interface ExchangeRate {
rate: number;
lastUpdated: Date;
}
const { data: exchangeRate } = useExchangeRate('USD', 'NPR');
// In JSX
<CurrencyConverterCard
initialAmount={1}
exchangeRate={exchangeRate?.rate}
lastUpdated={exchangeRate?.lastUpdated}
fromCurrency="USD"
toCurrency="NPR"
/>🧰 Tools
🪛 Biome
[error] 17-17: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 27-27: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
[error] 28-28: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
packages/query/src/calendar/index.ts (1)
27-31: Improve documentation clarity
The current comment about naming difficulty could be more professional. Consider replacing it with a clear explanation of the month/year transition logic.
- /**
- * Naming is hard :D
- * the previous year may be the previous year or the same year
- * the next year may be the next year or the same year
- */
+ /**
+ * Note on year transitions:
+ * When calculating adjacent months, the year may or may not change:
+ * - Previous month might be in the current year or previous year
+ * - Next month might be in the current year or next year
+ */apps/vite/src/components/calendar/CalendarGrid.tsx (1)
Line range hint 95-105: Extract hardcoded event text and improve event rendering logic.
The developer meetup text is hardcoded and the event rendering logic could be improved:
- Move the text to constants or props
- Consider extracting event rendering to a separate component
- The index % 3 condition seems arbitrary for event display
Consider creating an EventIndicator component:
type EventIndicatorProps = {
event: {
title: string;
type: string;
};
compact?: boolean;
}
const EventIndicator: React.FC<EventIndicatorProps> = ({ event, compact }) => (
<div className="absolute bottom-1 left-1 p-1.5 h-max rounded xl:bg-emerald-50 z-10 flex-row">
{compact ? (
<p className="w-2 h-2 rounded-full bg-emerald-600" />
) : (
<p className="text-xs font-medium text-emerald-600 mb-px whitespace-nowrap">
{event.title}
</p>
)}
</div>
);Then use it in the grid:
- {index % 3 === 0 && (
- <div className="absolute bottom-1 left-1 p-1.5 h-max rounded xl:bg-emerald-50 z-10 flex-row">
- // ... current event markup
- </div>
- )}
+ {day.eventDetails.map(event => (
+ <EventIndicator
+ key={event.id}
+ event={event}
+ compact={!isXLScreen}
+ />
+ ))}🧰 Tools
🪛 Biome
[error] 71-71: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 2-2: The default import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 61-76: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
apps/vite/src/components/Navbar.tsx (2)
83-92: Remove commented-out code.
The old implementation with nested ternaries has been improved, but the commented-out code should be removed to maintain cleaner codebase.
119-124: Clean up or document commented features.
There are multiple commented-out components (LanguageChangeDropDown and InstallPWA) in the mobile navigation panel. If these are planned features, they should be tracked in issues rather than kept as comments in the code.
Would you like me to help create GitHub issues to track these planned features?
apps/vite/src/components/calendar/CalendarHeader.tsx (1)
18-98: Remove commented code
Large blocks of commented code should be removed as they can be retrieved from version control if needed.
Remove all commented code between lines 18-98 to improve code readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (21)
- .gitignore (1 hunks)
- apps/vite/src/components/AddEventModal.tsx (0 hunks)
- apps/vite/src/components/DropDown.tsx (2 hunks)
- apps/vite/src/components/Navbar.tsx (2 hunks)
- apps/vite/src/components/YearMonthPicker.tsx (2 hunks)
- apps/vite/src/components/calendar/CalendarGrid.tsx (2 hunks)
- apps/vite/src/components/calendar/CalendarHeader.tsx (1 hunks)
- apps/vite/src/components/calendar/DayDetails.tsx (1 hunks)
- apps/vite/src/components/calendar/EventList.tsx (1 hunks)
- apps/vite/src/components/calendar/Panchang.tsx (2 hunks)
- apps/vite/src/components/calendar/Today.tsx (1 hunks)
- apps/vite/src/components/calendar/TodayEventList.tsx (1 hunks)
- apps/vite/src/components/calendar/UpcomingEvent.tsx (2 hunks)
- apps/vite/src/hooks/useMediaQuery.tsx (1 hunks)
- apps/vite/src/pages/Calendar2.tsx (1 hunks)
- apps/vite/src/pages/Home.tsx (1 hunks)
- apps/vite/tailwind.config.js (1 hunks)
- packages/query/src/calendar/index.ts (1 hunks)
- packages/query/src/event/index.ts (2 hunks)
- packages/query/src/query-provider.tsx (1 hunks)
- packages/query/src/user/useNavigatorOnline.ts (1 hunks)
💤 Files with no reviewable changes (1)
- apps/vite/src/components/AddEventModal.tsx
🧰 Additional context used
🪛 Biome
apps/vite/src/components/DropDown.tsx
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
[error] 49-49: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
apps/vite/src/components/Navbar.tsx
[error] 74-74: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
apps/vite/src/components/calendar/CalendarGrid.tsx
[error] 71-71: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 2-2: The default import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 61-76: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 114-114: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
apps/vite/src/components/calendar/CalendarHeader.tsx
[error] 152-158: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
[error] 1-1: The default import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 102-108: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 111-117: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 122-125: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 140-140: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 143-143: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 146-146: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 151-151: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
apps/vite/src/components/calendar/DayDetails.tsx
[error] 2-2: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
apps/vite/src/components/calendar/EventList.tsx
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 52-52: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
[error] 64-64: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
apps/vite/src/components/calendar/Panchang.tsx
[error] 2-2: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 30-30: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
[error] 119-119: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
apps/vite/src/components/calendar/Today.tsx
[error] 105-105: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 106-109: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 111-112: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 113-113: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 114-117: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 4-4: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 78-82: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
apps/vite/src/components/calendar/TodayEventList.tsx
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
apps/vite/src/components/calendar/UpcomingEvent.tsx
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
apps/vite/src/pages/Calendar2.tsx
[error] 17-17: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 27-27: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
[error] 28-28: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
packages/query/src/user/useNavigatorOnline.ts
[error] 8-8: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
🔇 Additional comments (37)
.gitignore (1)
51-51:
Security Risk: Remove .env from version control
Including .env files in version control is a security risk as they typically contain sensitive information like API keys and credentials.
Please make the following changes:
- Remove the
.enventry to ensure it's excluded from version control - Add a
.env.exampletemplate file instead - Update documentation to guide developers on creating their local
.envfile
Let's verify if the current .env file contains sensitive data:
packages/query/src/query-provider.tsx (1)
10-11: LGTM! Well-structured type extension.
The type extension is clean and maintains backward compatibility while adding flexibility for custom QueryClient instances.
apps/vite/src/hooks/useMediaQuery.tsx (1)
1-21: LGTM for the core implementation!
The hook implementation is solid with proper cleanup, memoized initial state, and correct dependency array usage. The previous suggestion about initializing state with the current media query match has been properly addressed.
packages/query/src/user/useNavigatorOnline.ts (1)
Line range hint 1-25: Verify the hook's usage across the codebase
The changes to the hook's implementation and type signature might affect its consumers.
✅ Verification successful
No breaking changes detected in hook usage
The hook is only used internally within the packages/query/src/user module, where it's imported and used in index.ts. The changes to the implementation are contained and don't affect external consumers since:
- The hook maintains its boolean return type
- It's only used in one place within the same module
- The export signature remains unchanged
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all usages of useNavigatorOnLine to verify compatibility
rg "useNavigatorOnLine" --type ts --type tsx
Length of output: 72
Script:
#!/bin/bash
# Search for all usages of useNavigatorOnLine with ripgrep, without file type filters
rg "useNavigatorOnLine"
# Also search for potential exports of this hook
rg "export.*useNavigatorOnLine"
Length of output: 466
🧰 Tools
🪛 Biome
[error] 8-8: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
packages/query/src/event/index.ts (2)
36-45: LGTM! Good security practice with URL encoding.
The addition of encodeURIComponent for the event ID is a good security practice that prevents URL injection and handles special characters correctly.
36-45: 🛠️ Refactor suggestion
Verify response status codes.
The function should verify that the delete operation was successful by checking the response status.
export const deleteEvent = async (apiBaseUrl: string, event: CalendarEvent) => {
if (!event?.id) {
throw new Error("Invalid event: Missing event ID");
}
try {
- await fetch(
+ const response = await fetch(
`${apiBaseUrl}/calendar/google/delete/${encodeURIComponent(event.id)}`,
{
method: "DELETE",
credentials: "include",
headers: {
"Content-Type": "application/json",
},
}
)
+ if (!response.ok) {
+ throw new Error(`Failed to delete event: ${response.status}`);
+ }
return true;
} catch (error) {
console.error("Failed to delete event:", error);
throw error;
}
}apps/vite/src/components/calendar/UpcomingEvent.tsx (4)
1-5: Previous import optimization comment is still applicable.
The existing review comment about optimizing imports for better tree-shaking remains valid.
🧰 Tools
🪛 Biome
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
7-10: LGTM! Props interface is well-defined.
The component props are properly typed and follow TypeScript best practices.
14-16: LGTM! Consistent usage of cn utility.
The className handling has been properly updated to use the new cn utility while maintaining the conditional styling logic.
Also applies to: 25-27
33-33: Verify event.enDate availability.
The component now uses relativeTimeFromDates(new Date(event.enDate)) instead of the previous static daysLeft property. Ensure that event.enDate is always provided to prevent runtime errors.
✅ Verification successful
The previous search revealed the Event interface but didn't show the enDate property. Let's search specifically for enDate usage to understand where it comes from and how it's being used.
Based on the search results, I can now generate the final response:
The enDate property is properly defined and safely used
The Event type in EventList.tsx explicitly defines enDate as a required string property. The property is reliably populated from day.calendarInfo.dates.ad.full.en with a fallback to empty string, ensuring it's always available:
enDate: day.calendarInfo.dates.ad.full.en ?? ""🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Event type definition and its usage to verify enDate property
echo "Searching for Event interface definition:"
ast-grep --pattern 'interface Event {
$$$
}'
echo -e "\nSearching for Event type usage and initialization:"
rg -A 5 'Event.*=.*{'
Length of output: 3471
Script:
#!/bin/bash
# Let's search for enDate property usage and definition
echo "Searching for enDate property usage:"
rg "enDate" -A 2 -B 2
echo -e "\nSearching for Event type initialization with enDate:"
rg -A 5 'const.*event.*:.*Event.*='
Length of output: 4810
apps/vite/src/components/calendar/TodayEventList.tsx (3)
1-5: Use import type for type-only imports.
This issue was previously identified. The EventDetail import should use import type as it's only used for type information.
🧰 Tools
🪛 Biome
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
6-24: LGTM! Well-structured component with proper error handling.
The component implements proper conditional rendering and follows React best practices.
26-53: Consider enhancing event display with additional details.
The previous suggestion about enhancing the event display with additional details remains valid.
apps/vite/tailwind.config.js (6)
11-12: Previous review comment is still applicable.
19-23: Well-structured border radius system!
Good use of CSS variables with logical calculations for different sizes.
24-65: Excellent color system implementation!
Well-structured semantic color system using HSL variables, providing flexibility for both light and dark themes.
68-69: LGTM! Dark mode and animation plugin configuration.
The dark mode configuration is now correct, and the addition of tailwindcss-animate will enhance UI interactions.
13-18: Verify the extra-large container width.
The xl breakpoint is set to 1660px, which is significantly larger than Tailwind's default (1280px). This might cause unexpected layout behavior on common desktop resolutions (1920px and below).
24-65: Verify CSS variable definitions.
Ensure all CSS variables (e.g., --background, --foreground, --chart-1, etc.) are properly defined in your CSS/SCSS files.
apps/vite/src/components/calendar/EventList.tsx (1)
22-35: 🛠️ Refactor suggestion
Add error handling for malformed data.
The data transformation logic should handle potential edge cases where the nested properties might be undefined. Consider adding try-catch blocks and validation.
Consider this safer implementation:
data.forEach((day) => {
+ try {
if (day.eventDetails.length > 0) {
day.eventDetails.forEach((event: EventDetail) => {
+ if (!day.calendarInfo) {
+ console.warn('Missing calendarInfo for day:', day);
+ return;
+ }
newEventDetails.push({
date: day.calendarInfo.dates.bs.day.np ?? "",
enDate: day.calendarInfo.dates.ad.full.en ?? "",
isHoliday: event.isHoliday,
day: day.calendarInfo.days.dayOfWeek.np ?? "",
title: event.title.np ?? "",
fullDate: day.calendarInfo.dates.bs.full.np ?? "",
})
})
}
+ } catch (error) {
+ console.error('Error processing day:', day, error);
+ }
})apps/vite/src/components/calendar/DayDetails.tsx (4)
5-9: LGTM! Well-structured component declaration.
The holiday check logic is cleanly extracted into a variable, improving readability and maintainability.
34-42: Previous review comment about fallback values is still applicable.
The suggestion to add fallback values for optional fields remains relevant for this section.
43-64: Previous review comment about image handling and accessibility is still applicable.
The suggestions regarding local icon imports and correct alt text remain relevant for this section.
65-71: LGTM! Clean component integration.
The Panchang component integration is well-implemented and follows React best practices.
apps/vite/src/components/calendar/Panchang.tsx (3)
Line range hint 5-22: LGTM! Well-structured presentational component.
The component has proper type definitions and a clean implementation.
🧰 Tools
🪛 Biome
[error] 2-2: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
23-27: LGTM! Improved prop flexibility.
The change from bgColor to optional classes prop provides better styling flexibility.
55-61: 🛠️ Refactor suggestion
Consider error handling for invalid dates.
The date formatting could throw if the date string is invalid. Consider adding error handling.
- value={new NepaliDate(data.calendarInfo.dates.bs.full.en ?? "")
- .getDateObject()
- .toLocaleDateString("en-US", {
- month: "long",
- day: "numeric",
- year: "numeric",
- })}
+ value={(() => {
+ try {
+ return new NepaliDate(data.calendarInfo.dates.bs.full.en ?? "")
+ .getDateObject()
+ .toLocaleDateString("en-US", {
+ month: "long",
+ day: "numeric",
+ year: "numeric",
+ });
+ } catch (e) {
+ console.error("Invalid date:", e);
+ return "-";
+ }
+ })()}apps/vite/src/components/YearMonthPicker.tsx (3)
Line range hint 18-37: LGTM! Well-implemented navigation logic.
The month navigation logic is clean and properly handles year boundaries.
84-86: **** The previous review suggestion about keeping the AD month display with a toggle option is still valid.
4-4: Verify the import path configuration.
The @/ path alias needs proper configuration in the TypeScript and build settings.
✅ Verification successful
The tsconfig.json for the Vite app (apps/vite/tsconfig.json) shows that the @ path alias is properly configured:
{
"compilerOptions": {
"baseUrl": ".",
"paths": {
"@/*": ["./src/*"]
}
}
}Let me verify if this is also configured in the Vite build settings:
Path alias @/ is correctly configured
The import path @/lib/utils is properly configured in both TypeScript (tsconfig.json) and Vite (vite.config.ts) settings, resolving to the src directory. The configuration is valid and working as expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the path alias is properly configured
cat $(fd -t f "tsconfig.*json")
Length of output: 3964
Script:
#!/bin/bash
# Check Vite configuration
cat $(fd -t f "vite.config.*")
Length of output: 326
apps/vite/src/pages/Calendar2.tsx (1)
29-32: Review date validation strategy
The current validation has several concerns:
- The year range (2075-2082) is very restrictive and hardcoded
- There's no error feedback when invalid dates are provided
- Silently falling back to current date might confuse users
Let's check if there are any existing constants or configurations for valid date ranges:
Consider:
- Moving date range constraints to configuration
- Adding user feedback for invalid dates
- Implementing proper error boundaries
packages/query/src/calendar/index.ts (2)
110-117: LGTM!
The implementation is clean and consistent with the established patterns in the codebase.
46-51: 🛠️ Refactor suggestion
Optimize fetchCurrentMonth implementation
The current implementation could be simplified by moving the padding logic to the fetchCalendarData function.
const fetchCurrentMonth = async () => {
return fetchCalendarData(
currentNepaliDate.getYear(),
- padNumber(currentNepaliDate.getMonth() + 1, 2)
+ currentNepaliDate.getMonth() + 1
)
}Likely invalid or redundant comment.
apps/vite/src/components/Navbar.tsx (3)
8-8: LGTM! Good use of path alias.
The change to use the @/ path alias instead of relative paths improves maintainability and follows modern React conventions.
93-96: LGTM! Improved UserSettings implementation.
The new implementation is more concise and properly handles the optional photoUrl prop using the optional chaining operator.
24-25: LGTM! Improved navigation structure.
The addition of the border wrapper and container classes provides better visual structure and consistency to the navigation component.
apps/vite/src/components/calendar/CalendarHeader.tsx (1)
16-16: LGTM: Clean implementation of state management and layout
The state management, props usage, and integration with YearMonthPicker component are well implemented. The responsive grid layout provides a good user experience across different screen sizes.
Also applies to: 99-135
| export const QueryProvider = memo( | ||
| ({ children, client = queryClient }: QueryProviderExtendedProps) => { | ||
| return <QueryClientProvider client={client}>{children}</QueryClientProvider> | ||
| } | ||
| ) |
There was a problem hiding this comment.
💡 Codebase verification
Memoization is unnecessary for this root-level provider component
The search results confirm that QueryProvider is used at the root level of the application (in App.tsx), mounted only once, and wraps the entire application. In this usage pattern, React's reconciliation process would never trigger re-renders of this component unless its props change, making the memo wrapper redundant and unnecessary overhead.
- The component is used in
apps/vite/src/App.tsxas a root-level provider - It only receives a stable
childrenprop and an optionalclientprop with a default value - Being at the root level, it's not subject to parent re-renders that would benefit from memoization
🔗 Analysis chain
Verify the benefit of memoization.
The implementation looks clean and provides good flexibility with the custom client option. However, since QueryProvider is typically mounted once at the app root, we should verify if memoization provides actual performance benefits in this case.
Let's check the component's usage pattern:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check QueryProvider usage patterns to assess memoization benefit
# Expected: QueryProvider should appear near the root of component trees
# Search for QueryProvider usage
rg -l "QueryProvider" | xargs rg -A 5 -B 5 "QueryProvider"
Length of output: 2225
| useEffect(() => { | ||
| const mediaQueryList = window.matchMedia(query) | ||
| setMatches(mediaQueryList.matches) | ||
| const handleChange = (event: MediaQueryListEvent) => { | ||
| setMatches(event.matches) | ||
| } | ||
| mediaQueryList.addEventListener("change", handleChange) | ||
|
|
||
| return () => { | ||
| mediaQueryList.removeEventListener("change", handleChange) | ||
| } | ||
| }, [query]) |
There was a problem hiding this comment.
Add error handling and browser compatibility checks.
The effect should handle potential errors and check for browser compatibility.
useEffect(() => {
+ try {
+ if (typeof window === 'undefined' || !window.matchMedia) {
+ console.warn('matchMedia is not supported in this environment');
+ return;
+ }
+
const mediaQueryList = window.matchMedia(query)
setMatches(mediaQueryList.matches)
const handleChange = (event: MediaQueryListEvent) => {
setMatches(event.matches)
}
mediaQueryList.addEventListener("change", handleChange)
return () => {
mediaQueryList.removeEventListener("change", handleChange)
}
+ } catch (error) {
+ console.error('Error setting up media query listener:', error);
+ return () => {};
+ }
}, [query])📝 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.
| useEffect(() => { | |
| const mediaQueryList = window.matchMedia(query) | |
| setMatches(mediaQueryList.matches) | |
| const handleChange = (event: MediaQueryListEvent) => { | |
| setMatches(event.matches) | |
| } | |
| mediaQueryList.addEventListener("change", handleChange) | |
| return () => { | |
| mediaQueryList.removeEventListener("change", handleChange) | |
| } | |
| }, [query]) | |
| useEffect(() => { | |
| try { | |
| if (typeof window === 'undefined' || !window.matchMedia) { | |
| console.warn('matchMedia is not supported in this environment'); | |
| return; | |
| } | |
| const mediaQueryList = window.matchMedia(query) | |
| setMatches(mediaQueryList.matches) | |
| const handleChange = (event: MediaQueryListEvent) => { | |
| setMatches(event.matches) | |
| } | |
| mediaQueryList.addEventListener("change", handleChange) | |
| return () => { | |
| mediaQueryList.removeEventListener("change", handleChange) | |
| } | |
| } catch (error) { | |
| console.error('Error setting up media query listener:', error); | |
| return () => {}; | |
| } | |
| }, [query]) |
| const setOnline = () => useCallback(() => setStatus(true), []) | ||
| const setOffline = () => useCallback(() => setStatus(false), []) |
There was a problem hiding this comment.
Fix incorrect useCallback implementation
The current implementation creates a new function that returns a callback on every render, defeating the purpose of memoization. The useCallback hook should wrap the function directly.
-const setOnline = () => useCallback(() => setStatus(true), [])
-const setOffline = () => useCallback(() => setStatus(false), [])
+const setOnline = useCallback(() => setStatus(true), [])
+const setOffline = useCallback(() => setStatus(false), [])📝 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.
| const setOnline = () => useCallback(() => setStatus(true), []) | |
| const setOffline = () => useCallback(() => setStatus(false), []) | |
| const setOnline = useCallback(() => setStatus(true), []) | |
| const setOffline = useCallback(() => setStatus(false), []) |
|
|
||
| const getOnLineStatus = () => | ||
| typeof navigator !== "undefined" && typeof navigator.onLine === "boolean" | ||
| ? navigator.onLine | ||
| : true | ||
|
|
||
| const useNavigatorOnLine = () => { | ||
| const useNavigatorOnLine = (): Boolean => { |
There was a problem hiding this comment.
Use lowercase 'boolean' type instead of 'Boolean'
The Boolean type refers to the object wrapper, while boolean is the primitive type. In TypeScript, it's recommended to use the primitive type for consistency and better type safety.
-const useNavigatorOnLine = (): Boolean => {
+const useNavigatorOnLine = (): boolean => {📝 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.
| const useNavigatorOnLine = (): Boolean => { | |
| const useNavigatorOnLine = (): boolean => { |
🧰 Tools
🪛 Biome
[error] 8-8: Don't use 'Boolean' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'boolean' instead
(lint/complexity/noBannedTypes)
| await fetch( | ||
| `${apiBaseUrl}/calendar/google/delete/${encodeURIComponent(event.id)}`, | ||
| { | ||
| method: "DELETE", | ||
| credentials: "include", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| } | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling.
The function should handle potential network failures and invalid responses. Consider adding try-catch and returning the response status.
export const deleteEvent = async (apiBaseUrl: string, event: CalendarEvent) => {
+ if (!event?.id) {
+ throw new Error("Invalid event: Missing event ID");
+ }
+ try {
await fetch(
`${apiBaseUrl}/calendar/google/delete/${encodeURIComponent(event.id)}`,
{
method: "DELETE",
credentials: "include",
headers: {
"Content-Type": "application/json",
},
}
)
+ return true;
+ } catch (error) {
+ console.error("Failed to delete event:", error);
+ throw error;
+ }
}📝 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.
| await fetch( | |
| `${apiBaseUrl}/calendar/google/delete/${encodeURIComponent(event.id)}`, | |
| { | |
| method: "DELETE", | |
| credentials: "include", | |
| headers: { | |
| "Content-Type": "application/json", | |
| }, | |
| } | |
| ) | |
| if (!event?.id) { | |
| throw new Error("Invalid event: Missing event ID"); | |
| } | |
| try { | |
| await fetch( | |
| `${apiBaseUrl}/calendar/google/delete/${encodeURIComponent(event.id)}`, | |
| { | |
| method: "DELETE", | |
| credentials: "include", | |
| headers: { | |
| "Content-Type": "application/json", | |
| }, | |
| } | |
| ) | |
| return true; | |
| } catch (error) { | |
| console.error("Failed to delete event:", error); | |
| throw error; | |
| } |
| ? "bg-gray-300 dark:bg-gray-600 dark:text-gray-900" | ||
| : "text-gray-900 dark:text-white", |
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent dark mode text contrast confirmed
The verification reveals an inconsistency in dark mode text contrast:
- In
MonthCalendar.tsx,dark:bg-gray-600is correctly paired withdark:text-gray-100for good contrast - In
Navbar.tsx, the same backgrounddark:bg-gray-600is paired withdark:text-gray-900, which would indeed create poor contrast - This inconsistency should be fixed by using light text color (
dark:text-gray-100ordark:text-white) with the dark background
🔗 Analysis chain
Review dark mode text contrast.
In the mobile navigation, when an item is active (bg-gray-600), it uses text-gray-900 which might have poor contrast in dark mode. Consider maintaining the light text color for better readability.
Let's check if this contrast issue exists elsewhere in the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar dark mode contrast patterns
rg "dark:bg-gray-600.*dark:text-gray-900" --type tsx
Length of output: 82
Script:
#!/bin/bash
# Search for similar dark mode contrast patterns with correct file type
rg "dark:bg-gray-600.*dark:text-gray-900"
# Also search for the specific background and text color combinations separately
rg "dark:bg-gray-600.+dark:text"
# Let's also check what other text colors are used with this background
rg "dark:bg-gray-600"
Length of output: 955
| <button className="py-2 px-5 rounded-lg bg-gray-100 text-xs font-medium text-gray-900 transition-all duration-300 hover:bg-white"> | ||
| Month | ||
| </button> | ||
| </div> | ||
| </div> | ||
| <div className="flex items-center gap-2 flex-1 justify-center"> | ||
| <button className="text-gray-500 rounded transition-all duration-300 hover: hover:text-gray-900"> | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width={16} | ||
| height={16} | ||
| viewBox="0 0 16 16" | ||
| fill="none" | ||
| > | ||
| <path | ||
| d="M10.0002 11.9999L6 7.99971L10.0025 3.99719" | ||
| stroke="currentcolor" | ||
| strokeWidth="1.3" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| /> | ||
| </svg> | ||
| </button> | ||
| <h5 className="text-xl leading-8 font-semibold text-gray-900"> | ||
| {month} {year} | ||
| </h5> | ||
| <button className="text-gray-500 rounded transition-all duration-300 hover: hover:text-gray-900"> | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width={16} | ||
| height={16} | ||
| viewBox="0 0 16 16" | ||
| fill="none" | ||
| > | ||
| <path | ||
| d="M6.00236 3.99707L10.0025 7.99723L6 11.9998" | ||
| stroke="currentcolor" | ||
| strokeWidth="1.3" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| /> | ||
| </svg> | ||
| </button> | ||
| </div> | ||
| <div className="flex items-center justify-end gap-2 flex-1"> | ||
| {view === 'event' && ( | ||
| <div className="flex items-center gap-px p-1 rounded-md bg-gray-100"> | ||
| <button className="py-2 px-5 rounded-lg bg-gray-100 text-xs font-medium text-gray-900 transition-all duration-300 hover:bg-white"> | ||
| Day | ||
| </button> | ||
| <button className="py-2 px-5 rounded-lg bg-white text-xs font-medium text-gray-900 transition-all duration-300 hover:bg-white"> | ||
| Week | ||
| </button> | ||
| <button className="py-2 px-5 rounded-lg bg-gray-100 text-xs font-medium text-gray-900 transition-all duration-300 hover:bg-white"> | ||
| Month | ||
| </button> | ||
| </div> | ||
| )} | ||
| <button className="hidden sm:flex p-3 text-gray-500 items-center justify-center transition-all duration-300 hover:text-gray-900"> | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width={20} | ||
| height={20} | ||
| viewBox="0 0 20 20" | ||
| fill="none" | ||
| > | ||
| <path | ||
| d="M16.7501 9.20285C17.4658 8.79486 17.7119 7.89657 17.3012 7.19137L16.559 5.91699C16.1385 5.19489 15.1995 4.94638 14.466 5.36301C13.4469 5.94185 12.1716 5.19438 12.1716 4.03885C12.1716 3.19732 11.4764 2.5 10.6188 2.5H9.17632C8.33949 2.5 7.66111 3.16571 7.66111 3.9869V4.09222C7.66111 5.19157 6.44713 5.8776 5.4782 5.3258C4.78224 4.92946 3.89057 5.16345 3.48911 5.84779L2.70103 7.19113C2.28763 7.89582 2.5327 8.79589 3.24919 9.20432C4.24554 9.77228 4.24337 11.1862 3.24904 11.7576C2.53341 12.1688 2.28715 13.0712 2.7009 13.7764L3.48911 15.12C3.89057 15.8043 4.78478 16.0369 5.48074 15.6406C6.4466 15.0905 7.66111 15.7719 7.66111 16.8677C7.66111 17.6529 8.30976 18.2895 9.10991 18.2895H10.6853C11.5061 18.2895 12.1716 17.6365 12.1716 16.831C12.1716 15.7075 13.4115 15.0058 14.4024 15.5686L14.466 15.6048C15.1995 16.0214 16.1385 15.7729 16.559 15.0508L17.3013 13.7762C17.7124 13.0704 17.4651 12.1699 16.7502 11.7591C15.7547 11.1871 15.7526 9.77146 16.7501 9.20285Z" | ||
| stroke="currentcolor" | ||
| strokeWidth="1.3" | ||
| /> | ||
| <path | ||
| d="M12.68 10.3947C12.68 11.8481 11.4794 13.0263 9.99834 13.0263C8.5173 13.0263 7.31668 11.8481 7.31668 10.3947C7.31668 8.94136 8.5173 7.76316 9.99834 7.76316C11.4794 7.76316 12.68 8.94136 12.68 10.3947Z" | ||
| stroke="currentcolor" | ||
| strokeWidth="1.3" | ||
| /> | ||
| </svg> | ||
| </button> | ||
| </div> | ||
| )} | ||
| <button className="hidden sm:flex p-3 text-gray-500 items-center justify-center transition-all duration-300 hover:text-gray-900"> | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width={20} | ||
| height={20} | ||
| viewBox="0 0 20 20" | ||
| fill="none" | ||
| > | ||
| <path | ||
| d="M16.7501 9.20285C17.4658 8.79486 17.7119 7.89657 17.3012 7.19137L16.559 5.91699C16.1385 5.19489 15.1995 4.94638 14.466 5.36301C13.4469 5.94185 12.1716 5.19438 12.1716 4.03885C12.1716 3.19732 11.4764 2.5 10.6188 2.5H9.17632C8.33949 2.5 7.66111 3.16571 7.66111 3.9869V4.09222C7.66111 5.19157 6.44713 5.8776 5.4782 5.3258C4.78224 4.92946 3.89057 5.16345 3.48911 5.84779L2.70103 7.19113C2.28763 7.89582 2.5327 8.79589 3.24919 9.20432C4.24554 9.77228 4.24337 11.1862 3.24904 11.7576C2.53341 12.1688 2.28715 13.0712 2.7009 13.7764L3.48911 15.12C3.89057 15.8043 4.78478 16.0369 5.48074 15.6406C6.4466 15.0905 7.66111 15.7719 7.66111 16.8677C7.66111 17.6529 8.30976 18.2895 9.10991 18.2895H10.6853C11.5061 18.2895 12.1716 17.6365 12.1716 16.831C12.1716 15.7075 13.4115 15.0058 14.4024 15.5686L14.466 15.6048C15.1995 16.0214 16.1385 15.7729 16.559 15.0508L17.3013 13.7762C17.7124 13.0704 17.4651 12.1699 16.7502 11.7591C15.7547 11.1871 15.7526 9.77146 16.7501 9.20285Z" | ||
| stroke="currentcolor" | ||
| strokeWidth="1.3" | ||
| /> | ||
| <path | ||
| d="M12.68 10.3947C12.68 11.8481 11.4794 13.0263 9.99834 13.0263C8.5173 13.0263 7.31668 11.8481 7.31668 10.3947C7.31668 8.94136 8.5173 7.76316 9.99834 7.76316C11.4794 7.76316 12.68 8.94136 12.68 10.3947Z" | ||
| stroke="currentcolor" | ||
| strokeWidth="1.3" | ||
| /> | ||
| </svg> | ||
| </button> |
There was a problem hiding this comment.
Add missing button types for accessibility
All buttons are missing the type="button" attribute which is required for accessibility and to prevent unintended form submissions.
Add type="button" to all buttons in the component. Here's a sample for the first few buttons:
<button
+ type="button"
className={cn(
"p-2 rounded-md bg-gray-200 border-2 border-gray-200 inline-flex",
view === "calendar" ? "bg-white" : ""
)}
onClick={() => setView("calendar")}
>Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 Biome
[error] 152-158: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
[error] 102-108: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 111-117: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 122-125: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 140-140: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 143-143: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 146-146: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 151-151: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
| import React, { useState } from "react" | ||
| import { cn } from "@/lib/utils" | ||
| import EventIcon from "../icons/EventIcon" | ||
| import ListIcon from "../icons/ListIcon" | ||
| import YearMonthPicker from "../YearMonthPicker" | ||
| import NepaliDate from "nepali-datetime" | ||
|
|
||
| const CalendarHeader: React.FC<{ month: string; year: string }> = ({ month, year }) => { | ||
| const [view, setView] = useState<'calendar' | 'event'>('calendar') | ||
| type CalendarHeaderProps = { | ||
| currentNepaliDate: NepaliDate | ||
| setCurrentNepaliDate: (date: NepaliDate) => void | ||
| } |
There was a problem hiding this comment.
Optimize imports by using import type
The React import is only used for type definitions. Let's optimize the module loading.
Apply this diff:
-import React, { useState } from "react"
+import { useState } from "react"
+import type { FC } from "react"
type CalendarHeaderProps = {
currentNepaliDate: NepaliDate
setCurrentNepaliDate: (date: NepaliDate) => void
}
-const CalendarHeader: React.FC<CalendarHeaderProps> = ({
+const CalendarHeader: FC<CalendarHeaderProps> = ({📝 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 React, { useState } from "react" | |
| import { cn } from "@/lib/utils" | |
| import EventIcon from "../icons/EventIcon" | |
| import ListIcon from "../icons/ListIcon" | |
| import YearMonthPicker from "../YearMonthPicker" | |
| import NepaliDate from "nepali-datetime" | |
| const CalendarHeader: React.FC<{ month: string; year: string }> = ({ month, year }) => { | |
| const [view, setView] = useState<'calendar' | 'event'>('calendar') | |
| type CalendarHeaderProps = { | |
| currentNepaliDate: NepaliDate | |
| setCurrentNepaliDate: (date: NepaliDate) => void | |
| } | |
| import { useState } from "react" | |
| import type { FC } from "react" | |
| import { cn } from "@/lib/utils" | |
| import EventIcon from "../icons/EventIcon" | |
| import ListIcon from "../icons/ListIcon" | |
| import YearMonthPicker from "../YearMonthPicker" | |
| import NepaliDate from "nepali-datetime" | |
| type CalendarHeaderProps = { | |
| currentNepaliDate: NepaliDate | |
| setCurrentNepaliDate: (date: NepaliDate) => void | |
| } |
🧰 Tools
🪛 Biome
[error] 1-1: The default import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
| {view === "event" && ( | ||
| <div className="flex items-center gap-px p-1 rounded-md bg-gray-100"> | ||
| <button className="py-2 px-5 rounded-lg bg-gray-100 text-xs font-medium text-gray-900 transition-all duration-300 hover:bg-white"> | ||
| Day | ||
| </button> | ||
| <button className="py-2 px-5 rounded-lg bg-white text-xs font-medium text-gray-900 transition-all duration-300 hover:bg-white"> | ||
| Week | ||
| </button> | ||
| <button className="py-2 px-5 rounded-lg bg-gray-100 text-xs font-medium text-gray-900 transition-all duration-300 hover:bg-white"> | ||
| Month | ||
| </button> | ||
| </div> | ||
| </div> | ||
| <div className="flex items-center gap-2 flex-1 justify-center"> | ||
| <button className="text-gray-500 rounded transition-all duration-300 hover: hover:text-gray-900"> | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width={16} | ||
| height={16} | ||
| viewBox="0 0 16 16" | ||
| fill="none" | ||
| > | ||
| <path | ||
| d="M10.0002 11.9999L6 7.99971L10.0025 3.99719" | ||
| stroke="currentcolor" | ||
| strokeWidth="1.3" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| /> | ||
| </svg> | ||
| </button> | ||
| <h5 className="text-xl leading-8 font-semibold text-gray-900"> | ||
| {month} {year} | ||
| </h5> | ||
| <button className="text-gray-500 rounded transition-all duration-300 hover: hover:text-gray-900"> | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width={16} | ||
| height={16} | ||
| viewBox="0 0 16 16" | ||
| fill="none" | ||
| > | ||
| <path | ||
| d="M6.00236 3.99707L10.0025 7.99723L6 11.9998" | ||
| stroke="currentcolor" | ||
| strokeWidth="1.3" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| /> | ||
| </svg> | ||
| </button> | ||
| </div> | ||
| <div className="flex items-center justify-end gap-2 flex-1"> | ||
| {view === 'event' && ( | ||
| <div className="flex items-center gap-px p-1 rounded-md bg-gray-100"> | ||
| <button className="py-2 px-5 rounded-lg bg-gray-100 text-xs font-medium text-gray-900 transition-all duration-300 hover:bg-white"> | ||
| Day | ||
| </button> | ||
| <button className="py-2 px-5 rounded-lg bg-white text-xs font-medium text-gray-900 transition-all duration-300 hover:bg-white"> | ||
| Week | ||
| </button> | ||
| <button className="py-2 px-5 rounded-lg bg-gray-100 text-xs font-medium text-gray-900 transition-all duration-300 hover:bg-white"> | ||
| Month | ||
| </button> | ||
| </div> | ||
| )} | ||
| <button className="hidden sm:flex p-3 text-gray-500 items-center justify-center transition-all duration-300 hover:text-gray-900"> | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width={20} | ||
| height={20} | ||
| viewBox="0 0 20 20" | ||
| fill="none" | ||
| > | ||
| <path | ||
| d="M16.7501 9.20285C17.4658 8.79486 17.7119 7.89657 17.3012 7.19137L16.559 5.91699C16.1385 5.19489 15.1995 4.94638 14.466 5.36301C13.4469 5.94185 12.1716 5.19438 12.1716 4.03885C12.1716 3.19732 11.4764 2.5 10.6188 2.5H9.17632C8.33949 2.5 7.66111 3.16571 7.66111 3.9869V4.09222C7.66111 5.19157 6.44713 5.8776 5.4782 5.3258C4.78224 4.92946 3.89057 5.16345 3.48911 5.84779L2.70103 7.19113C2.28763 7.89582 2.5327 8.79589 3.24919 9.20432C4.24554 9.77228 4.24337 11.1862 3.24904 11.7576C2.53341 12.1688 2.28715 13.0712 2.7009 13.7764L3.48911 15.12C3.89057 15.8043 4.78478 16.0369 5.48074 15.6406C6.4466 15.0905 7.66111 15.7719 7.66111 16.8677C7.66111 17.6529 8.30976 18.2895 9.10991 18.2895H10.6853C11.5061 18.2895 12.1716 17.6365 12.1716 16.831C12.1716 15.7075 13.4115 15.0058 14.4024 15.5686L14.466 15.6048C15.1995 16.0214 16.1385 15.7729 16.559 15.0508L17.3013 13.7762C17.7124 13.0704 17.4651 12.1699 16.7502 11.7591C15.7547 11.1871 15.7526 9.77146 16.7501 9.20285Z" | ||
| stroke="currentcolor" | ||
| strokeWidth="1.3" | ||
| /> | ||
| <path | ||
| d="M12.68 10.3947C12.68 11.8481 11.4794 13.0263 9.99834 13.0263C8.5173 13.0263 7.31668 11.8481 7.31668 10.3947C7.31668 8.94136 8.5173 7.76316 9.99834 7.76316C11.4794 7.76316 12.68 8.94136 12.68 10.3947Z" | ||
| stroke="currentcolor" | ||
| strokeWidth="1.3" | ||
| /> | ||
| </svg> | ||
| </button> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
💡 Codebase verification
Day/Week/Month view buttons need implementation
The buttons are currently non-functional as they lack:
- onClick handlers
- State management for the selected view
- Corresponding view components/logic
Key findings from the codebase:
- CalendarHeader only manages "calendar" vs "event" view state
- CalendarGrid only implements a month-view layout
- No implementation exists for Day or Week views
🔗 Analysis chain
Implement event view button functionality
The Day/Week/Month buttons in the event view appear to be non-functional placeholders.
Would you like help implementing the view switching functionality for these buttons?
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if these view modes are implemented elsewhere
rg -l "Day|Week|Month.*view" --type ts --type tsx
Length of output: 79
Script:
#!/bin/bash
# Check if these view modes are implemented elsewhere, fixing the file type
rg -l "Day|Week|Month.*view" -t ts -t tsx
# Look for any calendar-related view switching logic
rg -t ts -t tsx "calendar.*view|view.*calendar" -A 3
# Search for any onClick handlers or state management related to view switching
ast-grep --pattern 'onClick={$_}'
Length of output: 190
Script:
#!/bin/bash
# Search for calendar-related files first
fd -e ts -e tsx calendar
# Then look for view-related logic in these files
rg "view|Day|Week|Month" --type=typescript
# Look for state management or props related to view
rg "interface|type.*Props|useState.*view" --type=typescript
Length of output: 650
Script:
#!/bin/bash
# Look at the content of relevant calendar files
cat "apps/vite/src/components/calendar/CalendarHeader.tsx"
cat "apps/vite/src/components/calendar/CalendarGrid.tsx"
cat "apps/vite/src/pages/Calendar2.tsx"
Length of output: 18030
🧰 Tools
🪛 Biome
[error] 140-140: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 143-143: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 146-146: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width={20} | ||
| height={20} | ||
| viewBox="0 0 20 20" | ||
| fill="none" | ||
| > | ||
| <path | ||
| d="M16.7501 9.20285C17.4658 8.79486 17.7119 7.89657 17.3012 7.19137L16.559 5.91699C16.1385 5.19489 15.1995 4.94638 14.466 5.36301C13.4469 5.94185 12.1716 5.19438 12.1716 4.03885C12.1716 3.19732 11.4764 2.5 10.6188 2.5H9.17632C8.33949 2.5 7.66111 3.16571 7.66111 3.9869V4.09222C7.66111 5.19157 6.44713 5.8776 5.4782 5.3258C4.78224 4.92946 3.89057 5.16345 3.48911 5.84779L2.70103 7.19113C2.28763 7.89582 2.5327 8.79589 3.24919 9.20432C4.24554 9.77228 4.24337 11.1862 3.24904 11.7576C2.53341 12.1688 2.28715 13.0712 2.7009 13.7764L3.48911 15.12C3.89057 15.8043 4.78478 16.0369 5.48074 15.6406C6.4466 15.0905 7.66111 15.7719 7.66111 16.8677C7.66111 17.6529 8.30976 18.2895 9.10991 18.2895H10.6853C11.5061 18.2895 12.1716 17.6365 12.1716 16.831C12.1716 15.7075 13.4115 15.0058 14.4024 15.5686L14.466 15.6048C15.1995 16.0214 16.1385 15.7729 16.559 15.0508L17.3013 13.7762C17.7124 13.0704 17.4651 12.1699 16.7502 11.7591C15.7547 11.1871 15.7526 9.77146 16.7501 9.20285Z" | ||
| stroke="currentcolor" | ||
| strokeWidth="1.3" | ||
| /> | ||
| <path | ||
| d="M12.68 10.3947C12.68 11.8481 11.4794 13.0263 9.99834 13.0263C8.5173 13.0263 7.31668 11.8481 7.31668 10.3947C7.31668 8.94136 8.5173 7.76316 9.99834 7.76316C11.4794 7.76316 12.68 8.94136 12.68 10.3947Z" | ||
| stroke="currentcolor" | ||
| strokeWidth="1.3" | ||
| /> | ||
| </svg> |
There was a problem hiding this comment.
Add accessibility attributes to SVG
The settings icon SVG lacks accessibility attributes, making it difficult for screen readers to interpret.
Add appropriate ARIA labels:
<svg
xmlns="http://www.w3.org/2000/svg"
width={20}
height={20}
viewBox="0 0 20 20"
fill="none"
+ role="img"
+ aria-label="Settings"
>📝 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.
| <svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| width={20} | |
| height={20} | |
| viewBox="0 0 20 20" | |
| fill="none" | |
| > | |
| <path | |
| d="M16.7501 9.20285C17.4658 8.79486 17.7119 7.89657 17.3012 7.19137L16.559 5.91699C16.1385 5.19489 15.1995 4.94638 14.466 5.36301C13.4469 5.94185 12.1716 5.19438 12.1716 4.03885C12.1716 3.19732 11.4764 2.5 10.6188 2.5H9.17632C8.33949 2.5 7.66111 3.16571 7.66111 3.9869V4.09222C7.66111 5.19157 6.44713 5.8776 5.4782 5.3258C4.78224 4.92946 3.89057 5.16345 3.48911 5.84779L2.70103 7.19113C2.28763 7.89582 2.5327 8.79589 3.24919 9.20432C4.24554 9.77228 4.24337 11.1862 3.24904 11.7576C2.53341 12.1688 2.28715 13.0712 2.7009 13.7764L3.48911 15.12C3.89057 15.8043 4.78478 16.0369 5.48074 15.6406C6.4466 15.0905 7.66111 15.7719 7.66111 16.8677C7.66111 17.6529 8.30976 18.2895 9.10991 18.2895H10.6853C11.5061 18.2895 12.1716 17.6365 12.1716 16.831C12.1716 15.7075 13.4115 15.0058 14.4024 15.5686L14.466 15.6048C15.1995 16.0214 16.1385 15.7729 16.559 15.0508L17.3013 13.7762C17.7124 13.0704 17.4651 12.1699 16.7502 11.7591C15.7547 11.1871 15.7526 9.77146 16.7501 9.20285Z" | |
| stroke="currentcolor" | |
| strokeWidth="1.3" | |
| /> | |
| <path | |
| d="M12.68 10.3947C12.68 11.8481 11.4794 13.0263 9.99834 13.0263C8.5173 13.0263 7.31668 11.8481 7.31668 10.3947C7.31668 8.94136 8.5173 7.76316 9.99834 7.76316C11.4794 7.76316 12.68 8.94136 12.68 10.3947Z" | |
| stroke="currentcolor" | |
| strokeWidth="1.3" | |
| /> | |
| </svg> | |
| <svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| width={20} | |
| height={20} | |
| viewBox="0 0 20 20" | |
| fill="none" | |
| role="img" | |
| aria-label="Settings" | |
| > | |
| <path | |
| d="M16.7501 9.20285C17.4658 8.79486 17.7119 7.89657 17.3012 7.19137L16.559 5.91699C16.1385 5.19489 15.1995 4.94638 14.466 5.36301C13.4469 5.94185 12.1716 5.19438 12.1716 4.03885C12.1716 3.19732 11.4764 2.5 10.6188 2.5H9.17632C8.33949 2.5 7.66111 3.16571 7.66111 3.9869V4.09222C7.66111 5.19157 6.44713 5.8776 5.4782 5.3258C4.78224 4.92946 3.89057 5.16345 3.48911 5.84779L2.70103 7.19113C2.28763 7.89582 2.5327 8.79589 3.24919 9.20432C4.24554 9.77228 4.24337 11.1862 3.24904 11.7576C2.53341 12.1688 2.28715 13.0712 2.7009 13.7764L3.48911 15.12C3.89057 15.8043 4.78478 16.0369 5.48074 15.6406C6.4466 15.0905 7.66111 15.7719 7.66111 16.8677C7.66111 17.6529 8.30976 18.2895 9.10991 18.2895H10.6853C11.5061 18.2895 12.1716 17.6365 12.1716 16.831C12.1716 15.7075 13.4115 15.0058 14.4024 15.5686L14.466 15.6048C15.1995 16.0214 16.1385 15.7729 16.559 15.0508L17.3013 13.7762C17.7124 13.0704 17.4651 12.1699 16.7502 11.7591C15.7547 11.1871 15.7526 9.77146 16.7501 9.20285Z" | |
| stroke="currentcolor" | |
| strokeWidth="1.3" | |
| /> | |
| <path | |
| d="M12.68 10.3947C12.68 11.8481 11.4794 13.0263 9.99834 13.0263C8.5173 13.0263 7.31668 11.8481 7.31668 10.3947C7.31668 8.94136 8.5173 7.76316 9.99834 7.76316C11.4794 7.76316 12.68 8.94136 12.68 10.3947Z" | |
| stroke="currentcolor" | |
| strokeWidth="1.3" | |
| /> | |
| </svg> |
🧰 Tools
🪛 Biome
[error] 152-158: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Outside diff range and nitpick comments (14)
apps/vite/vite.config.js (1)
5-12: Configuration looks good, consider additional optimizations.The basic Vite configuration is well-structured with React plugin and path aliasing. However, you might want to consider adding these commonly used options for better development experience:
server.portfor consistent development portbuild.sourcemapfor production debuggingbuild.chunkSizeWarningLimitfor bundle size controlHere's an enhanced configuration you might consider:
export default defineConfig({ plugins: [react()], resolve: { alias: { "@": path.resolve(__dirname, "./src"), }, }, + server: { + port: 3000, + }, + build: { + sourcemap: true, + chunkSizeWarningLimit: 1000, + }, });apps/vite/src/components/SplitButton.tsx (1)
1-2: Consider usingimport typefor type-only imports.While
Reactimport is needed for JSX, you could optimize the type imports in the props interface.import React from "react" +import type { ReactNode } from "react" import { cn } from "@/lib/utils" const SplitButton = ({ buttons, selectedView, setView, }: { - buttons: { id: string; children: React.ReactNode }[] + buttons: { id: string; children: ReactNode }[] selectedView: string setView: (view: string) => void }) => {🧰 Tools
🪛 Biome
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
apps/vite/src/components/calendar/DayDialog.tsx (4)
1-12: Optimize imports for better tree-shakingSince React is only used for type definitions, we can optimize the bundle size by using
import type.-import * as React from "react" +import type * as React from "react"🧰 Tools
🪛 Biome
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
38-48: Remove unnecessary DrawerFooterThe DrawerFooter appears to be empty and only adds padding. Consider removing it if it serves no purpose.
{children} - <DrawerFooter className="pt-2"></DrawerFooter>🧰 Tools
🪛 Biome
[error] 45-47: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
45-45: Use self-closing tag for empty elementFor better code style and consistency, use self-closing syntax for elements without children.
- <DrawerFooter className="pt-2"></DrawerFooter> + <DrawerFooter className="pt-2" />🧰 Tools
🪛 Biome
[error] 45-47: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
14-49: Consider memoizing the component for better performanceSince the component relies on props that might frequently change (
openstate), consider wrapping it withReact.memoto prevent unnecessary re-renders.-export function DayDialog({ +export const DayDialog = React.memo(function DayDialog({ open, setOpen, children, }: { open: boolean setOpen: (open: boolean) => void children: React.ReactNode -}) { +}) { // ... component implementation ... -} +})🧰 Tools
🪛 Biome
[error] 45-47: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
apps/vite/src/components/Spinner.tsx (1)
5-8: Consider using semantic HTML elements.Instead of using a
divwithrole="status", consider using the HTML5<status>element for better semantics while maintaining the same accessibility benefits.- <div role="status"> + <status>🧰 Tools
🪛 Biome
[error] 6-6: The elements with the following roles can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
apps/vite/package.json (1)
17-18: Good choice of UI component librariesThe addition of Radix UI (for accessible components), styling utilities (class-variance-authority, clsx, tailwind-merge), and Lucide icons creates a solid foundation for building accessible and maintainable UI components.
Consider documenting the styling conventions and component usage patterns in a shared UI guide to maintain consistency across the team.
Also applies to: 20-21, 27-27, 34-36
apps/vite/src/components/ui/drawer.tsx (1)
58-78: Consider making spacing configurable.The components have hardcoded spacing values (
my-4,p-4). Consider making these configurable through props for better flexibility.const DrawerHeader = ({ className, + spacing = "my-4", ...props }: React.HTMLAttributes<HTMLDivElement> & { + spacing?: string; }) => ( <div - className={cn("grid gap-1.5 my-4 text-center sm:text-left", className)} + className={cn(`grid gap-1.5 ${spacing} text-center sm:text-left`, className)} {...props} /> )apps/vite/src/pages/Calendar2.tsx (2)
96-101: Optimize EventList rendering and data handlingThe two EventList components receive the same
combinedDataprop but filter it differently. This could be optimized to avoid duplicate data processing.Consider extracting the filtered data:
+const eventData = useMemo(() => { + return combinedData.filter(item => !item.isHoliday) +}, [combinedData]) + +const holidayData = useMemo(() => { + return combinedData.filter(item => item.isHoliday) +}, [combinedData]) -<EventList data={combinedData} title="आगामी इभेन्टहरु" /> -<EventList - data={combinedData} - title="आगामी बिदाहरु" - isHoliday={true} -/> +<EventList data={eventData} title="आगामी इभेन्टहरु" /> +<EventList data={holidayData} title="आगामी बिदाहरु" />
83-93: Simplify view toggle logicThe view toggle logic could be simplified using object literal lookup instead of conditional rendering.
Consider this improvement:
-{view === "calendar" ? ( - <CalendarGrid monthData={monthData} /> -) : ( - <TimelineView monthData={monthData} /> -)} +{ + { + calendar: <CalendarGrid monthData={monthData} />, + event: <TimelineView monthData={monthData} /> + }[view] +}apps/vite/src/components/calendar/CalendarHeader.tsx (2)
1-13: Optimize imports by usingimport typeThe React import is only used for type definitions. Let's optimize the module loading.
-import React, { useState } from "react" +import { useState } from "react" +import type { FC } from "react" type CalendarHeaderProps = { currentNepaliDate: NepaliDate setCurrentNepaliDate: (date: NepaliDate) => void view: "calendar" | "event" setView: (view: "calendar" | "event") => void }🧰 Tools
🪛 Biome
[error] 1-1: The default import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
141-153: Implement view switching functionality for Day/Week/Month buttonsThe Day/Week/Month view buttons in the event view appear to be non-functional placeholders.
Would you like help implementing:
- State management for the view type
- Corresponding view components
- Click handlers for the view switching buttons
🧰 Tools
🪛 Biome
[error] 143-143: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
[error] 146-146: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
[error] 149-149: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
apps/vite/src/components/ui/sheet.tsx (1)
66-69: Verify accessibility of the close buttonThe close button includes an icon and a visually hidden text for screen readers, which is good for accessibility. Ensure that all interactive elements have appropriate accessibility attributes. You might consider adding an
aria-labelto theSheetPrimitive.Closecomponent for clarity.Apply this refinement:
<SheetPrimitive.Close className="absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-secondary" + aria-label="Close" > <Cross2Icon className="h-4 w-4" /> <span className="sr-only">Close</span> </SheetPrimitive.Close>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (14)
apps/vite/package.json(1 hunks)apps/vite/src/components/Spinner.tsx(2 hunks)apps/vite/src/components/SplitButton.tsx(2 hunks)apps/vite/src/components/calendar/CalendarHeader.tsx(1 hunks)apps/vite/src/components/calendar/DayDialog.tsx(1 hunks)apps/vite/src/components/calendar/TimelineView.tsx(1 hunks)apps/vite/src/components/ui/dialog.tsx(1 hunks)apps/vite/src/components/ui/drawer.tsx(1 hunks)apps/vite/src/components/ui/sheet.tsx(1 hunks)apps/vite/src/pages/Calendar2.tsx(1 hunks)apps/vite/tsconfig.json(1 hunks)apps/vite/tsconfig.node.json(1 hunks)apps/vite/vite.config.d.ts(1 hunks)apps/vite/vite.config.js(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/vite/tsconfig.node.json
- apps/vite/vite.config.d.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/vite/tsconfig.json
🧰 Additional context used
🪛 Biome
apps/vite/src/components/Spinner.tsx
[error] 6-6: The elements with the following roles can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
apps/vite/src/components/SplitButton.tsx
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
apps/vite/src/components/calendar/CalendarHeader.tsx
[error] 155-161: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
[error] 1-1: The default import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 105-111: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 114-120: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 125-128: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 143-143: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 146-146: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 149-149: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 154-154: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
apps/vite/src/components/calendar/DayDialog.tsx
[error] 45-47: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
apps/vite/src/components/calendar/TimelineView.tsx
[error] 9-9: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
apps/vite/src/pages/Calendar2.tsx
[error] 17-17: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 27-27: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
[error] 28-28: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
apps/vite/vite.config.js
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
🔇 Additional comments (22)
apps/vite/src/components/SplitButton.tsx (1)
18-20: LGTM! Clean refactor to use the new cn utility.
The className concatenation logic is well-structured and maintains the same functionality while using the new utility.
apps/vite/src/components/calendar/DayDialog.tsx (2)
14-22: LGTM! Well-structured component interface
The props interface is clean, well-typed, and follows React best practices.
25-36: Previous accessibility concerns still apply
The desktop view implementation could benefit from the accessibility improvements suggested in the previous review.
apps/vite/src/components/Spinner.tsx (4)
1-3: LGTM! Clean TypeScript implementation.
The component declaration with optional className prop and the import using path alias follow best practices.
9-13: LGTM! Proper className composition.
The use of cn utility for merging default styles with optional className follows best practices.
Line range hint 14-28: LGTM! Well-implemented SVG spinner.
The SVG implementation follows best practices with proper attributes, theme-aware colors, and accessibility considerations.
🧰 Tools
🪛 Biome
[error] 6-6: The elements with the following roles can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
29-32: LGTM! Clean export.
The export statement follows standard practices.
apps/vite/src/components/ui/drawer.tsx (5)
1-16: LGTM! Clean implementation of the base Drawer component.
The imports are well-organized and the base Drawer component provides a clean wrapper with appropriate prop handling.
17-22: LGTM! Clean re-exports of primitive components.
109-120: LGTM! Well-organized exports.
The exports are clean and follow best practices.
36-56:
Remove @ts-ignore and improve component configurability.
- The @ts-ignore comment should be replaced with proper typing.
- The previous suggestion about making drawer dimensions configurable is still valid.
80-107:
Address type safety and theme consistency.
- Remove @ts-ignore comments by properly typing the refs
- The previous suggestion about using theme tokens for text styles is still valid
apps/vite/src/components/ui/dialog.tsx (6)
1-14: LGTM! Clean imports and component definitions.
The imports are well-organized and the basic component definitions are cleanly implemented using Radix UI primitives.
15-31: Verify the TypeScript ignore necessity.
While the comment suggests this is a package issue, it's worth verifying if there's a workaround or if the package has been updated to fix this.
#!/bin/bash
# Check if there's a newer version of @radix-ui/react-dialog that might fix the type issue
npm view @radix-ui/react-dialog versions --json | jq -r '.[]' | sort -V | tail -n 5The previous suggestion about extracting animation classes for better maintainability is still valid.
32-56: Address TypeScript and accessibility concerns.
- Another @ts-ignore is present - this should be investigated along with the previous one.
- The close button implementation has good accessibility with the sr-only text.
The previous suggestions about:
- Making the dialog more flexible with adjustable max-height
- Adding aria-describedby for better accessibility
are still valid and should be addressed.
57-84: LGTM! Well-implemented responsive components.
The DialogHeader and DialogFooter components are well-implemented with proper responsive design considerations and follow React best practices.
114-125: LGTM! Well-organized exports.
All components are properly exported with clear named exports.
85-112: Investigate TypeScript issues across components.
Multiple @ts-ignore comments suggest a systematic issue with the types. Consider:
- Checking if you're using the correct version of @radix-ui/react-dialog
- Ensuring your TypeScript version is compatible
- Looking for type definition issues in your project setup
✅ Verification successful
@ts-ignore comments are expected with current Radix UI Dialog version
The TypeScript issues with ref forwarding in the Dialog components are known issues with @radix-ui/[email protected]. The @ts-ignore comments are currently necessary and documented in the code ("cannot fix the type error, package error, not my fault"). This is consistent across all Dialog primitive components in the file.
Key findings:
- Using latest stable version (1.1.2) of @radix-ui/react-dialog
- TypeScript 5.4.5 is being used, which is up-to-date
- The issue is isolated to ref forwarding in Dialog components
- Similar pattern appears in multiple Dialog subcomponents, indicating this is a systematic issue with the package rather than local implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check TypeScript and package versions
echo "Checking TypeScript version..."
grep '"typescript":' package.json
echo "Checking @radix-ui/react-dialog version..."
grep '"@radix-ui/react-dialog":' package.json
Length of output: 276
Script:
#!/bin/bash
# Check package.json files recursively for @radix-ui/react-dialog dependency
echo "Searching for @radix-ui/react-dialog in all package.json files..."
fd package.json --exec grep -l "@radix-ui/react-dialog" {}
# Check for similar TypeScript ignores in dialog components
echo -e "\nChecking for @ts-ignore in dialog components..."
rg "@ts-ignore" -C 2 --glob "**/*dialog*.{ts,tsx}"
# Look for other Radix UI component implementations
echo -e "\nChecking other Radix UI component implementations..."
rg "React\.forwardRef.*typeof.*Primitive" -C 2 --glob "**/*.{ts,tsx}"
Length of output: 1916
Script:
#!/bin/bash
# Check the version of @radix-ui/react-dialog in the vite app
echo "Checking @radix-ui/react-dialog version in apps/vite/package.json..."
cat apps/vite/package.json | grep -A 1 -B 1 "@radix-ui/react-dialog"
# Look for similar patterns in other Radix UI components
echo -e "\nChecking other Radix UI components for similar patterns..."
fd -e tsx -e ts -p "apps/vite/src/components/ui/" --exec grep -l "React.forwardRef.*typeof.*Primitive" {}
# Check if there are any type definition files for dialog
echo -e "\nChecking for dialog type definitions..."
fd -e d.ts -p "node_modules/@radix-ui/react-dialog/dist" --exec cat {} 2>/dev/null
Length of output: 727
apps/vite/src/components/calendar/CalendarHeader.tsx (2)
14-19: LGTM! Props are well-typed and properly destructured
The component's props interface is clear and the destructuring is clean.
105-173: 🛠️ Refactor suggestion
Add missing button types and accessibility attributes
Several accessibility improvements are needed:
- Add
type="button"to all buttons - Add accessibility attributes to the settings SVG
// Example for one button (apply to all):
<button
+ type="button"
className={cn(
"p-2 rounded-md bg-gray-200 border-2 border-gray-200 inline-flex",
view === "calendar" ? "bg-white" : ""
)}
onClick={() => setView("calendar")}
>
// For the settings SVG:
<svg
xmlns="http://www.w3.org/2000/svg"
width={20}
height={20}
viewBox="0 0 20 20"
fill="none"
+ role="img"
+ aria-label="Settings"
>Likely invalid or redundant comment.
🧰 Tools
🪛 Biome
[error] 155-161: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
[error] 105-111: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 114-120: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 125-128: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 143-143: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 146-146: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 149-149: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 154-154: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
apps/vite/src/components/ui/sheet.tsx (2)
31-48: Double-check variant combinations in sheetVariants for consistency
The sheetVariants function defines styling for different side options. Ensure that all animations and styles are correctly applied for each variant, and that they behave as expected when the sheet is opened and closed from different sides.
You can test each variant in the application to confirm that the animations and positions are consistent.
1-141: Well-structured component utilizing Radix UI primitives
The new Sheet component and its subcomponents are well-defined using Radix UI's dialog primitives. The use of class-variance-authority for handling styling variants enhances maintainability and flexibility. Overall, the code is clean, and the components are exported correctly for use in the application.
| @@ -0,0 +1,12 @@ | |||
| import path from "path"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use the node: protocol for Node.js built-in modules.
For better clarity and to follow Node.js best practices, use the node: protocol when importing built-in modules.
-import path from "path";
+import path from "node:path";📝 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 path from "path"; | |
| import path from "node:path"; |
🧰 Tools
🪛 Biome
[error] 1-1: A Node.js builtin module should be imported with the node: protocol.
Using the node: protocol is more explicit and signals that the imported module belongs to Node.js.
Unsafe fix: Add the node: protocol.
(lint/style/useNodejsImportProtocol)
| "@miti/query": "*", | ||
| "@miti/types": "*", |
There was a problem hiding this comment.
Use explicit versions for workspace dependencies
Using "*" for workspace dependencies (@miti/query, @miti/types) can lead to unexpected breaking changes and non-deterministic builds. Consider using explicit versions or workspace: protocol.
- "@miti/query": "*",
- "@miti/types": "*",
+ "@miti/query": "workspace:*",
+ "@miti/types": "workspace:*",📝 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.
| "@miti/query": "*", | |
| "@miti/types": "*", | |
| "@miti/query": "workspace:*", | |
| "@miti/types": "workspace:*", |
| "@vitejs/plugin-react-swc": "^3.0.0", | ||
| "autoprefixer": "^10.4.14", | ||
| "eslint": "^9.11.1", | ||
| "eslint-plugin-react-hooks": "^5.1.0-rc.0", |
There was a problem hiding this comment.
Avoid using release candidate versions in production
The ESLint React Hooks plugin is using a release candidate version (^5.1.0-rc.0), which may not be stable for production use.
- "eslint-plugin-react-hooks": "^5.1.0-rc.0",
+ "eslint-plugin-react-hooks": "^4.6.0",📝 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.
| "eslint-plugin-react-hooks": "^5.1.0-rc.0", | |
| "eslint-plugin-react-hooks": "^4.6.0", |
| "@vitejs/plugin-react-swc": "^3.0.0", | ||
| "autoprefixer": "^10.4.14", | ||
| "eslint": "^9.11.1", |
There was a problem hiding this comment.
💡 Codebase verification
ESLint v9.11.1 is an incorrect version - this appears to be an error
The specified ESLint version (^9.11.1) in apps/vite/package.json is incorrect as ESLint's current stable version is in the 8.x range. Additionally, there are incompatible dependency versions:
@typescript-eslint/eslint-pluginand@typescript-eslint/parserare at v5.59.0 which are not compatible with ESLint 9.x- The workspace's shared ESLint config package (@miti/eslint-config) uses ESLint 8.56.0
The version should be downgraded to match the workspace's ESLint version (^8.56.0) to maintain compatibility across the monorepo.
🔗 Analysis chain
Verify compatibility with major version updates
The significant version updates to ESLint (^9.11.1), TypeScript (^5.5.3), and Vite (^5.4.8) may require additional configuration changes or code modifications.
Also applies to: 58-58, 60-60
Let me gather more information about the ESLint configuration and potential compatibility issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential TypeScript and ESLint compatibility issues
# Look for tsconfig.json and verify its contents are compatible with TS 5.5
echo "Checking TypeScript configuration..."
if [ -f "tsconfig.json" ]; then
cat tsconfig.json
fi
# Check for ESLint configuration
echo "Checking ESLint configuration..."
for config in .eslintrc.* .eslintrc; do
if [ -f "$config" ]; then
cat "$config"
fi
done
# Look for Vite configuration
echo "Checking Vite configuration..."
for config in vite.config.ts vite.config.js; do
if [ -f "$config" ]; then
cat "$config"
fi
done
Length of output: 612
Script:
#!/bin/bash
# Search for ESLint config files recursively and check package dependencies
echo "Searching for ESLint config files..."
fd -e .eslintrc.js -e .eslintrc.json -e .eslintrc.yaml -e .eslintrc.yml
echo -e "\nChecking package.json files for ESLint related dependencies..."
fd package.json --exec grep -l "eslint"
echo -e "\nChecking content of found package.json files..."
fd package.json --exec cat {} \; | grep -A 5 -B 5 "eslint"
echo -e "\nChecking for TypeScript related files..."
fd tsconfig.json
Length of output: 5390
| const DrawerOverlay = React.forwardRef< | ||
| React.ElementRef<typeof DrawerPrimitive.Overlay>, | ||
| React.ComponentPropsWithoutRef<typeof DrawerPrimitive.Overlay> | ||
| >(({ className, ...props }, ref) => ( | ||
| <DrawerPrimitive.Overlay | ||
| // @ts-ignore | ||
| ref={ref} | ||
| className={cn("fixed inset-0 z-50 bg-black/80", className)} | ||
| {...props} | ||
| /> | ||
| )) | ||
| DrawerOverlay.displayName = DrawerPrimitive.Overlay.displayName | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address TypeScript errors instead of using @ts-ignore.
The @ts-ignore comment suggests underlying type issues. Consider:
- Investigating why the type error occurs
- Properly typing the ref to avoid the need for @ts-ignore
Additionally, consider making the z-index configurable to prevent potential stacking context issues in different contexts.
const DrawerOverlay = React.forwardRef<
React.ElementRef<typeof DrawerPrimitive.Overlay>,
- React.ComponentPropsWithoutRef<typeof DrawerPrimitive.Overlay>
+ React.ComponentPropsWithoutRef<typeof DrawerPrimitive.Overlay> & {
+ zIndex?: number;
+ }
->(({ className, ...props }, ref) => (
+>(({ className, zIndex = 50, ...props }, ref) => (
<DrawerPrimitive.Overlay
- // @ts-ignore
ref={ref}
- className={cn("fixed inset-0 z-50 bg-black/80", className)}
+ className={cn(`fixed inset-0 z-${zIndex} bg-black/80`, className)}
{...props}
/>
))Committable suggestion skipped: line range outside the PR's diff.
| const validYearAndMonth = useMemo(() => { | ||
| if (!BSYear || !BSMonth) return new NepaliDate() | ||
| const year = parseInt(BSYear) | ||
| const month = parseInt(BSMonth) | ||
| const isValid = year >= 2075 && year <= 2082 && month >= 1 && month <= 12 | ||
|
|
||
| if (isValid) return new NepaliDate(year, month - 1, 1) | ||
| return new NepaliDate() | ||
| }, [BSYear, BSMonth]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving date validation handling
The validation logic has two potential issues:
- The year range (2075-2082) is hardcoded without clear reasoning
- Invalid dates silently fall back to current date without user notification
Consider:
- Moving year range to constants with clear documentation
- Adding error handling for invalid dates:
const validYearAndMonth = useMemo(() => {
if (!BSYear || !BSMonth) return new NepaliDate()
const year = Number.parseInt(BSYear)
const month = Number.parseInt(BSMonth)
- const isValid = year >= 2075 && year <= 2082 && month >= 1 && month <= 12
+ const MIN_YEAR = 2075
+ const MAX_YEAR = 2082
+
+ const isValid = year >= MIN_YEAR && year <= MAX_YEAR && month >= 1 && month <= 12
+
+ if (!isValid) {
+ console.warn(`Invalid date: year must be between ${MIN_YEAR}-${MAX_YEAR}, month between 1-12`)
+ // Consider showing user feedback here
+ }
if (isValid) return new NepaliDate(year, month - 1, 1)
return new NepaliDate()
}, [BSYear, BSMonth])Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome
[error] 27-27: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
[error] 28-28: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
| const { data: calendarData } = useCalendarData(currentNepaliDate) | ||
|
|
||
| const currentMonth = currentNepaliDate.getMonth() + 1 | ||
|
|
||
| const monthData = useMemo(() => { | ||
| if (!calendarData) return [] | ||
| return calendarData | ||
| }, [calendarData, currentMonth]) as unknown as NewCalendarData[] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety and loading state handling
The current implementation has two issues:
- Unsafe type casting with
as unknown as NewCalendarData[] - Missing loading state handling for
calendarData
Consider this improvement:
+const { data: calendarData, isLoading: calendarDataLoading } = useCalendarData(currentNepaliDate)
const monthData = useMemo(() => {
+ if (calendarDataLoading) return []
if (!calendarData) return []
return calendarData
-}, [calendarData, currentMonth]) as unknown as NewCalendarData[]
+}, [calendarData, calendarDataLoading, currentMonth])Also, ensure that useCalendarData hook properly types its return value as NewCalendarData[].
Committable suggestion skipped: line range outside the PR's diff.
| // <div className="flex flex-row gap-4 items-center justify-between mb-5"> | ||
| // <div className="w-full max-w-7xl"> | ||
| // <div className="flex items-center justify-between gap-3"> | ||
| // <div className="flex items-center gap-4 flex-1"> | ||
| // <div className="flex items-center gap-2 flex-1"> | ||
| // <div className="bg-gray-200 text-sm text-gray-500 leading-none border-2 border-gray-200 rounded-md inline-flex"> | ||
| // <button | ||
| // className={cn( | ||
| // "inline-flex items-center transition-colors duration-300 ease-in focus:outline-none hover:text-blue-400 focus:text-blue-400 rounded-md px-2 py-2 active", | ||
| // view === "calendar" ? "bg-white" : "" | ||
| // )} | ||
| // id="grid" | ||
| // onClick={() => setView("calendar")} | ||
| // > | ||
| // <EventIcon /> | ||
| // </button> | ||
| // <button | ||
| // className={cn( | ||
| // "inline-flex items-center transition-colors duration-300 ease-in focus:outline-none hover:text-blue-400 focus:text-blue-400 rounded-md px-2 py-2", | ||
| // view === "event" ? "bg-white" : "" | ||
| // )} | ||
| // id="list" | ||
| // onClick={() => setView("event")} | ||
| // > | ||
| // <ListIcon /> | ||
| // </button> | ||
| // </div> | ||
| // <button | ||
| // className="hidden md:flex py-2 pl-1.5 pr-3 rounded-md bg-gray-50 border border-gray-300 items-center gap-1.5 text-xs font-medium text-gray-900 transition-all duration-500 hover:bg-gray-200" | ||
| // onClick={() => setCurrentNepaliDate(new NepaliDate())} | ||
| // > | ||
| // <EventIcon /> | ||
| // Today | ||
| // </button> | ||
| // </div> | ||
| // </div> | ||
| // <div> | ||
| // <YearMonthPicker | ||
| // currentNepaliDate={currentNepaliDate} | ||
| // setCurrentNepaliDate={setCurrentNepaliDate} | ||
| // /> | ||
| // </div> | ||
|
|
||
| // <div className="flex items-center justify-end gap-2 flex-1"> | ||
| // {view === "event" && ( | ||
| // <div className="flex items-center gap-px p-1 rounded-md bg-gray-100"> | ||
| // <button className="py-2 px-5 rounded-lg bg-gray-100 text-xs font-medium text-gray-900 transition-all duration-300 hover:bg-white"> | ||
| // Day | ||
| // </button> | ||
| // <button className="py-2 px-5 rounded-lg bg-white text-xs font-medium text-gray-900 transition-all duration-300 hover:bg-white"> | ||
| // Week | ||
| // </button> | ||
| // <button className="py-2 px-5 rounded-lg bg-gray-100 text-xs font-medium text-gray-900 transition-all duration-300 hover:bg-white"> | ||
| // Month | ||
| // </button> | ||
| // </div> | ||
| // )} | ||
| // <button className="hidden sm:flex p-3 text-gray-500 items-center justify-center transition-all duration-300 hover:text-gray-900"> | ||
| // <svg | ||
| // xmlns="http://www.w3.org/2000/svg" | ||
| // width={20} | ||
| // height={20} | ||
| // viewBox="0 0 20 20" | ||
| // fill="none" | ||
| // > | ||
| // <path | ||
| // d="M16.7501 9.20285C17.4658 8.79486 17.7119 7.89657 17.3012 7.19137L16.559 5.91699C16.1385 5.19489 15.1995 4.94638 14.466 5.36301C13.4469 5.94185 12.1716 5.19438 12.1716 4.03885C12.1716 3.19732 11.4764 2.5 10.6188 2.5H9.17632C8.33949 2.5 7.66111 3.16571 7.66111 3.9869V4.09222C7.66111 5.19157 6.44713 5.8776 5.4782 5.3258C4.78224 4.92946 3.89057 5.16345 3.48911 5.84779L2.70103 7.19113C2.28763 7.89582 2.5327 8.79589 3.24919 9.20432C4.24554 9.77228 4.24337 11.1862 3.24904 11.7576C2.53341 12.1688 2.28715 13.0712 2.7009 13.7764L3.48911 15.12C3.89057 15.8043 4.78478 16.0369 5.48074 15.6406C6.4466 15.0905 7.66111 15.7719 7.66111 16.8677C7.66111 17.6529 8.30976 18.2895 9.10991 18.2895H10.6853C11.5061 18.2895 12.1716 17.6365 12.1716 16.831C12.1716 15.7075 13.4115 15.0058 14.4024 15.5686L14.466 15.6048C15.1995 16.0214 16.1385 15.7729 16.559 15.0508L17.3013 13.7762C17.7124 13.0704 17.4651 12.1699 16.7502 11.7591C15.7547 11.1871 15.7526 9.77146 16.7501 9.20285Z" | ||
| // stroke="currentcolor" | ||
| // strokeWidth="1.3" | ||
| // /> | ||
| // <path | ||
| // d="M12.68 10.3947C12.68 11.8481 11.4794 13.0263 9.99834 13.0263C8.5173 13.0263 7.31668 11.8481 7.31668 10.3947C7.31668 8.94136 8.5173 7.76316 9.99834 7.76316C11.4794 7.76316 12.68 8.94136 12.68 10.3947Z" | ||
| // stroke="currentcolor" | ||
| // strokeWidth="1.3" | ||
| // /> | ||
| // </svg> | ||
| // </button> | ||
| // </div> | ||
| // </div> | ||
| // </div> | ||
| // </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented out code
Large blocks of commented out code should be removed as they:
- Add unnecessary noise to the codebase
- Make the file harder to maintain
- Can become outdated and misleading
If this code needs to be referenced later, it can be found in the git history.
| // @ts-ignore | ||
| ref={ref} |
There was a problem hiding this comment.
Avoid suppressing TypeScript errors with // @ts-ignore; properly type forwarded refs
The use of // @ts-ignore at lines 25, 61, 110, and 122 suppresses TypeScript errors, which may hide potential issues and reduce type safety. It's recommended to resolve these TypeScript errors by properly typing the components that use React.forwardRef. This ensures type safety and maintainability of the code.
Consider adjusting the typings of the forwarded ref components. Here's how you can modify one of them:
const SheetOverlay = React.forwardRef<
React.ElementRef<typeof SheetPrimitive.Overlay>,
React.ComponentPropsWithoutRef<typeof SheetPrimitive.Overlay>
>(
- ({ className, ...props }, ref) => (
+ (props, ref) => (
<SheetPrimitive.Overlay
className={cn(
"fixed inset-0 z-50 bg-black/80 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0",
- className
)}
{...props}
ref={ref}
/>
)
)By spreading props before using ref, and moving className inside props, you can eliminate the need for // @ts-ignore. Apply similar changes to the other components to ensure proper typing without suppressing errors.
Also applies to: 61-62, 110-111, 122-123
Deploying miti-web with
|
| Latest commit: |
ad921cb
|
| Status: | 🚫 Build failed. |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (13)
apps/vite/src/index.css (3)
15-27: Consider using theme variables for scrollbar colors and adding dark mode support.The scrollbar styles currently use hardcoded colors which may not be visible in dark mode and don't maintain consistency with the theme.
Consider applying this improvement:
::-webkit-scrollbar-track { - background: #f1f1f1; + background: hsl(var(--muted)); } ::-webkit-scrollbar-thumb { border-radius: 8px; - background: #888; + background: hsl(var(--muted-foreground)); } + +.dark ::-webkit-scrollbar-track { + background: hsl(var(--muted)); +} + +.dark ::-webkit-scrollbar-thumb { + background: hsl(var(--muted-foreground)); +}
56-58: Simplify identical grid auto row variables.The grid auto row variables for all sizes (lg, md, sm) have identical values of
1fr. Consider consolidating them into a single variable if there's no plan for different values.- --grid-auto-rows-lg: 1fr; - --grid-auto-rows-md: 1fr; - --grid-auto-rows-sm: 1fr; + --grid-auto-rows: 1fr;
29-86: Consider adding documentation for the theming system.The theme implementation is well-structured, but adding documentation would help other developers understand:
- The color system organization (background, foreground, primary, etc.)
- Usage guidelines for different color variables
- When to use specific semantic colors (e.g., destructive vs accent)
Consider adding a comment block at the start of the
@layer basesection explaining the theming system and its usage.apps/vite/src/components/calendar/TimelineView.tsx (2)
1-7: Useimport typefor type-only importsUsing
import typefor type-only imports helps with tree-shaking and ensures better compile-time optimization.-import { NewCalendarData } from "@miti/types" +import type { NewCalendarData } from "@miti/types"🧰 Tools
🪛 Biome
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
9-9: Consider implementing real-time updatesThe current time indicator is static after initial render. Consider implementing a useEffect hook with an interval to update the current time periodically for real-time accuracy.
const TimelineView = ({ monthData, scope }: TimelineViewProps) => { const [time, setTime] = useState(new Date()); useEffect(() => { const interval = setInterval(() => setTime(new Date()), 60000); // Update every minute return () => clearInterval(interval); }, []); // Pass time to Day component instead of using static currentTime return ( // ... existing render logic ); };apps/vite/src/components/calendar/CalendarGrid.tsx (3)
14-17: Consider using undefined instead of null for stateIn TypeScript/React,
undefinedis generally preferred overnullfor representing the absence of a value. This aligns better with TypeScript's strictNullChecks and React's patterns.- const [dayDialogData, setDayDialogData] = useState<NewCalendarData | null>( - null - ) + const [dayDialogData, setDayDialogData] = useState<NewCalendarData | undefined>( + undefined + )
Line range hint
94-104: Remove hardcoded event display logicThe event display logic contains hardcoded values ("Developer Meetup") and conditions (
index % 3 === 0). This should be driven by the actual event data.- {index % 3 === 0 && ( + {day.eventDetails.length > 0 && ( <div className="absolute bottom-1 left-1 p-1.5 h-max rounded xl:bg-emerald-50 z-10 flex-row"> <p className="hidden xl:block text-xs font-medium text-emerald-600 mb-px whitespace-nowrap"> - Developer Meetup + {day.eventDetails[0].title.np} </p> <p className="xl:hidden w-2 h-2 rounded-full bg-emerald-600" /> <p className="hidden xl:block text-xs font-medium text-emerald-600 mb-px whitespace-nowrap"> - Developer Meetup + {day.eventDetails[0].title.en} </p> <p className="xl:hidden w-2 h-2 rounded-full bg-emerald-600" /> </div> )}🧰 Tools
🪛 Biome
[error] 70-70: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 2-2: The default import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 60-75: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
110-114: Add proper TypeScript types for dialog propsConsider creating an interface for the dialog props to improve type safety and documentation.
interface DayDialogProps { open: boolean; setOpen: (open: boolean) => void; children: React.ReactNode; }🧰 Tools
🪛 Biome
[error] 113-113: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
apps/vite/src/pages/Calendar2.tsx (4)
17-17: Useimport typefor type-only importsThe
NewCalendarDataimport is only used for type annotations. Usingimport typehelps the bundler optimize the code by removing type-only imports during compilation.-import { NewCalendarData } from "@miti/types" +import type { NewCalendarData } from "@miti/types"🧰 Tools
🪛 Biome
[error] 17-17: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
30-30: Document the significance of the year rangeThe hardcoded year range
2075-2082appears to be specific to the Nepali calendar system. Consider adding a comment explaining the significance of these bounds or moving them to named constants with documentation.+ // BS 2075-2082 represents the supported date range in the Nepali calendar + const MIN_SUPPORTED_YEAR = 2075 + const MAX_SUPPORTED_YEAR = 2082 - const isValid = year >= 2075 && year <= 2082 && month >= 1 && month <= 12 + const isValid = year >= MIN_SUPPORTED_YEAR && year <= MAX_SUPPORTED_YEAR && month >= 1 && month <= 12
99-104: Handle loading states in EventList componentsThe EventList components receive combined data but don't handle loading states. Consider passing loading states to improve user experience during data fetching.
- <EventList data={combinedData} title="आगामी इभेन्टहरु" /> + <EventList + data={combinedData} + title="आगामी इभेन्टहरु" + isLoading={nextMonthDataLoading} + /> <EventList data={combinedData} title="आगामी बिदाहरु" isHoliday={true} + isLoading={nextMonthDataLoading} />
84-91: Consider memoizing child componentsThe CalendarHeader component receives multiple props and callback functions. Consider memoizing it to prevent unnecessary re-renders.
+const MemoizedCalendarHeader = React.memo(CalendarHeader) - <CalendarHeader + <MemoizedCalendarHeader currentNepaliDate={currentNepaliDate} setCurrentNepaliDate={setCurrentNepaliDate} view={view} setView={setView} scope={scope} setScope={setScope} />apps/vite/src/components/calendar/CalendarHeader.tsx (1)
16-23: Consider using React.memo for performance optimizationThe component receives multiple props that could trigger frequent re-renders. Consider wrapping it with React.memo to prevent unnecessary re-renders when parent components update.
-const CalendarHeader: React.FC<CalendarHeaderProps> = ({ +const CalendarHeader: React.FC<CalendarHeaderProps> = React.memo(({ currentNepaliDate, setCurrentNepaliDate, view, setView, scope, setScope, -}) => { +})) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
apps/vite/src/components/calendar/CalendarGrid.tsx(2 hunks)apps/vite/src/components/calendar/CalendarHeader.tsx(1 hunks)apps/vite/src/components/calendar/TimelineView.tsx(1 hunks)apps/vite/src/index.css(1 hunks)apps/vite/src/pages/Calendar2.tsx(1 hunks)
🧰 Additional context used
🪛 Biome
apps/vite/src/components/calendar/CalendarGrid.tsx
[error] 70-70: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 2-2: The default import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 60-75: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 113-113: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
apps/vite/src/components/calendar/CalendarHeader.tsx
[error] 95-101: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
[error] 1-1: The default import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 28-34: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 37-43: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 48-51: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 65-71: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 74-80: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 83-89: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 94-94: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
apps/vite/src/components/calendar/TimelineView.tsx
[error] 41-41: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 45-45: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 46-46: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 49-56: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 17-17: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
[error] 33-33: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
[error] 83-83: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 91-91: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
apps/vite/src/pages/Calendar2.tsx
[error] 17-17: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 28-28: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
[error] 29-29: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
🔇 Additional comments (3)
apps/vite/src/components/calendar/CalendarGrid.tsx (2)
9-11: LGTM: Clean type definition
The CalendarGridProps type is well-defined and properly typed with NewCalendarData[].
34-37: LGTM: Clean conditional styling implementation
Good use of the cn utility for conditional class names and proper handling of border cases for the first element.
apps/vite/src/components/calendar/CalendarHeader.tsx (1)
63-93: LGTM: View mode implementation is complete
The Day/Week/Month view implementation looks complete with:
- Proper state management through
scopeandsetScopeprops - Correct conditional rendering based on
view === "event" - Working click handlers for each view mode
🧰 Tools
🪛 Biome
[error] 65-71: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 74-80: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 83-89: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
| const generateTimeline = ( | ||
| scope: TimelineViewProps["scope"], | ||
| monthData: TimelineViewProps["monthData"] | ||
| ) => { | ||
| switch (scope) { | ||
| case "day": | ||
| return <Day monthData={monthData} /> | ||
| case "week": | ||
| return <Week monthData={monthData} /> | ||
| case "month": | ||
| return <div>Month</div> | ||
| } | ||
| } |
There was a problem hiding this comment.
Implement error handling and complete month view
The month view is incomplete, and there's no error handling for invalid scope values.
const generateTimeline = (
scope: TimelineViewProps["scope"],
monthData: TimelineViewProps["monthData"]
) => {
switch (scope) {
case "day":
return <Day monthData={monthData} />
case "week":
return <Week monthData={monthData} />
case "month":
- return <div>Month</div>
+ return <div>Month view not implemented</div>
+ default:
+ console.error(`Invalid scope: ${scope}`);
+ return null;
}
}Would you like me to help implement the month view component or create a GitHub issue to track this task?
Committable suggestion skipped: line range outside the PR's diff.
| {Array.from({ length: 24 }).map((_, i) => ( | ||
| <div | ||
| key={i} | ||
| className="h-[120px] bg-gray-200 flex items-center justify-center px-2 text-xs font-semibold" | ||
| > | ||
| {i === 0 | ||
| ? "12:00 AM" | ||
| : i < 12 | ||
| ? `${i}:00 AM` | ||
| : i === 12 | ||
| ? "12:00 PM" | ||
| : `${i - 12}:00 PM`} | ||
| </div> | ||
| ))} |
There was a problem hiding this comment.
Avoid using array index as React key
Using array indices as keys can lead to rendering issues if the order of items changes. Consider using a more stable unique identifier.
-{Array.from({ length: 24 }).map((_, i) => (
+{Array.from({ length: 24 }).map((_, i) => {
+ const timeKey = `time-${i}`
+ return (
<div
- key={i}
+ key={timeKey}📝 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.
| {Array.from({ length: 24 }).map((_, i) => ( | |
| <div | |
| key={i} | |
| className="h-[120px] bg-gray-200 flex items-center justify-center px-2 text-xs font-semibold" | |
| > | |
| {i === 0 | |
| ? "12:00 AM" | |
| : i < 12 | |
| ? `${i}:00 AM` | |
| : i === 12 | |
| ? "12:00 PM" | |
| : `${i - 12}:00 PM`} | |
| </div> | |
| ))} | |
| {Array.from({ length: 24 }).map((_, i) => { | |
| const timeKey = `time-${i}` | |
| return ( | |
| <div | |
| key={timeKey} | |
| className="h-[120px] bg-gray-200 flex items-center justify-center px-2 text-xs font-semibold" | |
| > | |
| {i === 0 | |
| ? "12:00 AM" | |
| : i < 12 | |
| ? `${i}:00 AM` | |
| : i === 12 | |
| ? "12:00 PM" | |
| : `${i - 12}:00 PM`} | |
| </div> | |
| ) | |
| })} |
🧰 Tools
🪛 Biome
[error] 17-17: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
| {weekData.map((data) => ( | ||
| <div className="h-[120px] flex items-center "> | ||
| <div className="px-2 flex flex-col items-center justify-center bg-gray-200 h-full w-20"> | ||
| <p>{data.calendarInfo.dates.bs.day.np}</p> | ||
| <p>{data.calendarInfo.days.dayOfWeek.np}</p> | ||
| </div> | ||
| <div className="flex-1"> | ||
| {data.eventDetails.map((event) => { | ||
| return ( | ||
| <div className="px-2 flex flex-col items-center justify-center h-full w-full"> | ||
| <p>{event.title.np}</p> | ||
| </div> | ||
| ) | ||
| })} | ||
| </div> | ||
| </div> | ||
| ))} |
There was a problem hiding this comment.
Add missing key props to mapped elements
React requires unique key props for elements in iterables for optimal rendering performance.
-{weekData.map((data) => (
+{weekData.map((data) => (
- <div className="h-[120px] flex items-center ">
+ <div key={data.calendarInfo.dates.bs.day.en} className="h-[120px] flex items-center">
{/* ... */}
{data.eventDetails.map((event) => {
return (
- <div className="px-2 flex flex-col items-center justify-center h-full w-full">
+ <div key={event.title.np} className="px-2 flex flex-col items-center justify-center h-full w-full">📝 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.
| {weekData.map((data) => ( | |
| <div className="h-[120px] flex items-center "> | |
| <div className="px-2 flex flex-col items-center justify-center bg-gray-200 h-full w-20"> | |
| <p>{data.calendarInfo.dates.bs.day.np}</p> | |
| <p>{data.calendarInfo.days.dayOfWeek.np}</p> | |
| </div> | |
| <div className="flex-1"> | |
| {data.eventDetails.map((event) => { | |
| return ( | |
| <div className="px-2 flex flex-col items-center justify-center h-full w-full"> | |
| <p>{event.title.np}</p> | |
| </div> | |
| ) | |
| })} | |
| </div> | |
| </div> | |
| ))} | |
| {weekData.map((data) => ( | |
| <div key={data.calendarInfo.dates.bs.day.en} className="h-[120px] flex items-center"> | |
| <div className="px-2 flex flex-col items-center justify-center bg-gray-200 h-full w-20"> | |
| <p>{data.calendarInfo.dates.bs.day.np}</p> | |
| <p>{data.calendarInfo.days.dayOfWeek.np}</p> | |
| </div> | |
| <div className="flex-1"> | |
| {data.eventDetails.map((event) => { | |
| return ( | |
| <div key={event.title.np} className="px-2 flex flex-col items-center justify-center h-full w-full"> | |
| <p>{event.title.np}</p> | |
| </div> | |
| ) | |
| })} | |
| </div> | |
| </div> | |
| ))} |
🧰 Tools
🪛 Biome
[error] 83-83: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 91-91: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
| {currentTime.getHours() === i && ( | ||
| <div className="relative h-full w-full"> | ||
| <div | ||
| className="absolute flex items-center justify-center transform -translate-x-1/2 -translate-y-1/2 origin-center" | ||
| style={{ | ||
| top: (currentTime.getMinutes() / 60) * 100 + "%", | ||
| }} | ||
| > | ||
| <div className="relative flex items-center justify-center size-2"> | ||
| <span className="absolute w-2 h-2 rounded-full bg-red-500 z-10"></span> | ||
| <span className="absolute w-2 h-2 rounded-full bg-red-500 animate-ping z-0"></span> | ||
| </div> | ||
| </div> | ||
| <div | ||
| className="absolute h-0.5 w-full bg-red-500" | ||
| style={{ | ||
| top: `calc(${ | ||
| (currentTime.getMinutes() / 60) * 100 | ||
| }% - 1px)`, | ||
| }} | ||
| ></div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize current time indicator calculations
The current time position calculation is duplicated and could be extracted for better maintainability and performance.
+const getCurrentTimePosition = (currentTime: Date) => {
+ return (currentTime.getMinutes() / 60) * 100
+}
{currentTime.getHours() === i && (
<div className="relative h-full w-full">
+ {/* Get position once and reuse */}
+ {(() => {
+ const position = getCurrentTimePosition(currentTime)
+ return (
<div
className="absolute flex items-center justify-center transform -translate-x-1/2 -translate-y-1/2 origin-center"
- style={{
- top: (currentTime.getMinutes() / 60) * 100 + "%",
- }}
+ style={{ top: `${position}%` }}
>
{/* ... */}
</div>
<div
className="absolute h-0.5 w-full bg-red-500"
- style={{
- top: `calc(${(currentTime.getMinutes() / 60) * 100}% - 1px)`,
- }}
+ style={{ top: `calc(${position}% - 1px)` }}
/>
+ )
+ })()}
</div>
)}📝 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.
| {currentTime.getHours() === i && ( | |
| <div className="relative h-full w-full"> | |
| <div | |
| className="absolute flex items-center justify-center transform -translate-x-1/2 -translate-y-1/2 origin-center" | |
| style={{ | |
| top: (currentTime.getMinutes() / 60) * 100 + "%", | |
| }} | |
| > | |
| <div className="relative flex items-center justify-center size-2"> | |
| <span className="absolute w-2 h-2 rounded-full bg-red-500 z-10"></span> | |
| <span className="absolute w-2 h-2 rounded-full bg-red-500 animate-ping z-0"></span> | |
| </div> | |
| </div> | |
| <div | |
| className="absolute h-0.5 w-full bg-red-500" | |
| style={{ | |
| top: `calc(${ | |
| (currentTime.getMinutes() / 60) * 100 | |
| }% - 1px)`, | |
| }} | |
| ></div> | |
| </div> | |
| )} | |
| {currentTime.getHours() === i && ( | |
| <div className="relative h-full w-full"> | |
| {(() => { | |
| const position = getCurrentTimePosition(currentTime) | |
| return ( | |
| <div | |
| className="absolute flex items-center justify-center transform -translate-x-1/2 -translate-y-1/2 origin-center" | |
| style={{ top: `${position}%` }} | |
| > | |
| <div className="relative flex items-center justify-center size-2"> | |
| <span className="absolute w-2 h-2 rounded-full bg-red-500 z-10"></span> | |
| <span className="absolute w-2 h-2 rounded-full bg-red-500 animate-ping z-0"></span> | |
| </div> | |
| </div> | |
| <div | |
| className="absolute h-0.5 w-full bg-red-500" | |
| style={{ top: `calc(${position}% - 1px)` }} | |
| ></div> | |
| ) | |
| })()} | |
| </div> | |
| )} | |
| ``` | |
| Note: The `getCurrentTimePosition` function definition should be added outside of this component or in a suitable location in the same file: | |
| ```typescript | |
| const getCurrentTimePosition = (currentTime: Date) => { | |
| return (currentTime.getMinutes() / 60) * 100 | |
| } |
🧰 Tools
🪛 Biome
[error] 41-41: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 45-45: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 46-46: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 49-56: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
| day.calendarInfo.dates.bs.full.en ?? "" | ||
| ).getDateObject() |
There was a problem hiding this comment.
Add null safety for date conversion
The date conversion could throw an error if the date string is null. Consider adding a fallback.
- new NepaliDate(
- day.calendarInfo.dates.bs.full.en ?? ""
- ).getDateObject()
+ new NepaliDate(
+ day.calendarInfo.dates.bs.full.en || new Date().toISOString()
+ ).getDateObject()Committable suggestion skipped: line range outside the PR's diff.
| const combinedData = useMemo(() => { | ||
| if (!nextMonthData || !calendarData) return [] | ||
| return [...calendarData, ...nextMonthData] | ||
| }, [calendarData, nextMonthData]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add data validation in combinedData
The current implementation doesn't validate that the combined data is consistent. Consider adding checks for data integrity and proper month sequence.
const combinedData = useMemo(() => {
- if (!nextMonthData || !calendarData) return []
+ if (!Array.isArray(nextMonthData) || !Array.isArray(calendarData)) return []
+
+ // Validate month sequence
+ const currentMonth = calendarData[0]?.month
+ const nextMonth = nextMonthData[0]?.month
+ if (currentMonth !== undefined && nextMonth !== undefined && nextMonth !== currentMonth + 1) {
+ console.warn('Month sequence mismatch in calendar data')
+ return calendarData
+ }
+
return [...calendarData, ...nextMonthData]
}, [calendarData, nextMonthData])📝 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.
| const combinedData = useMemo(() => { | |
| if (!nextMonthData || !calendarData) return [] | |
| return [...calendarData, ...nextMonthData] | |
| }, [calendarData, nextMonthData]) | |
| const combinedData = useMemo(() => { | |
| if (!Array.isArray(nextMonthData) || !Array.isArray(calendarData)) return [] | |
| // Validate month sequence | |
| const currentMonth = calendarData[0]?.month | |
| const nextMonth = nextMonthData[0]?.month | |
| if (currentMonth !== undefined && nextMonth !== undefined && nextMonth !== currentMonth + 1) { | |
| console.warn('Month sequence mismatch in calendar data') | |
| return calendarData | |
| } | |
| return [...calendarData, ...nextMonthData] | |
| }, [calendarData, nextMonthData]) |
| <button className="hidden sm:flex p-3 text-gray-500 items-center justify-center transition-all duration-300 hover:text-gray-900"> | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width={20} | ||
| height={20} | ||
| viewBox="0 0 20 20" | ||
| fill="none" | ||
| > | ||
| <path | ||
| d="M16.7501 9.20285C17.4658 8.79486 17.7119 7.89657 17.3012 7.19137L16.559 5.91699C16.1385 5.19489 15.1995 4.94638 14.466 5.36301C13.4469 5.94185 12.1716 5.19438 12.1716 4.03885C12.1716 3.19732 11.4764 2.5 10.6188 2.5H9.17632C8.33949 2.5 7.66111 3.16571 7.66111 3.9869V4.09222C7.66111 5.19157 6.44713 5.8776 5.4782 5.3258C4.78224 4.92946 3.89057 5.16345 3.48911 5.84779L2.70103 7.19113C2.28763 7.89582 2.5327 8.79589 3.24919 9.20432C4.24554 9.77228 4.24337 11.1862 3.24904 11.7576C2.53341 12.1688 2.28715 13.0712 2.7009 13.7764L3.48911 15.12C3.89057 15.8043 4.78478 16.0369 5.48074 15.6406C6.4466 15.0905 7.66111 15.7719 7.66111 16.8677C7.66111 17.6529 8.30976 18.2895 9.10991 18.2895H10.6853C11.5061 18.2895 12.1716 17.6365 12.1716 16.831C12.1716 15.7075 13.4115 15.0058 14.4024 15.5686L14.466 15.6048C15.1995 16.0214 16.1385 15.7729 16.559 15.0508L17.3013 13.7762C17.7124 13.0704 17.4651 12.1699 16.7502 11.7591C15.7547 11.1871 15.7526 9.77146 16.7501 9.20285Z" | ||
| stroke="currentcolor" | ||
| strokeWidth="1.3" | ||
| /> | ||
| <path | ||
| d="M12.68 10.3947C12.68 11.8481 11.4794 13.0263 9.99834 13.0263C8.5173 13.0263 7.31668 11.8481 7.31668 10.3947C7.31668 8.94136 8.5173 7.76316 9.99834 7.76316C11.4794 7.76316 12.68 8.94136 12.68 10.3947Z" | ||
| stroke="currentcolor" | ||
| strokeWidth="1.3" | ||
| /> | ||
| </svg> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility attributes to settings icon
The settings icon SVG needs accessibility improvements:
<button className="hidden sm:flex p-3 text-gray-500 items-center justify-center transition-all duration-300 hover:text-gray-900">
<svg
xmlns="http://www.w3.org/2000/svg"
width={20}
height={20}
viewBox="0 0 20 20"
fill="none"
+ role="img"
+ aria-label="Settings"
>
+ <title>Settings</title>
<path📝 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.
| <button className="hidden sm:flex p-3 text-gray-500 items-center justify-center transition-all duration-300 hover:text-gray-900"> | |
| <svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| width={20} | |
| height={20} | |
| viewBox="0 0 20 20" | |
| fill="none" | |
| > | |
| <path | |
| d="M16.7501 9.20285C17.4658 8.79486 17.7119 7.89657 17.3012 7.19137L16.559 5.91699C16.1385 5.19489 15.1995 4.94638 14.466 5.36301C13.4469 5.94185 12.1716 5.19438 12.1716 4.03885C12.1716 3.19732 11.4764 2.5 10.6188 2.5H9.17632C8.33949 2.5 7.66111 3.16571 7.66111 3.9869V4.09222C7.66111 5.19157 6.44713 5.8776 5.4782 5.3258C4.78224 4.92946 3.89057 5.16345 3.48911 5.84779L2.70103 7.19113C2.28763 7.89582 2.5327 8.79589 3.24919 9.20432C4.24554 9.77228 4.24337 11.1862 3.24904 11.7576C2.53341 12.1688 2.28715 13.0712 2.7009 13.7764L3.48911 15.12C3.89057 15.8043 4.78478 16.0369 5.48074 15.6406C6.4466 15.0905 7.66111 15.7719 7.66111 16.8677C7.66111 17.6529 8.30976 18.2895 9.10991 18.2895H10.6853C11.5061 18.2895 12.1716 17.6365 12.1716 16.831C12.1716 15.7075 13.4115 15.0058 14.4024 15.5686L14.466 15.6048C15.1995 16.0214 16.1385 15.7729 16.559 15.0508L17.3013 13.7762C17.7124 13.0704 17.4651 12.1699 16.7502 11.7591C15.7547 11.1871 15.7526 9.77146 16.7501 9.20285Z" | |
| stroke="currentcolor" | |
| strokeWidth="1.3" | |
| /> | |
| <path | |
| d="M12.68 10.3947C12.68 11.8481 11.4794 13.0263 9.99834 13.0263C8.5173 13.0263 7.31668 11.8481 7.31668 10.3947C7.31668 8.94136 8.5173 7.76316 9.99834 7.76316C11.4794 7.76316 12.68 8.94136 12.68 10.3947Z" | |
| stroke="currentcolor" | |
| strokeWidth="1.3" | |
| /> | |
| </svg> | |
| <button className="hidden sm:flex p-3 text-gray-500 items-center justify-center transition-all duration-300 hover:text-gray-900"> | |
| <svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| width={20} | |
| height={20} | |
| viewBox="0 0 20 20" | |
| fill="none" | |
| role="img" | |
| aria-label="Settings" | |
| > | |
| <title>Settings</title> | |
| <path | |
| d="M16.7501 9.20285C17.4658 8.79486 17.7119 7.89657 17.3012 7.19137L16.559 5.91699C16.1385 5.19489 15.1995 4.94638 14.466 5.36301C13.4469 5.94185 12.1716 5.19438 12.1716 4.03885C12.1716 3.19732 11.4764 2.5 10.6188 2.5H9.17632C8.33949 2.5 7.66111 3.16571 7.66111 3.9869V4.09222C7.66111 5.19157 6.44713 5.8776 5.4782 5.3258C4.78224 4.92946 3.89057 5.16345 3.48911 5.84779L2.70103 7.19113C2.28763 7.89582 2.5327 8.79589 3.24919 9.20432C4.24554 9.77228 4.24337 11.1862 3.24904 11.7576C2.53341 12.1688 2.28715 13.0712 2.7009 13.7764L3.48911 15.12C3.89057 15.8043 4.78478 16.0369 5.48074 15.6406C6.4466 15.0905 7.66111 15.7719 7.66111 16.8677C7.66111 17.6529 8.30976 18.2895 9.10991 18.2895H10.6853C11.5061 18.2895 12.1716 17.6365 12.1716 16.831C12.1716 15.7075 13.4115 15.0058 14.4024 15.5686L14.466 15.6048C15.1995 16.0214 16.1385 15.7729 16.559 15.0508L17.3013 13.7762C17.7124 13.0704 17.4651 12.1699 16.7502 11.7591C15.7547 11.1871 15.7526 9.77146 16.7501 9.20285Z" | |
| stroke="currentcolor" | |
| strokeWidth="1.3" | |
| /> | |
| <path | |
| d="M12.68 10.3947C12.68 11.8481 11.4794 13.0263 9.99834 13.0263C8.5173 13.0263 7.31668 11.8481 7.31668 10.3947C7.31668 8.94136 8.5173 7.76316 9.99834 7.76316C11.4794 7.76316 12.68 8.94136 12.68 10.3947Z" | |
| stroke="currentcolor" | |
| strokeWidth="1.3" | |
| /> | |
| </svg> |
🧰 Tools
🪛 Biome
[error] 95-101: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
[error] 94-94: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
|
@nirajacharyaa can you resolve the conflict so that I can merge it? |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
apps/vite/src/components/calendar/EventList.tsx (2)
1-3: Useimport typefor type-only imports.The imports are only used for types. Using
import typehelps the compiler remove unnecessary modules.Apply this diff:
-import React from "react" -import { EventDetail, NewCalendarData } from "@miti/types" +import type React from "react" +import type { EventDetail, NewCalendarData } from "@miti/types"🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
22-35: Simplify event processing and add null checks.The nested loops can be simplified using array methods, and null checks should be added for safer code.
Apply this diff:
- data.forEach((day) => { - if (day.eventDetails.length > 0) { - day.eventDetails.forEach((event: EventDetail) => { - newEventDetails.push({ - date: day.calendarInfo.dates.bs.day.np ?? "", - enDate: day.calendarInfo.dates.ad.full.en ?? "", - isHoliday: event.isHoliday, - day: day.calendarInfo.days.dayOfWeek.np ?? "", - title: event.title.np ?? "", - fullDate: day.calendarInfo.dates.bs.full.np ?? "", - }) - }) - } - }) + const newEventDetails = data + .filter(day => day.eventDetails?.length > 0) + .flatMap(day => + day.eventDetails.map((event: EventDetail) => ({ + date: day.calendarInfo?.dates?.bs?.day?.np ?? "", + enDate: day.calendarInfo?.dates?.ad?.full?.en ?? "", + isHoliday: event.isHoliday ?? false, + day: day.calendarInfo?.days?.dayOfWeek?.np ?? "", + title: event.title?.np ?? "", + fullDate: day.calendarInfo?.dates?.bs?.full?.np ?? "", + })) + )apps/vite/src/components/calendar/TimelineView.tsx (1)
16-29: Optimize Day component implementation.Several improvements can be made to the Day component:
- Replace array indices with stable keys
- Use template literals
- Simplify time formatting
Apply this diff:
+const formatTime = (hour: number) => { + if (hour === 0) return "12:00 AM" + if (hour === 12) return "12:00 PM" + return `${hour % 12}:00 ${hour < 12 ? 'AM' : 'PM'}` +} {Array.from({ length: 24 }).map((_, i) => ( <div - key={i} + key={`time-${i}`} className="h-[120px] bg-gray-200 flex items-center justify-center px-2 text-xs font-semibold" > - {i === 0 - ? "12:00 AM" - : i < 12 - ? `${i}:00 AM` - : i === 12 - ? "12:00 PM" - : `${i - 12}:00 PM`} + {formatTime(i)} </div> ))} {Array.from({ length: 24 }).map((_, i) => ( <div - key={i} + key={`timeline-${i}`} className="h-[120px] flex flex-1 items-center justify-center px-2 text-sm font-semibold" >Also applies to: 32-61
🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.(lint/suspicious/noArrayIndexKey)
apps/vite/src/pages/Calendar2.tsx (3)
17-17: Useimport typefor type-only imports.The
NewCalendarDataimport is only used as a type. To optimize bundle size, useimport type.-import { NewCalendarData } from "@miti/types" +import type { NewCalendarData } from "@miti/types"🧰 Tools
🪛 Biome (1.9.4)
[error] 17-17: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
62-63: Handle loading state for nextMonthData.The
nextMonthDataLoadingstate is fetched but never used in the UI. Add loading indicators wherenextMonthDatais used.<EventList data={combinedData} title="आगामी इभेन्टहरु" + isLoading={nextMonthDataLoading} />
65-68: Add data validation in combinedData.The current implementation doesn't validate that the combined data is consistent.
const combinedData = useMemo(() => { - if (!nextMonthData || !calendarData) return [] + if (!Array.isArray(nextMonthData) || !Array.isArray(calendarData)) return [] + + // Validate month sequence + const currentMonth = calendarData[0]?.month + const nextMonth = nextMonthData[0]?.month + if (currentMonth !== undefined && nextMonth !== undefined && nextMonth !== currentMonth + 1) { + console.warn('Month sequence mismatch in calendar data') + return calendarData + } + return [...calendarData, ...nextMonthData] }, [calendarData, nextMonthData])apps/vite/src/components/calendar/CalendarGrid.tsx (3)
1-2: Useimport typefor type-only imports.Both imports are only used as types. To optimize bundle size:
-import { NewCalendarData } from "@miti/types" -import React, { useState } from "react" +import type { NewCalendarData } from "@miti/types" +import type React from "react" +import { useState } from "react"🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
[error] 2-2: The default import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
64-79: Add explicit button type.Buttons without an explicit type default to "submit", which can cause unintended form submissions.
<button + type="button" key={day.calendarInfo.dates.bs.day.np} className={cn(🧰 Tools
🪛 Biome (1.9.4)
[error] 74-74: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 64-79: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
117-123: Pass children as JSX elements.Instead of using the children prop, pass children between the component's opening and closing tags.
{dayDialogData && ( <DayDialog open={dayDialogOpen} setOpen={setDayDialogOpen} - children={<DayDetail dayData={dayDialogData} />} - /> + > + <DayDetail dayData={dayDialogData} /> + </DayDialog> )}🧰 Tools
🪛 Biome (1.9.4)
[error] 121-121: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/api/package.json(1 hunks)apps/vite/src/components/calendar/CalendarGrid.tsx(2 hunks)apps/vite/src/components/calendar/EventList.tsx(1 hunks)apps/vite/src/components/calendar/Panchang.tsx(2 hunks)apps/vite/src/components/calendar/TimelineView.tsx(1 hunks)apps/vite/src/pages/Calendar2.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/package.json
🧰 Additional context used
🪛 Biome (1.9.4)
apps/vite/src/pages/Calendar2.tsx
[error] 17-17: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 28-28: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
[error] 29-29: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.
(lint/style/useNumberNamespace)
apps/vite/src/components/calendar/EventList.tsx
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 52-52: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
[error] 64-64: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
apps/vite/src/components/calendar/CalendarGrid.tsx
[error] 74-74: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 2-2: The default import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 64-79: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 121-121: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
apps/vite/src/components/calendar/TimelineView.tsx
[error] 42-42: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 46-46: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 47-47: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 50-57: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 18-18: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
[error] 34-34: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
[error] 84-84: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 92-92: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
apps/vite/src/components/calendar/Panchang.tsx
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 31-31: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
[error] 121-121: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: miti-web
🔇 Additional comments (10)
apps/vite/src/components/calendar/Panchang.tsx (5)
3-3: Useimport typefor type-only importsSince
NewCalendarDatais only used as a type, usingimport typeensures that it is removed during compilation.-import { NewCalendarData } from "@miti/types" +import type { NewCalendarData } from "@miti/types"🧰 Tools
🪛 Biome (1.9.4)
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
31-31: Remove unnecessary template literalThe template literal in
classNameis not needed since there's no string interpolation.- <div className={`my-8`}> + <div className="my-8">🧰 Tools
🪛 Biome (1.9.4)
[error] 31-31: Do not use template literals if interpolation and special-character handling are not needed.
Unsafe fix: Replace with string literal
(lint/style/noUnusedTemplateLiteral)
38-45: LGTM! Good type safety improvementMaking the time parameter optional improves type safety and component flexibility.
129-134:⚠️ Potential issueAdd key prop and improve null safety
The mapping of muhurats needs a key prop and could benefit from better null safety handling.
- ? data.auspiciousMoments.muhurats.map((muhurat) => ( - <MuhuratItem - name={muhurat.periodName ?? ""} - time={muhurat.duration ?? ""} - /> + ? data.auspiciousMoments.muhurats.map((muhurat, index) => ( + <MuhuratItem + key={muhurat.periodName ?? index} + name={muhurat.periodName ?? ""} + time={muhurat.duration ?? ""} + />Likely invalid or redundant comment.
113-115:⚠️ Potential issueAdd key prop to mapped elements
When mapping over arrays to create React elements, each element needs a unique key prop.
- ? data.auspiciousMoments.sahits.map((sahit) => ( - <MuhuratItem name={sahit.title.np ?? ""} /> + ? data.auspiciousMoments.sahits.map((sahit, index) => ( + <MuhuratItem + key={sahit.title.np ?? index} + name={sahit.title.np ?? ""} + />Likely invalid or redundant comment.
apps/vite/src/components/calendar/TimelineView.tsx (1)
112-124: 🛠️ Refactor suggestionAdd error handling for invalid scope values.
The switch statement should handle invalid scope values to prevent potential issues.
Apply this diff:
const generateTimeline = ( scope: TimelineViewProps["scope"], monthData: TimelineViewProps["monthData"] ) => { + if (!monthData?.length) { + return null; + } + switch (scope) { case "day": return <Day monthData={monthData} /> case "week": return <Week monthData={monthData} /> case "month": return <CalendarGrid monthData={monthData} timelineMode /> + default: + console.error(`Invalid scope: ${scope}`); + return null; } }Likely invalid or redundant comment.
apps/vite/src/pages/Calendar2.tsx (3)
39-47: 🛠️ Refactor suggestionUse
useNavigatehook instead of directly modifying history.Direct manipulation of browser history is not recommended in React applications. Use the
useNavigatehook fromreact-router-domfor better integration with React Router.+import { useNavigate } from "react-router-dom" +const navigate = useNavigate() useEffect(() => { - history.replaceState( - null, - "", - `/calendar/${currentNepaliDate.getYear()}/${ - currentNepaliDate.getMonth() + 1 - }` - ) + navigate( + `/calendar/${currentNepaliDate.getYear()}/${ + currentNepaliDate.getMonth() + 1 + }`, + { replace: true } + ) }, [currentNepaliDate])Likely invalid or redundant comment.
53-56: 🛠️ Refactor suggestionRemove unnecessary type casting.
The double type casting suggests a type mismatch. Instead, ensure that
useCalendarDatahook returns the correct type.const monthData = useMemo(() => { if (!calendarData) return [] return calendarData -}, [calendarData, currentMonth]) as unknown as NewCalendarData[] +}, [calendarData, currentMonth])Likely invalid or redundant comment.
26-34: 🛠️ Refactor suggestionImprove date validation and error handling.
Several improvements can be made to the date validation logic:
- Use
Number.parseIntinstead of globalparseInt- Document or extract the year range constants
- Add user notification for invalid dates
const validYearAndMonth = useMemo(() => { if (!BSYear || !BSMonth) return new NepaliDate() - const year = parseInt(BSYear) - const month = parseInt(BSMonth) - const isValid = year >= 2075 && year <= 2082 && month >= 1 && month <= 12 + const year = Number.parseInt(BSYear) + const month = Number.parseInt(BSMonth) + const MIN_YEAR = 2075 + const MAX_YEAR = 2082 + + const isValid = year >= MIN_YEAR && year <= MAX_YEAR && month >= 1 && month <= 12 + + if (!isValid) { + console.warn(`Invalid date: year must be between ${MIN_YEAR}-${MAX_YEAR}, month between 1-12`) + // Consider showing user feedback here + } if (isValid) return new NepaliDate(year, month - 1, 1) return new NepaliDate() }, [BSYear, BSMonth])Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 28-28: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.(lint/style/useNumberNamespace)
[error] 29-29: Use Number.parseInt instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.parseInt instead.(lint/style/useNumberNamespace)
apps/vite/src/components/calendar/CalendarGrid.tsx (1)
74-74: 🛠️ Refactor suggestionReplace non-null assertion with safe fallback.
Using the non-null assertion operator (
!) can lead to runtime errors. Provide a fallback value instead.- gridColumnStart: day.calendarInfo.days.codes.en!, + gridColumnStart: day.calendarInfo.days.codes.en ?? 1,Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 74-74: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
| <PanchangTableRow | ||
| label="तारिख" | ||
| value={new NepaliDate(data.calendarInfo.dates.bs.full.en ?? "") | ||
| .getDateObject() | ||
| .toLocaleDateString("en-US", { | ||
| month: "long", | ||
| day: "numeric", | ||
| year: "numeric", | ||
| })} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add fallback for date formatting
The current implementation might throw an error if the date string is undefined. Consider adding a fallback.
- value={new NepaliDate(data.calendarInfo.dates.bs.full.en ?? "")
+ value={data.calendarInfo.dates.bs.full.en
+ ? new NepaliDate(data.calendarInfo.dates.bs.full.en)
.getDateObject()
.toLocaleDateString("en-US", {
month: "long",
day: "numeric",
year: "numeric",
- })}
+ })
+ : "-"}📝 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.
| <PanchangTableRow | |
| label="तारिख" | |
| value={new NepaliDate(data.calendarInfo.dates.bs.full.en ?? "") | |
| .getDateObject() | |
| .toLocaleDateString("en-US", { | |
| month: "long", | |
| day: "numeric", | |
| year: "numeric", | |
| })} | |
| /> | |
| <PanchangTableRow | |
| label="तारिख" | |
| value={data.calendarInfo.dates.bs.full.en | |
| ? new NepaliDate(data.calendarInfo.dates.bs.full.en) | |
| .getDateObject() | |
| .toLocaleDateString("en-US", { | |
| month: "long", | |
| day: "numeric", | |
| year: "numeric", | |
| }) | |
| : "-"} | |
| /> |
| {isHoliday && ( | ||
| <> | ||
| {newEventDetails | ||
| .filter( | ||
| (event) => | ||
| event.isHoliday && | ||
| !isBefore(new Date(event.enDate), new Date()) | ||
| ) | ||
| .map((event, index) => ( | ||
| <UpcomingEvent key={index} event={event} isHoliday /> | ||
| ))} | ||
| </> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize event filtering and fix React key usage.
The event filtering logic is duplicated, and using array indices as keys can cause rendering issues.
Apply this diff:
+const filterEvents = (events: Event[], isHoliday: boolean) =>
+ events.filter(event =>
+ event.isHoliday === isHoliday &&
+ !isBefore(new Date(event.enDate), new Date())
+ );
{isHoliday && (
<>
- {newEventDetails
- .filter(
- (event) =>
- event.isHoliday &&
- !isBefore(new Date(event.enDate), new Date())
- )
+ {filterEvents(newEventDetails, true)
.map((event) => (
- <UpcomingEvent key={index} event={event} isHoliday />
+ <UpcomingEvent key={`${event.date}-${event.title}`} event={event} isHoliday />
))}
</>
)}
{!isHoliday &&
- newEventDetails
- .filter(
- (event) =>
- !event.isHoliday &&
- !isBefore(new Date(event.enDate), new Date())
- )
+ filterEvents(newEventDetails, false)
.map((event) => (
- <UpcomingEvent key={index} event={event} />
+ <UpcomingEvent key={`${event.date}-${event.title}`} event={event} />
))}Also applies to: 57-64
🧰 Tools
🪛 Biome (1.9.4)
[error] 52-52: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
| const weekData = monthData.filter((data) => { | ||
| return ( | ||
| Number.parseInt(data.calendarInfo.dates.bs.day.en ?? "") === | ||
| currentNepaliDate || | ||
| (Number.parseInt(data.calendarInfo.dates.bs.day.en ?? "") >= | ||
| currentNepaliDate - 3 && | ||
| Number.parseInt(data.calendarInfo.dates.bs.day.en ?? "") <= | ||
| currentNepaliDate + 3) | ||
| ) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve Week component implementation.
The component needs several improvements:
- Simplify date filtering logic
- Add missing React keys
- Add error handling for date parsing
Apply this diff:
+const isDateInCurrentWeek = (dateStr: string | undefined, currentDate: number) => {
+ if (!dateStr) return false;
+ const date = Number.parseInt(dateStr);
+ return !isNaN(date) && (date === currentDate || (date >= currentDate - 3 && date <= currentDate + 3));
+}
-const weekData = monthData.filter((data) => {
- return (
- Number.parseInt(data.calendarInfo.dates.bs.day.en ?? "") ===
- currentNepaliDate ||
- (Number.parseInt(data.calendarInfo.dates.bs.day.en ?? "") >=
- currentNepaliDate - 3 &&
- Number.parseInt(data.calendarInfo.dates.bs.day.en ?? "") <=
- currentNepaliDate + 3)
- )
-})
+const weekData = monthData.filter(data =>
+ isDateInCurrentWeek(data.calendarInfo.dates.bs.day.en, currentNepaliDate)
+)
{weekData.map((data) => (
- <div className="h-[120px] flex items-center">
+ <div key={data.calendarInfo.dates.bs.day.en} className="h-[120px] flex items-center">
{data.eventDetails.map((event) => {
return (
- <div className="px-2 flex flex-col items-center justify-center h-full w-full">
+ <div key={event.title.np} className="px-2 flex flex-col items-center justify-center h-full w-full">Also applies to: 83-99
Summary by CodeRabbit
Release Notes
New Features
DayDialog,DayDetail,TimelineView, andTodayEventList.useCalendarData,useTodayData,useNextMonthData).Improvements
Bug Fixes
Documentation