-
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 all commits
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,11 @@ export const InboxItems: React.FC = () => { | |
const { session } = useAuth() | ||
|
||
const [isControlHeld, setIsControlHeld] = useState(false) | ||
const [dateChanged, setDateChanged] = useState(false) | ||
const [reschedulingItemId, setReschedulingItemId] = useState<string | null>( | ||
null | ||
) | ||
const [date, setDate] = React.useState<Date | null>(new Date()) | ||
const { inbox, currentItem, setCurrentItem, fetchInbox, updateItem, error } = | ||
useCycleItemStore() | ||
|
||
|
@@ -54,6 +54,14 @@ export const InboxItems: React.FC = () => { | |
} | ||
}, []) | ||
|
||
useEffect(() => { | ||
if (dateChanged && reschedulingItemId && date) { | ||
updateItem(session, { dueDate: date, status: "todo" }, reschedulingItemId) | ||
setReschedulingItemId(null) | ||
setDateChanged(false) | ||
} | ||
}, [date, updateItem, session, reschedulingItemId, dateChanged]) | ||
|
||
const handleExpand = useCallback( | ||
(item: CycleItem) => { | ||
if (isControlHeld && item.type === "link") { | ||
|
@@ -80,7 +88,7 @@ export const InboxItems: React.FC = () => { | |
session, | ||
{ | ||
status: newStatus, | ||
dueDate: today.toISOString(), | ||
dueDate: today, | ||
cycle: { | ||
startsAt: startDate, | ||
endsAt: endDate, | ||
|
@@ -121,6 +129,17 @@ export const InboxItems: React.FC = () => { | |
|
||
const filteredItems = items.filter((item) => item.status !== "done") | ||
|
||
const handleRescheduleCalendar = ( | ||
e: React.MouseEvent, | ||
id: string, | ||
dueDate: Date | null | ||
) => { | ||
e.stopPropagation() | ||
|
||
setReschedulingItemId(id) | ||
setDate(dueDate) | ||
} | ||
|
||
return ( | ||
<div className="no-scrollbar flex h-full flex-col gap-2 overflow-y-auto"> | ||
{filteredItems.length === 0 ? ( | ||
|
@@ -172,7 +191,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 +206,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>
+ }
|
||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -395,6 +395,17 @@ export const useCycleItemStore = create<ExtendedCycleItemStore>((set, get) => ({ | |||||||||||||||||||||||||||||||||||||||||
id: string | ||||||||||||||||||||||||||||||||||||||||||
) => { | ||||||||||||||||||||||||||||||||||||||||||
set((state) => { | ||||||||||||||||||||||||||||||||||||||||||
const updateItemsInViewInbox = (items: CycleItem[]) => { | ||||||||||||||||||||||||||||||||||||||||||
// remove item from inbox if dueDate is set (not null) | ||||||||||||||||||||||||||||||||||||||||||
if (updates.dueDate) { | ||||||||||||||||||||||||||||||||||||||||||
return items.filter((item) => item._id !== id) | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
// otherwise, just update the item in inbox | ||||||||||||||||||||||||||||||||||||||||||
return items.map((item) => | ||||||||||||||||||||||||||||||||||||||||||
item._id === id ? { ...item, ...updates } : item | ||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+398
to
+407
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 enhancing type safety and edge cases in updateItemsInViewInbox. The function correctly implements the inbox view update logic, removing items when a due date is set and updating them otherwise. However, consider these improvements:
Consider this enhanced implementation: const updateItemsInViewInbox = (items: CycleItem[]) => {
- // remove item from inbox if dueDate is set (not null)
- if (updates.dueDate) {
+ // Remove item from inbox if dueDate is set (not null)
+ if (updates.dueDate !== undefined && updates.dueDate !== null) {
return items.filter((item) => item._id !== id)
}
- // otherwise, just update the item in inbox
+ // Otherwise, just update the item in inbox
return items.map((item) =>
item._id === id ? { ...item, ...updates } : item
)
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
const updateItemsInView = (items: CycleItem[], isOverdue = false) => { | ||||||||||||||||||||||||||||||||||||||||||
// Only filter out done items from overdue list | ||||||||||||||||||||||||||||||||||||||||||
if (isOverdue && updates.status === "done") { | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -408,7 +419,10 @@ export const useCycleItemStore = create<ExtendedCycleItemStore>((set, get) => ({ | |||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||||||||||||
inbox: { ...state.inbox, items: updateItemsInView(state.inbox.items) }, | ||||||||||||||||||||||||||||||||||||||||||
inbox: { | ||||||||||||||||||||||||||||||||||||||||||
...state.inbox, | ||||||||||||||||||||||||||||||||||||||||||
items: updateItemsInViewInbox(state.inbox.items), | ||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||
today: { ...state.today, items: updateItemsInView(state.today.items) }, // No filtering | ||||||||||||||||||||||||||||||||||||||||||
overdue: { | ||||||||||||||||||||||||||||||||||||||||||
...state.overdue, | ||||||||||||||||||||||||||||||||||||||||||
|
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.
🛠️ Refactor suggestion
Add error handling to the date update effect.
The useEffect should handle potential failures in the updateItem call to prevent silent failures and provide feedback to users.
Consider this enhancement: