[Feat]: Timesheet Improvements #3480
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/web/app/[locale]/timesheet/[memberId]/components/EditTaskModal.tsxOops! Something went wrong! :( ESLint: 8.46.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThis pull request introduces comprehensive improvements to the timesheet management system across multiple components. The changes focus on enhancing the user interface, refining time entry logic, and improving data handling for timesheet-related functionalities. Key modifications include updates to modal components, date handling helpers, selection components, and detailed views for timesheet entries. The changes aim to provide more flexible and user-friendly interactions when managing timesheet data. Changes
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 1
🧹 Nitpick comments (18)
apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx (1)
411-411: Ensure consistent dark mode styling across interactive states.Adding the
dark:text-whiteclass improves dark mode readability. However, consider adding explicit hover/focus states for better user feedback, particularly when interacting with items in this dropdown.apps/web/app/[locale]/timesheet/[memberId]/components/TimesheetCard.tsx (2)
133-133: Consider extracting the styling of the Badge into reusable Tailwind classes or theme variables.While the inline styles for the Badge component provide a clear, custom look, placing them in utility classes or referencing them in the Tailwind configuration may improve consistency, reduce duplication, and make maintaining the design system easier.
136-136: Align text color with Tailwind’s default classes or theme.Inline color like
text-[#293241]may reduce clarity and theming consistency. If possible, switch to a Tailwind or theme-based color class (e.g.,text-primary,text-gray-800) for better maintainability, consistency, and dark-mode support.apps/web/app/[locale]/timesheet/[memberId]/components/TimesheetDetailModal.tsx (2)
Line range hint
72-153: Consider extracting common logic into helper functions for improved maintainability.The
MembersWorkedCardcomponent includes nested accordions, grouping logic, and multiple repeated code segments (e.g., repeated<Badge>usage). Extracting repeated elements or grouping logic into smaller helper functions and sub-components could help improve readability, make future changes easier, and reduce code duplication.
Line range hint
155-217: Review the naming and consider adding inline documentation.The
MenHoursCardis well-structured and follows an accordion pattern similar toMembersWorkedCard. However, the naming “MenHoursCard” might be misleading or ambiguous to others on the team. Consider using a more descriptive name such asManHoursCardorTeamHoursCardif the card is intended to represent total labor hours. Additionally, a few inline comments clarifying the grouping logic by timesheet status would make the component’s purpose clearer.apps/web/lib/features/multiple-select/index.tsx (3)
95-101: Expand or refine type definitions for better clarity.By switching
options: string[] | any[]tooptions: any[], you're broadening the accepted types, but losing some type safety. If the project benefits from stronger typing, consider creating a generic type parameter, e.g.CustomSelectProps<T>, and usingT[]instead ofany[].
104-114: Ensure documentation accurately reflects code behavior.Comments mention
(option: string) => React.ReactNode, but the new signature is(option: any) => React.ReactNode. To avoid confusion, update the docstrings to align with the new type usage so future maintainers accurately understand how to pass options and rendering logic.
135-153: Validate value changes correctly propagate up the component tree.The onValueChange directly calls
onChange. IfonChangeis optional, ensure logic downstream (like parent components) safely handles an undefined callback. IfonChangeis crucial, consider making it required or providing a meaningful default.apps/web/app/helpers/date.ts (1)
102-114: Clarify or unify date handling throughout the codebase.
toDateis a helpful utility, but the project also uses other date libraries (e.g.,moment). Make sure these approaches align or are consistently applied throughout the codebase to avoid confusion or subtle bugs.apps/web/app/[locale]/timesheet/[memberId]/components/EditTaskModal.tsx (6)
8-17: Imports look good but watch for potential duplication.Multiple date/time libraries (
date-fns,moment,parseISOetc.) can introduce confusion. Consolidating either moment-based or date-fns-based utilities may simplify maintenance.
42-46: Sufficient documentation on “updateTime”.The docstring is concise and beneficial. Adding brief parameter constraints or examples would further enhance clarity for new contributors.
62-66: Document side effects for “handleSelectedValuesChange”.This function updates
projectIdin the state. If side effects or calls to external APIs are planned with newly selected values, consider adding in-code comments to clarify.
108-109: Remove or convert debug logs.
console.logstatements are useful during development but can clutter production logs. Remove or gate them behind a debug flag if needed.
190-204: Employee selection UI improvements.The custom rendering with employee avatars is good. Consider adding a fallback avatar or placeholder text for cases where
option.employee.user.imageUrlis missing.
222-222: Initialize time inputs more dynamically.You’re using
"09:00"as the fallback. If the timesheet started at a drastically different hour, consider a more context-aware default.apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx (3)
137-137: Minor spacing observation.
Consider removing extra whitespace if it's unintentional and does not serve a stylistic purpose.
143-143: Consistent drop-shadow usage.
Check if this shadow styling aligns with design guidelines in other components.
269-269: Repeated styling pattern.
The same shadow and size classes are used in multiple places. Consider factoring out a shared style or component as this pattern grows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/web/app/[locale]/timesheet/[memberId]/components/AddTaskModal.tsx(2 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx(4 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/EditTaskModal.tsx(8 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/TimesheetCard.tsx(1 hunks)apps/web/app/[locale]/timesheet/[memberId]/components/TimesheetDetailModal.tsx(1 hunks)apps/web/app/[locale]/timesheet/[memberId]/page.tsx(2 hunks)apps/web/app/helpers/date.ts(2 hunks)apps/web/app/hooks/features/useTimesheet.ts(1 hunks)apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx(2 hunks)apps/web/lib/features/integrations/calendar/table-time-sheet.tsx(1 hunks)apps/web/lib/features/multiple-select/index.tsx(2 hunks)
🔇 Additional comments (20)
apps/web/components/pages/task/details-section/blocks/task-secondary-info.tsx (1)
361-361: Looks good!
The additional dark:text-white class enhances readability in dark mode. The usage of clsxm for conditionally applying Tailwind classes here is consistent and clear.
apps/web/app/hooks/features/useTimesheet.ts (2)
170-172: Ensure the correct property is used for comparison.
Previously, the comparison likely involved item.timesheet?.id. Now it directly compares item.id to response.data.id. Confirm that the item.id property truly maps to a timesheet record ID rather than a nested timesheet.id.
Line range hint 208-210: Good addition for robust error handling.
Throwing an error when logIds is empty prevents unintended deletions and ensures clarity on what's being deleted. This is a suitable approach to avoid potential no-op calls or ambiguous error messages.
apps/web/lib/features/multiple-select/index.tsx (1)
126-129: Double-check default props for potential edge cases.
Defaulting valueKey = 'id' and placeholder = "Select an option" is helpful, but ensure any existing consumers of CustomSelect handle these defaults gracefully (e.g., objects without an 'id' field or situations where no placeholder is desired).
apps/web/app/helpers/date.ts (1)
82-101: Validate date edge cases in formatTimeFromDate.
When date is near day boundaries (e.g., 23:59 changing to next day) or is invalid, the function silently returns empty string or an approximate result. Consider adding optional error logs or fallback logic if invalid date values are encountered.
apps/web/app/[locale]/timesheet/[memberId]/page.tsx (2)
274-274: Match vertical spacing to the rest of the UI.
Changing the button height to 76px and adding py-6 can look significantly larger than adjacent UI elements. Verify that this design aligns with the overall style guidelines for consistent spacing.
283-283: Use descriptive displayName for dev tooling consistency.
Explicitly setting ViewToggleButton.displayName helps with debugging in React DevTools. This is a good practice.
apps/web/app/[locale]/timesheet/[memberId]/components/EditTaskModal.tsx (8)
26-26: Verify team membership data usage.
Reading activeTeam from the organization teams is beneficial for scoping employee selection. Confirm that activeTeam is always set, or handle the null case gracefully if an organization has no active team.
29-32: Leverage consistent naming for “initialTimeRange”.
Great practice centralizing the initial state. Just ensure naming remains consistent throughout the code (some places mention “startTime”, others “startedAt”).
37-40: Check for large time gaps or negative durations.
differenceBetweenHours returns 0 when invalid. Confirm any code handling negative or extremely large intervals is tested.
57-57: Confirm fallback for employeeId in all usage scenarios.
Defaulting to an empty string is fine for initialization, but ensure subsequent references to employeeId handle the empty state gracefully (e.g. no crash on submit).
113-113: Ensure employeeId is valid before sending to the API.
Passing an invalid (or empty) employeeId could cause server-side errors. Consider pre-validating or gracefully handling the case where no employee is selected.
164-169: Check time rounding logic in “getMinEndTime”.
addMinutes(startDate, 5) enforces a minimum 5-minute duration. Confirm that edge cases such as near-midnight times or time zone boundary are handled correctly.
209-212: Re-check internationalization for time display.
String(hours).padStart(2, '0') is straightforward, but ensure it aligns with potential local time formatting needs (e.g., 12-hour vs. 24-hour). If 24-hour is always used, this is fine.
241-245: Prevent user confusion with minimum end time.
min={getMinEndTime()} is helpful, but ensure that if the user sets the start time close to midnight, the min end time remains valid or triggers appropriate error messages.
apps/web/app/[locale]/timesheet/[memberId]/components/CalendarView.tsx (2)
127-127: No concerns with styling.
This class addition is consistent with the rest of the UI code.
164-173: Helpful fallback for undefined project.
Using "No Project" as a fallback avoids confusion when project data is missing. If it requires localization, consider internationalizing this string in the future.
apps/web/lib/features/integrations/calendar/table-time-sheet.tsx (1)
227-227: Fallback label for missing project data.
Displaying "No Project" consistently ensures clarity. If required elsewhere, make sure it’s properly localized.
apps/web/app/[locale]/timesheet/[memberId]/components/AddTaskModal.tsx (2)
131-132: Verify time zone impact.
Removing toUTC() might affect how start and end times are interpreted in different locales/time zones. Confirm that this change aligns with intended time zone handling.
185-194: Potential console log leftover & robust error handling.
- Remove or replace
console.log(value)with appropriate logging or debugging strategy before production to avoid clutter in logs. - Ensure that
employeeIdis always defined to prevent potential runtime errors from empty or null selections.
Description
Please include a summary of the changes and the related issue.
Type of Change
Checklist
Previous screenshots
Please add here videos or images of previous status
Current screenshots
Please add here videos or images of previous status
Summary by CodeRabbit
Release Notes
New Features
MembersWorkedCardandMenHoursCard.AddTaskModalandEditTaskModalwith improved employee selection and time handling.Improvements
CalendarView.Bug Fixes
Documentation
CustomSelectcomponent to reflect new props and usage.