-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: reschedule calendar modal on inbox #518
Changes from 1 commit
de6281f
9ddd039
b7a5bee
dd326f0
3393e4a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,15 +2,10 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import React, { useEffect, useCallback, useState } from "react" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
CalendarIcon, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
MoveIcon, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GithubIcon, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
MailsIcon, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
XIcon, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} from "lucide-react" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { CalendarIcon, MoveIcon, GithubIcon, MailsIcon } from "lucide-react" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import Image from "next/image" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { RescheduleCalendar } from "./RescheduleCalendar/RescheduleCalendar" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import BoxIcon from "@/public/icons/box.svg" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import LinearIcon from "@/public/icons/linear.svg" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { useAuth } from "@/src/contexts/AuthContext" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -23,6 +18,12 @@ export const InboxItems: React.FC = () => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const { session } = useAuth() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [isControlHeld, setIsControlHeld] = useState(false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [dateChanged, setDateChanged] = useState(false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [updateInbox, setUpdateInbox] = useState(false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [reschedulingItemId, setReschedulingItemId] = useState<string | null>( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
null | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [date, setDate] = React.useState<Date | string | null>(new Date()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const { inbox, currentItem, setCurrentItem, fetchInbox, updateItem, error } = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
useCycleItemStore() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -54,6 +55,19 @@ export const InboxItems: React.FC = () => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, []) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (dateChanged && reschedulingItemId && date) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
updateItem(session, { dueDate: date }, reschedulingItemId) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setReschedulingItemId(null) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setDateChanged(false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setUpdateInbox(!updateInbox) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, [date, updateItem, session, reschedulingItemId, dateChanged, updateInbox]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fetchInbox(session) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, [updateInbox, fetchInbox, session]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const handleExpand = useCallback( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
(item: CycleItem) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (isControlHeld && item.type === "link") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -121,6 +135,17 @@ export const InboxItems: React.FC = () => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const filteredItems = items.filter((item) => item.status !== "done") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const handleRescheduleCalendar = ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
e: React.MouseEvent, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
id: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
dueDate: Date | string | null | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
e.stopPropagation() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setReschedulingItemId(id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setDate(dueDate) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance type safety and error handling in handleRescheduleCalendar While the function works, it could benefit from improved type safety and validation. Consider this enhancement: const handleRescheduleCalendar = (
e: React.MouseEvent,
id: string,
- dueDate: Date | string | null
+ dueDate: Date | string | null,
) => {
e.stopPropagation()
+
+ // Ensure string dates are parsed correctly
+ const parsedDate = dueDate instanceof Date
+ ? dueDate
+ : dueDate
+ ? new Date(dueDate)
+ : new Date()
+
+ // Validate the parsed date
+ if (isNaN(parsedDate.getTime())) {
+ console.error('Invalid date provided')
+ return
+ }
setReschedulingItemId(id)
- setDate(dueDate)
+ setDate(parsedDate)
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div className="no-scrollbar flex h-full flex-col gap-2 overflow-y-auto"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{filteredItems.length === 0 ? ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -172,7 +197,9 @@ export const InboxItems: React.FC = () => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<CalendarIcon | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
size={14} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
className="hover-text" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
onClick={(e) => e.stopPropagation()} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
onClick={(e) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
handleRescheduleCalendar(e, item._id, item.dueDate) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+194
to
+196
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider memoizing the click handler for better performance. The inline arrow function creates a new function instance on each render. Consider memoizing it with useCallback. + const handleCalendarClick = useCallback(
+ (id: string, dueDate: Date | string | null) => (e: React.MouseEvent) => {
+ handleRescheduleCalendar(e, id, dueDate)
+ },
+ [handleRescheduleCalendar]
+ )
// In JSX:
- onClick={(e) => handleRescheduleCalendar(e, item._id, item.dueDate)}
+ onClick={handleCalendarClick(item._id, item.dueDate)}
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<MoveIcon | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
size={14} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -185,6 +212,30 @@ export const InboxItems: React.FC = () => { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{reschedulingItemId !== null && ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
className="fixed inset-0 z-50 cursor-default bg-black/80" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
role="button" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
onClick={() => setReschedulingItemId(null)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
onKeyDown={(e) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (e.key === "Escape" || e.key === "Esc") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setReschedulingItemId(null) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
tabIndex={0} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
></div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<div className="fixed left-1/2 top-1/2 z-50 -translate-x-1/2 -translate-y-1/2 rounded-md bg-background shadow-lg"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<RescheduleCalendar | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
date={date} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setDate={setDate} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
dateChanged={dateChanged} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
setDateChanged={setDateChanged} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
)} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+210
to
+232
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enhance modal accessibility The modal implementation needs accessibility improvements for better keyboard navigation and screen reader support. Consider these accessibility enhancements: {reschedulingItemId !== null && (
- <div>
+ <div role="dialog" aria-modal="true" aria-label="Reschedule Item">
<div
className="fixed inset-0 z-50 cursor-default bg-black/80"
role="button"
+ aria-label="Close modal"
onClick={() => setReschedulingItemId(null)}
onKeyDown={(e) => {
if (e.key === "Escape" || e.key === "Esc") {
setReschedulingItemId(null)
}
}}
tabIndex={0}
/>
- <div className="fixed left-1/2 top-1/2 z-50 -translate-x-1/2 -translate-y-1/2 rounded-md bg-background shadow-lg">
+ <div
+ className="fixed left-1/2 top-1/2 z-50 -translate-x-1/2 -translate-y-1/2 rounded-md bg-background shadow-lg"
+ role="document"
+ >
<RescheduleCalendar
date={date}
setDate={setDate}
dateChanged={dateChanged}
setDateChanged={setDateChanged}
/>
</div>
</div>
)} Consider also:
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,71 @@ | ||||||||||||||||||||||||||
"use client" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
import React from "react" | ||||||||||||||||||||||||||
import { DayPicker } from "react-day-picker" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
import { ChevronLeftIcon, ChevronRightIcon } from "@radix-ui/react-icons" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
import { cn } from "@/lib/utils" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
export type CalendarProps = React.ComponentProps<typeof DayPicker> | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
function Calendar({ | ||||||||||||||||||||||||||
className, | ||||||||||||||||||||||||||
classNames, | ||||||||||||||||||||||||||
showOutsideDays = true, | ||||||||||||||||||||||||||
...props | ||||||||||||||||||||||||||
}: CalendarProps) { | ||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||
<DayPicker | ||||||||||||||||||||||||||
showOutsideDays={showOutsideDays} | ||||||||||||||||||||||||||
className={cn("p-3", className)} | ||||||||||||||||||||||||||
classNames={{ | ||||||||||||||||||||||||||
months: "flex flex-col sm:flex-row space-y-4 sm:space-x-4 sm:space-y-0", | ||||||||||||||||||||||||||
month: "space-y-4 text-primary-foreground", | ||||||||||||||||||||||||||
caption: "flex justify-center pt-1 relative items-center", | ||||||||||||||||||||||||||
caption_label: "text-sm font-medium", | ||||||||||||||||||||||||||
nav: "space-x-1 flex items-center", | ||||||||||||||||||||||||||
nav_button: cn( | ||||||||||||||||||||||||||
"h-7 w-7 text-center bg-transparent p-1 text-secondary-foreground hover-text" | ||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||
nav_button_previous: "absolute left-1", | ||||||||||||||||||||||||||
nav_button_next: "absolute right-1", | ||||||||||||||||||||||||||
table: "w-full border-collapse space-y-1", | ||||||||||||||||||||||||||
head_row: "flex", | ||||||||||||||||||||||||||
head_cell: | ||||||||||||||||||||||||||
"text-secondary-foreground rounded-md w-8 font-normal text-[0.8rem]", | ||||||||||||||||||||||||||
row: "flex w-full mt-2", | ||||||||||||||||||||||||||
cell: cn( | ||||||||||||||||||||||||||
"relative p-0 text-center text-sm focus-within:relative focus-within:z-20 [&:has([aria-selected])]:bg-accent [&:has([aria-selected].day-outside)]:bg-accent/50 [&:has([aria-selected].day-range-end)]:rounded-r-md", | ||||||||||||||||||||||||||
props.mode === "range" | ||||||||||||||||||||||||||
? "[&:has(>.day-range-end)]:rounded-r-md [&:has(>.day-range-start)]:rounded-l-md first:[&:has([aria-selected])]:rounded-l-md last:[&:has([aria-selected])]:rounded-r-md" | ||||||||||||||||||||||||||
: "[&:has([aria-selected])]:rounded-md" | ||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||
day: cn( | ||||||||||||||||||||||||||
"h-8 w-8 p-0 font-normal aria-selected:opacity-100 text-secondary-foreground hover-bg hover-text rounded-lg" | ||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||
day_range_start: "day-range-start", | ||||||||||||||||||||||||||
day_range_end: "day-range-end", | ||||||||||||||||||||||||||
day_selected: | ||||||||||||||||||||||||||
"bg-background-hover border border-border !text-foreground", | ||||||||||||||||||||||||||
day_today: "bg-background-hover text-foreground", | ||||||||||||||||||||||||||
day_outside: | ||||||||||||||||||||||||||
"day-outside text-muted-foreground opacity-50 aria-selected:bg-accent/50 aria-selected:text-muted-foreground aria-selected:opacity-30", | ||||||||||||||||||||||||||
day_disabled: "text-muted-foreground opacity-50", | ||||||||||||||||||||||||||
day_range_middle: | ||||||||||||||||||||||||||
"aria-selected:bg-accent aria-selected:text-accent-foreground", | ||||||||||||||||||||||||||
day_hidden: "invisible", | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
...classNames, | ||||||||||||||||||||||||||
}} | ||||||||||||||||||||||||||
Comment on lines
+19
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider extracting class names and enhancing accessibility. While the styling implementation is comprehensive, consider these improvements:
Example refactor: const CALENDAR_CLASSES = {
BASE: "p-3",
MONTHS: "flex flex-col sm:flex-row space-y-4 sm:space-x-4 sm:space-y-0",
// ... other class definitions
} as const;
// In the component
<DayPicker
showOutsideDays={showOutsideDays}
className={cn(CALENDAR_CLASSES.BASE, className)}
aria-label="Date picker calendar"
// ... other props
/> |
||||||||||||||||||||||||||
components={{ | ||||||||||||||||||||||||||
IconLeft: ({ ...props }) => <ChevronLeftIcon className="size-4" />, | ||||||||||||||||||||||||||
IconRight: ({ ...props }) => <ChevronRightIcon className="size-4" />, | ||||||||||||||||||||||||||
}} | ||||||||||||||||||||||||||
Comment on lines
+61
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enhance navigation accessibility and remove unused props. The navigation implementation can be improved:
Apply this diff: components={{
- IconLeft: ({ ...props }) => <ChevronLeftIcon className="size-4" />,
- IconRight: ({ ...props }) => <ChevronRightIcon className="size-4" />,
+ IconLeft: () => (
+ <ChevronLeftIcon className="size-4" aria-label="Previous month" />
+ ),
+ IconRight: () => (
+ <ChevronRightIcon className="size-4" aria-label="Next month" />
+ ),
}} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
{...props} | ||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
Calendar.displayName = "Calendar" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
export { Calendar } |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
export default function DateSelectionButton({ | ||
label, | ||
icon: Icon, | ||
formattedDate, | ||
onClick, | ||
}: { | ||
label: string | ||
icon: React.ElementType | ||
formattedDate: string | ||
onClick: () => void | ||
}) { | ||
return ( | ||
<button | ||
className="hover-bg hover-text flex w-full cursor-pointer items-center justify-between gap-4 rounded-lg p-2" | ||
onClick={onClick} | ||
> | ||
<span className="flex items-center gap-2"> | ||
<Icon size={24} /> {label} | ||
</span> | ||
<span className="text-sm">{formattedDate}</span> | ||
</button> | ||
Comment on lines
+13
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance accessibility and keyboard interaction. While the basic implementation is solid, consider these improvements for better accessibility and user experience: <button
className="hover-bg hover-text flex w-full cursor-pointer items-center justify-between gap-4 rounded-lg p-2"
onClick={onClick}
+ type="button"
+ aria-label={`Schedule for ${formattedDate}`}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ onClick();
+ }
+ }}
> Also consider making the icon size configurable through props instead of hardcoding it to 24: - <Icon size={24} /> {label}
+ <Icon size={iconSize ?? 24} /> {label} And update the props interface: }: {
label: string
icon: React.ElementType
formattedDate: string
onClick: () => void
+ iconSize?: number
})
|
||
) | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,110 @@ | ||||||||||||||||||||||||||||||||||||||||||||
"use client" | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
import * as React from "react" | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
import { CalendarDot, CalendarDots, CalendarX } from "@phosphor-icons/react" | ||||||||||||||||||||||||||||||||||||||||||||
import { addDays, format } from "date-fns" | ||||||||||||||||||||||||||||||||||||||||||||
import { Calendar as CalendarIcon, SquareChevronRightIcon } from "lucide-react" | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
import { Calendar } from "@/src/components/Inbox/RescheduleCalendar/Calendar" | ||||||||||||||||||||||||||||||||||||||||||||
import DateSelectionButton from "@/src/components/Inbox/RescheduleCalendar/DateSelectionButton" | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
interface RescheduleCalendarProps { | ||||||||||||||||||||||||||||||||||||||||||||
date: Date | null | ||||||||||||||||||||||||||||||||||||||||||||
setDate: (date: Date | null) => void | ||||||||||||||||||||||||||||||||||||||||||||
dateChanged: boolean | ||||||||||||||||||||||||||||||||||||||||||||
setDateChanged: (state: boolean) => void | ||||||||||||||||||||||||||||||||||||||||||||
icon?: React.ReactNode | ||||||||||||||||||||||||||||||||||||||||||||
[key: string]: any | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+12
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve type safety of the RescheduleCalendarProps interface. The interface has two potential issues:
Consider this improved interface definition: interface RescheduleCalendarProps {
date: Date | null
setDate: (date: Date | null) => void
dateChanged: boolean
setDateChanged: (state: boolean) => void
- icon?: React.ReactNode
- [key: string]: any
+ className?: string // Add specific props you need instead of [key: string]: any
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
export function RescheduleCalendar({ | ||||||||||||||||||||||||||||||||||||||||||||
date, | ||||||||||||||||||||||||||||||||||||||||||||
setDate, | ||||||||||||||||||||||||||||||||||||||||||||
dateChanged, | ||||||||||||||||||||||||||||||||||||||||||||
setDateChanged, | ||||||||||||||||||||||||||||||||||||||||||||
...props | ||||||||||||||||||||||||||||||||||||||||||||
}: RescheduleCalendarProps) { | ||||||||||||||||||||||||||||||||||||||||||||
const [noDate, setNoDateFlag] = React.useState<boolean>(false) | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+21
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Optimize state management by removing redundant state. The Replace the state with a computed value: - const [noDate, setNoDateFlag] = React.useState<boolean>(false)
+ const noDate = date === null Then update the setter functions to remove
|
||||||||||||||||||||||||||||||||||||||||||||
const setToday = () => { | ||||||||||||||||||||||||||||||||||||||||||||
setNoDateFlag(false) | ||||||||||||||||||||||||||||||||||||||||||||
setDate(new Date()) | ||||||||||||||||||||||||||||||||||||||||||||
setDateChanged(true) | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
const setTomorrow = () => { | ||||||||||||||||||||||||||||||||||||||||||||
setNoDateFlag(false) | ||||||||||||||||||||||||||||||||||||||||||||
setDate(addDays(new Date(), 1)) | ||||||||||||||||||||||||||||||||||||||||||||
setDateChanged(true) | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
const setNextWeek = () => { | ||||||||||||||||||||||||||||||||||||||||||||
setNoDateFlag(false) | ||||||||||||||||||||||||||||||||||||||||||||
setDate(addDays(new Date(), 7)) | ||||||||||||||||||||||||||||||||||||||||||||
setDateChanged(true) | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
const setNoDate = () => { | ||||||||||||||||||||||||||||||||||||||||||||
setNoDateFlag(true) | ||||||||||||||||||||||||||||||||||||||||||||
setDate(null) | ||||||||||||||||||||||||||||||||||||||||||||
setDateChanged(true) | ||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
const todayFormatted = format(new Date(), "eee") | ||||||||||||||||||||||||||||||||||||||||||||
const tomorrowFormatted = format(addDays(new Date(), 1), "eee") | ||||||||||||||||||||||||||||||||||||||||||||
const nextWeekFormatted = format(addDays(new Date(), 7), "eee MMM dd") | ||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
const dueDateFormatted = noDate | ||||||||||||||||||||||||||||||||||||||||||||
? "No Date Assigned" | ||||||||||||||||||||||||||||||||||||||||||||
: date | ||||||||||||||||||||||||||||||||||||||||||||
? format(date, "eee MMM dd") | ||||||||||||||||||||||||||||||||||||||||||||
: "No Date Assigned" | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+51
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve date formatting maintainability. The date format strings should be constants, and the due date formatting logic can be simplified. Consider these improvements: + const DATE_FORMATS = {
+ dayOfWeek: "eee",
+ fullDate: "eee MMM dd"
+ } as const
+
- const todayFormatted = format(new Date(), "eee")
- const tomorrowFormatted = format(addDays(new Date(), 1), "eee")
- const nextWeekFormatted = format(addDays(new Date(), 7), "eee MMM dd")
+ const todayFormatted = format(new Date(), DATE_FORMATS.dayOfWeek)
+ const tomorrowFormatted = format(addDays(new Date(), 1), DATE_FORMATS.dayOfWeek)
+ const nextWeekFormatted = format(addDays(new Date(), 7), DATE_FORMATS.fullDate)
- const dueDateFormatted = noDate
- ? "No Date Assigned"
- : date
- ? format(date, "eee MMM dd")
- : "No Date Assigned"
+ const dueDateFormatted = date
+ ? format(date, DATE_FORMATS.fullDate)
+ : "No Date Assigned" 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||||||||||
<div | ||||||||||||||||||||||||||||||||||||||||||||
className="flex flex-row-reverse overflow-hidden rounded-lg bg-background text-secondary-foreground" | ||||||||||||||||||||||||||||||||||||||||||||
{...props} | ||||||||||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||||||||||
<div className="p-4 text-sm"> | ||||||||||||||||||||||||||||||||||||||||||||
<div className="mb-2 text-primary-foreground"> | ||||||||||||||||||||||||||||||||||||||||||||
Due Date:{" "} | ||||||||||||||||||||||||||||||||||||||||||||
<strong className="text-foreground">{dueDateFormatted}</strong> | ||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||
<div> | ||||||||||||||||||||||||||||||||||||||||||||
<DateSelectionButton | ||||||||||||||||||||||||||||||||||||||||||||
label="Today" | ||||||||||||||||||||||||||||||||||||||||||||
icon={CalendarDot} | ||||||||||||||||||||||||||||||||||||||||||||
formattedDate={todayFormatted} | ||||||||||||||||||||||||||||||||||||||||||||
onClick={setToday} | ||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||
<DateSelectionButton | ||||||||||||||||||||||||||||||||||||||||||||
label="Tomorrow" | ||||||||||||||||||||||||||||||||||||||||||||
icon={SquareChevronRightIcon} | ||||||||||||||||||||||||||||||||||||||||||||
formattedDate={tomorrowFormatted} | ||||||||||||||||||||||||||||||||||||||||||||
onClick={setTomorrow} | ||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||
<DateSelectionButton | ||||||||||||||||||||||||||||||||||||||||||||
label="Next Week" | ||||||||||||||||||||||||||||||||||||||||||||
icon={CalendarDots} | ||||||||||||||||||||||||||||||||||||||||||||
formattedDate={nextWeekFormatted} | ||||||||||||||||||||||||||||||||||||||||||||
onClick={setNextWeek} | ||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||
<DateSelectionButton | ||||||||||||||||||||||||||||||||||||||||||||
label="No Date" | ||||||||||||||||||||||||||||||||||||||||||||
icon={CalendarX} | ||||||||||||||||||||||||||||||||||||||||||||
formattedDate={""} | ||||||||||||||||||||||||||||||||||||||||||||
onClick={setNoDate} | ||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||
<Calendar | ||||||||||||||||||||||||||||||||||||||||||||
mode="single" | ||||||||||||||||||||||||||||||||||||||||||||
selected={date || undefined} | ||||||||||||||||||||||||||||||||||||||||||||
onSelect={(selectedDate) => { | ||||||||||||||||||||||||||||||||||||||||||||
setNoDateFlag(false) | ||||||||||||||||||||||||||||||||||||||||||||
setDate(selectedDate || null) | ||||||||||||||||||||||||||||||||||||||||||||
setDateChanged(true) | ||||||||||||||||||||||||||||||||||||||||||||
}} | ||||||||||||||||||||||||||||||||||||||||||||
initialFocus | ||||||||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+61
to
+108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enhance accessibility and prop handling. The component needs accessibility improvements and better prop handling for the Calendar component. Consider these improvements:
<div
className="flex flex-row-reverse overflow-hidden rounded-lg bg-background text-secondary-foreground"
+ role="dialog"
+ aria-label="Reschedule calendar"
{...props}
>
<Calendar
mode="single"
- selected={date || undefined}
+ selected={date}
onSelect={(selectedDate) => {
+ if (error) {
+ return <div>Error loading calendar</div>
+ }
+
+ if (loading) {
+ return <div>Loading calendar...</div>
+ }
|
||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential race conditions and unnecessary refetches in useEffect hooks
Several issues need attention:
updateInbox
toggle in dependencies could cause extra rendersConsider this refactor:
This change:
updateInbox
toggle📝 Committable suggestion