Skip to content
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: Implement Timesheet Filter Component With Status, Date Filters #3208

Merged
merged 5 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/web/app/[locale]/calendar/component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
SelectItem,
SelectTrigger,
SelectValue,
} from "@components/ui/select"
} from "@components/ui/select";
import { cn } from "lib/utils";
import { CalendarDays } from "lucide-react";
import React from "react";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,15 @@ export function FilterWithStatus({
];

return (
<div className={clsxm('grid grid-cols-4 h-[2.4rem] items-center justify-start bg-[#e2e8f0aa] rounded-xl w-full', className)}>
<div className={clsxm('flex flex-nowrap h-[2.2rem] items-center bg-[#e2e8f0aa] rounded-xl w-[520px]', className)}>
{buttonData.map(({ label, count, icon }, index) => (
<Button
key={index}
className={clsxm('group flex items-center justify-start h-[2.4rem] rounded-xl border dark:bg-dark--theme-light dark:border-gray-700 bg-[#e2e8f0aa] text[#71717A]', `${activeStatus === label ? "text-primary bg-white shadow-lg font-bold" : ""}`)}
className={clsxm(
'group flex items-center justify-start h-[2.2rem] rounded-xl border w-full',
'dark:bg-dark--theme-light dark:border-gray-700 bg-[#e2e8f0aa] text[#71717A] w-[80px]',
activeStatus === label && 'text-primary bg-white shadow-lg font-bold'
)}
onClick={() => onToggle(label)}>
<span className={clsxm('font-medium ml-1 text-[#71717A]', `${activeStatus === label ? "text-primary" : ""}`)}>{label}</span>
<span className='font-medium ml-1 text-[#71717A]'>{count}</span>
Expand Down
124 changes: 105 additions & 19 deletions apps/web/app/[locale]/timesheet/components/FrequencySelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ import {
DropdownMenuTrigger,
} from "@components/ui/dropdown-menu";
import { Button } from "lib/components/button";
import { DatePicker } from "@components/ui/DatePicker";
import { cn } from "@/lib/utils";
import { format } from "date-fns";
import { PiCalendarDotsThin } from "react-icons/pi";

interface DatePickerInputProps {
date: Date | null;
label: string;
}

export function FrequencySelect() {
const [selectedValue, setSelectedValue] = React.useState<string | undefined>(undefined);
Expand All @@ -30,7 +39,7 @@ export function FrequencySelect() {
<Select
value={selectedValue}
onValueChange={handleSelectChange}>
<SelectTrigger className="w-[180px] border border-gray-200 dark:border-gray-700 bg-white dark:bg-dark--theme-light focus:ring-2 focus:ring-transparent">
<SelectTrigger className="w-[170px] border border-gray-200 dark:border-gray-700 bg-white dark:bg-dark--theme-light focus:ring-2 focus:ring-transparent">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Width change appears inconsistent with similar components

The width reduction from 180px to 170px creates an inconsistency with the calendar component that uses the same select pattern. The calendar component at apps/web/app/[locale]/calendar/component.tsx maintains a width of 180px for its SelectTrigger, which suggests this width reduction might be unintentional.

  • calendar/component.tsx: Uses w-[180px]
  • timesheet/components/FrequencySelect.tsx: Changed to w-[170px]

Both components appear to serve similar purposes with the same placeholder "Select a daily", so they should maintain consistent widths for a uniform UI experience.

🔗 Analysis chain

Verify the impact of width reduction.

The SelectTrigger width has been reduced from 180px to 170px. Please ensure this change doesn't cause content truncation or layout shifts in the UI.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other instances of similar width classes to verify consistency
rg -l 'w-\[1[7-8]0px\]' --type tsx

Length of output: 64


Script:

#!/bin/bash
# Search for width classes with proper file type
rg -l 'w-\[1[7-8]0px\]' --type ts --type tsx --type jsx --type js

# Also search for SelectTrigger components to understand the width patterns
rg -l 'SelectTrigger' --type ts --type tsx --type jsx --type js

Length of output: 187


Script:

#!/bin/bash
# Search for width classes without file type restrictions
rg -l 'w-\[1[7-8]0px\]'

# Search for SelectTrigger components without file type restrictions
rg 'SelectTrigger.*w-\[' -A 1 -B 1

Length of output: 2085

<SelectValue placeholder="Select a daily" />
</SelectTrigger>
<SelectContent>
Expand All @@ -46,49 +55,126 @@ export function FrequencySelect() {

export const FilterTaskActionMenu = () => {
return (
<DropdownMenu open={true} >
<DropdownMenu >
<DropdownMenuTrigger asChild>
<Button variant="ghost" className="h-8 w-8 p-0 text-sm sm:text-base">
<span>Today</span>
</Button>
<div className="w-[120px]">
<Button variant="ghost" className="text-sm font-normal">
<span className="sr-only">Open menu</span>
<span className="">20-10-20</span>
</Button>
Comment on lines +63 to +64
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Replace hard-coded date with dynamic value

The date displayed is hard-coded as "20-10-20". This should be dynamic to reflect the selected date range or the current date for better user experience.

Consider updating the code to use a state variable that holds the selected date:

- <span className="">20-10-20</span>
+ <span className="">{selectedDate ? format(selectedDate, 'dd-MM-yy') : 'Select Date'}</span>

Ensure selectedDate is managed via React.useState and updated appropriately.

Committable suggestion was skipped due to low confidence.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No changes here because I’ll add some modifications in the last PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Innocent-Akim, understood.

(_/)
(..)
(")
(")


🧠 Learnings used
Learnt from: Innocent-Akim
PR: ever-co/ever-teams#3208
File: apps/web/app/[locale]/timesheet/components/FrequencySelect.tsx:67-79
Timestamp: 2024-10-30T16:25:19.910Z
Learning: The date range selection functionality for the date range options in the `FilterTaskActionMenu` component will be implemented in a future PR.

</div>
</DropdownMenuTrigger>
<DropdownMenuContent align="end" className="z-50">
<DropdownMenuItem className="cursor-pointer" >
<DropdownMenuContent align="end">
<DropdownMenuItem onClick={(e) => e.preventDefault()} className="cursor-pointer hover:bg-primary" >
Today
</DropdownMenuItem>
<DropdownMenuItem className="cursor-pointer" >
<DropdownMenuItem onClick={(e) => e.preventDefault()} className="cursor-pointer hover:bg-primary" >
Last 7 days
</DropdownMenuItem>
<DropdownMenuItem className="cursor-pointer" >
<DropdownMenuItem onClick={(e) => e.preventDefault()} className="cursor-pointer hover:bg-primary" >
Last 30 days
</DropdownMenuItem>
<DropdownMenuItem className="cursor-pointer" >
This year (2024)
{/* ({new Date().getFullYear()}) */}
<DropdownMenuItem onClick={(e) => e.preventDefault()} className="cursor-pointer hover:bg-primary" >
This year ({new Date().getFullYear()})
</DropdownMenuItem>
{/* <DropdownMenuSeparator /> */}
<CustomDateRange />
Comment on lines +68 to 80
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Implement functionality for date range selection

The onClick handlers for the date range options currently prevent default behavior but do not update any state. To make the menu functional, these handlers should update the component's state to reflect the selected date range.

For example:

<DropdownMenuItem
-  onClick={(e) => e.preventDefault()}
+  onClick={() => handleDateRangeSelect('today')}
   className="cursor-pointer hover:bg-primary" >
   Today
</DropdownMenuItem>

Define the handleDateRangeSelect function to update the state based on the selected range.

Committable suggestion was skipped due to low confidence.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No changes here because I’ll add some modifications in the last PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Innocent-Akim, understood. Thank you for letting me know. I'll note that you'll be adding these modifications in a future PR.

(_/)
(•‿•)
( 📝)


✏️ Learnings added
Learnt from: Innocent-Akim
PR: ever-co/ever-teams#3208
File: apps/web/app/[locale]/timesheet/components/FrequencySelect.tsx:67-79
Timestamp: 2024-10-30T16:25:19.660Z
Learning: The date range selection functionality for the date range options in the `FilterTaskActionMenu` component will be implemented in a future PR.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

</DropdownMenuContent>
</DropdownMenu>
);
};

export const CustomDateRange = () => {
const [dateRange, setDateRange] = React.useState<{ from: Date | null; to: Date | null }>({
from: new Date(),
to: new Date(),
});
Innocent-Akim marked this conversation as resolved.
Show resolved Hide resolved
const [isDropdownOpen, setDropdownOpen] = React.useState(true);

const cancelChanges = () => {
setDropdownOpen(false);
};
const applyChanges = () => {
setDropdownOpen(false);
};
Innocent-Akim marked this conversation as resolved.
Show resolved Hide resolved
const handleFromChange = (fromDate: Date | null) => {
setDateRange((prev) => ({ ...prev, from: fromDate }));
};

const handleToChange = (toDate: Date | null) => {
if (dateRange.from && toDate && toDate < dateRange.from) {
return;
}
setDateRange((prev) => ({ ...prev, to: toDate }));
};
return (
<DropdownMenuSub>
<DropdownMenuSub open={isDropdownOpen} onOpenChange={setDropdownOpen}>
<DropdownMenuSubTrigger>
<span>Custom Date Range</span>
</DropdownMenuSubTrigger>
<DropdownMenuPortal>
<DropdownMenuSubContent>
<DropdownMenuItem className="cursor-pointer">
<div className="flex items-center gap-3">
<div className="h-1 w-1 rounded-full bg-black dark:bg-white"></div>
<span>Calendar</span>
<DropdownMenuSubContent className="bg-white hover:bg-white">
<DropdownMenuItem className="cursor-pointer bg-white hover:bg-white">
<div className="flex flex-col gap-3">
<DynamicDatePicker
label="From"
date={dateRange.from}
setDate={handleFromChange}
/>
<DynamicDatePicker
label="To"
date={dateRange.to}
setDate={handleToChange}
/>
<div className="flex w-full justify-end gap-4">
<button className="text-primary" onClick={cancelChanges}>Cancel</button>
<button className="text-primary" onClick={applyChanges}>Apply</button>
</div>
</div>
</DropdownMenuItem>
</DropdownMenuSubContent>
</DropdownMenuPortal>
</DropdownMenuSub>
)
}

const DatePickerInput: React.FC<DatePickerInputProps> = ({ date, label }) => (
<>
<Button
variant="outline"
className={cn(
"w-[150px] justify-start text-left font-normal text-black h-10 border border-transparent dark:border-transparent",
!date && "text-muted-foreground"
)}
>
{date ? format(date, "LLL dd, y") : <span>{label}</span>}
</Button>
<PiCalendarDotsThin className="h-5 w-5 dark:text-gray-500" />
</>
);

export function DynamicDatePicker({
label,
date,
setDate,
}: {
label: string;
date: Date | null;
setDate: (date: Date | null) => void;
}) {
return (
<div>
<DatePicker
buttonVariant={'link'}
className="dark:bg-dark--theme-light rounded-lg bg-white"
buttonClassName={'decoration-transparent flex items-center w-full bg-white dark:bg-dark--theme-light border-gray-300 justify-start text-left font-normal text-black h-10 border dark:border-slate-600 rounded-md'}
customInput={<DatePickerInput date={date} label={label} />}
mode="single"
numberOfMonths={1}
initialFocus
defaultMonth={date ?? new Date()}
selected={date ?? new Date()}
onSelect={(selectedDate) => selectedDate && setDate(selectedDate)}
/>
</div>
);
}
Comment on lines +155 to +180
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding error boundary protection.

The DynamicDatePicker component handles dates well but could benefit from error boundary protection for robust error handling.

Consider wrapping the component with an error boundary:

class DatePickerErrorBoundary extends React.Component {
  state = { hasError: false };
  
  static getDerivedStateFromError() {
    return { hasError: true };
  }
  
  render() {
    if (this.state.hasError) {
      return <div>Error loading date picker. Please try again.</div>;
    }
    return this.props.children;
  }
}

// Usage
export function DynamicDatePicker({...props}) {
  return (
    <DatePickerErrorBoundary>
      <div>
        <DatePicker {...existing props} />
      </div>
    </DatePickerErrorBoundary>
  );
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skip

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Innocent-Akim, understood, I'll skip this suggestion.

(_/)
(;_;)


🧠 Learnings used
Learnt from: Innocent-Akim
PR: ever-co/ever-teams#3208
File: apps/web/app/[locale]/timesheet/components/FrequencySelect.tsx:67-79
Timestamp: 2024-10-30T16:25:19.910Z
Learning: The date range selection functionality for the date range options in the `FilterTaskActionMenu` component will be implemented in a future PR.

7 changes: 3 additions & 4 deletions apps/web/app/[locale]/timesheet/components/TimesheetCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,17 @@ export function TimesheetCard({ ...props }: ITimesheetCard) {
)} />
</Button>
</div>
<Card
shadow='custom'
<div
className={clsxm(
'h-7 w-7',
'h-16 w-16 rounded-lg p-5',
'flex items-center justify-center',
'text-white font-bold text-sm',
'shadow-lg',
classNameIcon
)}
aria-hidden="true">
{icon}
</Card>
</div>
</Card>
)
}
68 changes: 38 additions & 30 deletions apps/web/app/[locale]/timesheet/components/TimesheetFilter.tsx
Original file line number Diff line number Diff line change
@@ -1,39 +1,47 @@
import React from 'react'
import { FilterWithStatus } from './FilterWithStatus'
import { FrequencySelect } from '.';
import { FilterWithStatus } from './FilterWithStatus';
import { FilterTaskActionMenu, FrequencySelect } from '.';
import { Button } from 'lib/components';
import { SettingFilterIcon } from 'assets/svg';
import { TimeSheetFilterPopover } from './time-sheet-filter-popover';
import { Cross2Icon } from '@radix-ui/react-icons';

export function TimesheetFilter() {
return (
<div className="grid grid-cols-3 w-full">
<div className="col-span-1">
<FilterWithStatus
activeStatus="Rejected"
onToggle={(label) => {
console.log(label)
}}
/>
</div>
<div className="col-span-1"></div>
<div className="col-span-1">
<div className='flex gap-2'>
<FrequencySelect />
<button
onClick={() => null}
className='flex items-center justify-center h-10 rounded-lg bg-white dark:bg-dark--theme-light border dark:border-gray-700 hover:bg-white p-3 gap-2' >
<SettingFilterIcon className="text-gray-700 dark:text-white w-3.5" strokeWidth="1.8" />
<span className="text-gray-700 dark:text-white">Filter</span>
</button>
<Button
onClick={() => null}
variant='outline'
className='bg-primary/5 dark:bg-primary-light h-10 w-[2.5rem] font-medium'>
Add Time
</Button>
<>

<div className="flex flex-col md:flex-row w-full gap-4">
<div className="flex-1">
<FilterWithStatus
activeStatus="All Tasks"
onToggle={(label) => {
// TODO: Implement filter toggle handler
}}
/>
</div>
<div className="flex-1 flex justify-end">
<div className='flex gap-2'>
<FrequencySelect />
<div className='flex items-center border border-gray-100 rounded-md h-10 '>
<FilterTaskActionMenu />
<button
aria-label="Clear filters"
onClick={() => {
// TODO: Implement clear filters logic
}}
className='border-l h-10 w-10 font-normal flex items-center justify-center text-gray-400 hover:bg-gray-50'>
<Cross2Icon />
</button>
</div>
<TimeSheetFilterPopover />
<Button
onClick={() => null}
variant='outline'
className='bg-primary/5 dark:bg-primary-light h-10 w-[2.5rem] font-medium'>
Add Time
</Button>
</div>
Comment on lines +35 to +40
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Based on the search results, I can see that there's an existing implementation of the Add Time functionality in the codebase. The AddManualTimeModal component from lib/features/manual-time/add-manual-time-modal.tsx is used for this purpose, and it's already being used in other parts of the application.

The Add Time button should use the existing AddManualTimeModal component

The button's onClick handler should open the AddManualTimeModal component. Here are the specific changes needed:

  • Import the AddManualTimeModal component from 'lib/features/manual-time/add-manual-time-modal'
  • Add state management for the modal's open/close status
  • Connect the button's onClick handler to open the modal

Example implementation found in apps/web/lib/features/task/task-filters.tsx:

<AddManualTimeModal closeModal={closeManualTimeModal} isOpen={isManualTimeModalOpen} params="AddTime" />
🔗 Analysis chain

Implement Add Time button functionality

The Add Time button has an empty onClick handler. This seems to be incomplete implementation.

Let's check if there are any existing implementations we should use:

Would you like me to help implement the Add Time functionality or create a GitHub issue to track this task?


Let me search for implementations using different patterns to ensure we don't miss anything. The previous command failed because of incorrect file type specification, so I've corrected that and added more comprehensive searches.


Let me try different search patterns and commands since the previous ones had issues with file type recognition. I'll also check the timesheet directory structure to understand the context better.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for existing Add Time implementations
rg -i "add.*time" --type tsx

Length of output: 247


Script:

#!/bin/bash
# Search for existing Add Time implementations with ripgrep, using correct file type
rg -i "add.*time" -t ts -t tsx

# Look for any modal or form components related to time entry
rg -i "(timeentry|addtime|timeinput)" -t ts -t tsx

# Search for any state management or handlers related to time
ast-grep --pattern 'const [$_] = useState($$$)'
ast-grep --pattern 'function handle[$_]Time($$$) { $$$ }'

Length of output: 697


Script:

#!/bin/bash
# Try searching with just 't' type for TypeScript files
rg -i "add.*time" -t ts
rg -i "(timesheet|timelog)" -t ts

# Look for any timesheet-related components or handlers
fd -e tsx -e ts | grep -i "time"

# Check the specific directory for related components
ls -R apps/web/app/*/timesheet/

Length of output: 39438

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No changes here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Innocent-Akim, understood.

(A small rabbit looks down sadly.)


✏️ Learnings added
Learnt from: Innocent-Akim
PR: ever-co/ever-teams#3208
File: apps/web/app/[locale]/timesheet/components/TimesheetFilter.tsx:30-35
Timestamp: 2024-10-30T16:31:00.562Z
Learning: In the `TimesheetFilter` component, the `Add Time` button does not need to use the `AddManualTimeModal` component, as per the user's decision.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

</div>
</div>
</div>
</>

)
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { DataTableTimeSheet } from 'lib/features/integrations/calendar'
import React from 'react'

export function TimesheetView() {
return (
Expand Down
Loading
Loading