-
-
Notifications
You must be signed in to change notification settings - Fork 19
Improve Time Tracker UX and layout #3097
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
Conversation
- Implement mobile-first layout and comprehensive keyboard accessibility ## Mobile Layout Optimization - Reorder grid layout to prioritize timer controls first on mobile devices - Use CSS order classes (order-1 lg:order-2) for responsive priority switching - Maintain desktop layout with analytics sidebar (2/5) and timer controls (3/5) ## Comprehensive Keyboard Accessibility ### Core Timer Controls - ⌘/Ctrl + Enter: Start/stop timer with current selection - ⌘/Ctrl + P: Pause active timer session - Space: Quick start timer (when not typing in inputs) - Escape: Close dropdowns or show drag cancellation hints ### Task Management Shortcuts - ⌘/Ctrl + T: Toggle task selection dropdown - ⌘/Ctrl + M: Switch between task/manual session modes - ↑/↓ Arrow Keys: Navigate through filtered tasks in dropdown - Enter: Select currently highlighted task from dropdown ### Smart Input Detection - Context-aware shortcut handling that disables when typing - Prevents interference with normal text input - Maintains accessibility for screen readers ## UI/UX Improvements - Fixed double scrollbar issue by optimizing scroll container hierarchy - Changed body overflow from 'scroll' to 'auto' for better UX - Removed redundant overflow-y-auto from main content area - Enhanced visual feedback with keyboard shortcut hints Files changed: - apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx - apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx - apps/web/src/app/[locale]/layout.tsx (scrollbar fix) - packages/ui/src/components/ui/custom/structure.tsx (scrollbar fix)
Caution Review failedThe pull request is closed. WalkthroughThis update refactors the time tracker UI and its components, introducing a new sidebar layout, an interactive and filterable task selection dropdown with drag-and-drop support, enhanced keyboard navigation, and a quick actions carousel. It also adjusts scroll handling to prevent double scrollbars by shifting overflow management to the body element. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TimerControls
participant Sidebar (Tasks)
participant Toast
User->>TimerControls: Drag task over input/dropdown
TimerControls->>TimerControls: Handle drag enter/over/leave/drop events
TimerControls->>Sidebar (Tasks): On drop, set session mode, populate fields
TimerControls->>Toast: Show success message
User->>TimerControls: Use keyboard shortcuts (e.g., Ctrl+T, arrows, Enter, Space)
TimerControls->>TimerControls: Update dropdown, select task, start timer
Suggested labels
Poem
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/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsxOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
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.
Summary of Changes
Hello @Adinorio, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a significant user experience overhaul for the Time Tracker feature. I've implemented a new responsive two-column layout, enhanced task selection with search, filtering, and drag-and-drop, added new keyboard shortcuts, and reorganized the analytics and task views into a dedicated sidebar. It also includes a new quick actions area and fixes a minor scrollbar issue.
Highlights
- Layout Redesign: The Time Tracker page has been refactored into a two-column layout on larger screens, featuring a switchable sidebar for Analytics, Tasks, Reports, and Settings, alongside the main timer and session tabs.
- Enhanced Task Selection: The timer controls now include a custom searchable and filterable dropdown for selecting tasks, replacing the standard select component. This improves the process of associating time entries with specific tasks.
- Drag and Drop for Tasks: Tasks from the new sidebar list can now be dragged directly onto the timer controls area to quickly select them for tracking. Visual feedback is provided during the drag and drop process.
- New Keyboard Shortcuts: Several keyboard shortcuts have been added for faster interaction, including toggling the task dropdown (Cmd/Ctrl + T), switching between task/manual modes (Cmd/Ctrl + M), navigating tasks in the dropdown (Arrow keys), selecting a task (Enter), and starting the timer (Space).
- Integrated Analytics and Tasks Sidebar: The previous stats overview and activity heatmap have been integrated into a new 'Analytics' sidebar view. A dedicated 'Tasks' sidebar view has been added, listing tasks with search and filtering capabilities.
- Quick Actions Carousel: A new carousel section has been added below the header, providing quick access buttons for common actions (some are placeholders for future features like 'Continue Last Session' or 'Start Most Used Task').
- UI/UX Improvements: Various styling and minor UI adjustments have been made throughout the Time Tracker components for a more polished look and feel, including improved status indicators and clearer empty states.
- Scrollbar Fix: Adjusted overflow styles in the main layout and structure components to resolve a potential double scrollbar issue.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configureGemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3097 +/- ##
==========================================
- Coverage 0.83% 0.83% -0.01%
==========================================
Files 2500 2500
Lines 309069 310912 +1843
Branches 3002 3002
==========================================
Hits 2582 2582
- Misses 304352 306195 +1843
Partials 2135 2135 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Code Review
This pull request introduces significant UX improvements to the time tracker page, including task drag-and-drop integration, a searchable task selection dropdown, a quick actions carousel, and a new sidebar layout for analytics, tasks, reports, and settings. The changes enhance usability and organization, although some features are marked as unfinished. The implementation involves managing various states, handling drag/drop events, implementing search/filtering, calculating dropdown positioning, and adding keyboard shortcuts. Overall, it's a substantial step towards a more interactive and feature-rich time tracking interface.
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx
Outdated
Show resolved
Hide resolved
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.
LGTM! Thanks @Adinorio.
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.
Actionable comments posted: 1
🔭 Outside diff range comments (3)
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx (3)
377-392
:⚠️ Potential issueStale
fetchData
reference inside auto-refresh interval
useEffect
creates an interval that callsfetchData
, yetfetchData
is not part of the dependency array.
IfselectedUserId
,wsId
, etc. change, a newfetchData
is created but the interval still closes over the old one, so it will keep refreshing the wrong workspace / user.-}, [isLoading, retryCount]); // Remove fetchData dependency +}, [isLoading, retryCount, fetchData]);Or keep the deps empty and store
fetchData
in auseRef
that you update on every render (fetchRef.current = fetchData
) and callfetchRef.current()
inside the interval.
Same issue re-appears in the online/offline effect below.
424-442
:⚠️ Potential issue
handleOnline
may call an outdatedfetchData
Because
fetchData
is omitted from the deps, switching users/workspaces after the effect mounts will havehandleOnline
invoking a stale closure.
AddfetchData
to the dependency list or refactor with a mutable ref as suggested above.
104-1800
: 🛠️ Refactor suggestionComponent is ~1800 LOC – extraction strongly advised
TimeTrackerContent
now handles: data fetching, sockets, timer logic, three carousels, four sidebars, drag-and-drop, etc.
Consider splitting into smaller focused components/hooks:
•useTimeTrackerData
(all fetching / polling)
•QuickActionsCarousel
•Sidebar
with sub-components per view
•Header
This will improve readability, testability, and bundle diff size.🧰 Tools
🪛 GitHub Check: CodeFactor
[notice] 757-757: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L757
Unresolved 'todo' comment. (eslint/no-warning-comments)
[notice] 691-691: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L691
Unresolved 'todo' comment. (eslint/no-warning-comments)
[notice] 735-735: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L735
Unresolved 'todo' comment. (eslint/no-warning-comments)
[notice] 713-713: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L713
Unresolved 'todo' comment. (eslint/no-warning-comments)
♻️ Duplicate comments (3)
packages/ui/src/components/ui/custom/structure.tsx (1)
144-149
: Verify scrolling behavior across all pages using the Structure component.The removal of
overflow-y-auto
is well-documented with explanatory comments. However, as noted in the previous review, ensure this change doesn't negatively impact scrolling on other pages.Run the following script to identify all pages using the Structure component and verify they handle scrolling appropriately:
#!/bin/bash # Description: Find all files importing and using the Structure component to assess the impact of scrolling changes # Find all files that import the Structure component echo "=== Files importing Structure component ===" rg -l "import.*Structure.*from.*structure" --type tsx --type ts # Show usage patterns of Structure component echo -e "\n=== Structure component usage patterns ===" ast-grep --pattern 'Structure $_' -l # Look for any custom scrolling implementations in these files echo -e "\n=== Custom scrolling implementations in files using Structure ===" rg -A 5 -B 5 "(overflow-y|scroll|scrollbar)" $(rg -l "import.*Structure.*from.*structure" --type tsx --type ts 2>/dev/null) 2>/dev/null | head -50Also verify the user experience by testing scrolling behavior on different viewport sizes:
What are the best practices for handling page-level scrolling in React applications when using nested layout components? Specifically, how to prevent double scrollbars while maintaining smooth scrolling behavior?
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx (1)
764-778
: Escape key drag cancellation limitation is properly handled.The code correctly acknowledges that Escape cannot cancel the parent-controlled drag operation and provides appropriate user feedback via toast. Consider documenting this limitation in a code comment for future maintainers.
// Escape to close dropdown or cancel drag if (event.key === 'Escape') { if (isTaskDropdownOpen) { setIsTaskDropdownOpen(false); return; } if (isDraggingTask) { - // Note: This won't actually cancel the drag since it's controlled by parent - // But it provides visual feedback + // Note: This won't actually cancel the drag since it's controlled by parent component. + // The actual drag cancellation logic needs to be implemented in the parent (time-tracker-content.tsx). + // This toast provides visual feedback about the limitation. toast.info( 'Press ESC while dragging to cancel (drag outside to cancel)' ); return; } }apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx (1)
690-771
: Unresolved TODOs already highlighted in previous reviews“Continue last session”, “Start most used task”, “Quick focus timer”, “From template” remain TODOs.
Tagging as duplicate to avoid noise.Also applies to: 717-746, 741-760
🧰 Tools
🪛 GitHub Check: CodeFactor
[notice] 757-757: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L757
Unresolved 'todo' comment. (eslint/no-warning-comments)
[notice] 691-691: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L691
Unresolved 'todo' comment. (eslint/no-warning-comments)
[notice] 735-735: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L735
Unresolved 'todo' comment. (eslint/no-warning-comments)
[notice] 713-713: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L713
Unresolved 'todo' comment. (eslint/no-warning-comments)
🧹 Nitpick comments (2)
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx (2)
199-227
: Consider the isSearchMode behavior when clearing task selection.When no task is selected (lines 222-225), the code sets
isSearchMode
totrue
. This might be confusing as clearing a selection doesn't necessarily mean the user wants to search. Consider keeping the current mode state or explicitly asking the user.} else { // Reset when no task selected setNewSessionTitle(''); setNewSessionDescription(''); - setIsSearchMode(true); + // Keep current search mode state when clearing selection }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 204-205: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L204-L205
Added lines #L204 - L205 were not covered by tests
[warning] 207-219: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L207-L219
Added lines #L207 - L219 were not covered by tests
[warning] 222-222: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L222
Added line #L222 was not covered by tests
[warning] 224-225: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L224-L225
Added lines #L224 - L225 were not covered by tests
1329-1552
: Consider accessibility for the custom dropdown implementation.The custom dropdown implementation is feature-rich but may have accessibility concerns. Ensure proper ARIA attributes, keyboard navigation, and screen reader support.
Consider adding ARIA attributes to improve accessibility:
{/* Dropdown Content */} {isTaskDropdownOpen && ( <div ref={dropdownContentRef} className={cn( 'absolute right-0 left-0 z-[100] rounded-md border bg-popover shadow-lg transition-all duration-200', dropdownPosition === 'above' ? 'bottom-full mb-1' : 'top-full mt-1' )} + role="listbox" + aria-label="Task selection dropdown" + aria-expanded="true" onClick={(e) => { e.stopPropagation(); }} >Also ensure each task option has proper role and aria attributes:
<button key={task.id} type="button" + role="option" + aria-selected={task.id === selectedTaskId} onClick={(e) => { e.preventDefault(); e.stopPropagation(); if (task.id) { handleTaskSelectionChange(task.id); } }}🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 1463-1549: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L1463-L1549
Added lines #L1463 - L1549 were not covered by tests
[warning] 1551-1552: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L1551-L1552
Added lines #L1551 - L1552 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lock
is excluded by!**/*.lock
📒 Files selected for processing (4)
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx
(12 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx
(6 hunks)apps/web/src/app/[locale]/layout.tsx
(1 hunks)packages/ui/src/components/ui/custom/structure.tsx
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx
[warning] 45-45: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L45
Added line #L45 was not covered by tests
[warning] 103-104: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L103-L104
Added lines #L103 - L104 were not covered by tests
[warning] 121-122: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L121-L122
Added lines #L121 - L122 were not covered by tests
[warning] 135-158: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L135-L158
Added lines #L135 - L158 were not covered by tests
[warning] 204-205: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L204-L205
Added lines #L204 - L205 were not covered by tests
[warning] 207-219: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L207-L219
Added lines #L207 - L219 were not covered by tests
[warning] 222-222: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L222
Added line #L222 was not covered by tests
[warning] 224-225: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L224-L225
Added lines #L224 - L225 were not covered by tests
[warning] 242-244: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L242-L244
Added lines #L242 - L244 were not covered by tests
[warning] 521-576: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L521-L576
Added lines #L521 - L576 were not covered by tests
[warning] 597-754: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L597-L754
Added lines #L597 - L754 were not covered by tests
[warning] 758-782: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L758-L782
Added lines #L758 - L782 were not covered by tests
[warning] 798-858: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L798-L858
Added lines #L798 - L858 were not covered by tests
[warning] 863-873: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L863-L873
Added lines #L863 - L873 were not covered by tests
[warning] 877-883: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L877-L883
Added lines #L877 - L883 were not covered by tests
[warning] 898-905: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L898-L905
Added lines #L898 - L905 were not covered by tests
[warning] 1009-1063: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L1009-L1063
Added lines #L1009 - L1063 were not covered by tests
[warning] 1107-1255: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L1107-L1255
Added lines #L1107 - L1255 were not covered by tests
[warning] 1257-1287: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L1257-L1287
Added lines #L1257 - L1287 were not covered by tests
[warning] 1289-1459: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L1289-L1459
Added lines #L1289 - L1459 were not covered by tests
[warning] 1463-1549: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L1463-L1549
Added lines #L1463 - L1549 were not covered by tests
[warning] 1551-1552: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L1551-L1552
Added lines #L1551 - L1552 were not covered by tests
[warning] 1554-1561: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L1554-L1561
Added lines #L1554 - L1561 were not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx
[warning] 21-23: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L21-L23
Added lines #L21 - L23 were not covered by tests
[warning] 25-28: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L25-L28
Added lines #L25 - L28 were not covered by tests
[warning] 30-30: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L30
Added line #L30 was not covered by tests
[warning] 32-34: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L32-L34
Added lines #L32 - L34 were not covered by tests
[warning] 38-38: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L38
Added line #L38 was not covered by tests
[warning] 40-47: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L40-L47
Added lines #L40 - L47 were not covered by tests
[warning] 49-49: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L49
Added line #L49 was not covered by tests
🪛 GitHub Check: CodeFactor
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx
[notice] 757-757: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L757
Unresolved 'todo' comment. (eslint/no-warning-comments)
[notice] 691-691: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L691
Unresolved 'todo' comment. (eslint/no-warning-comments)
[notice] 735-735: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L735
Unresolved 'todo' comment. (eslint/no-warning-comments)
[notice] 713-713: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx#L713
Unresolved 'todo' comment. (eslint/no-warning-comments)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Verify generated types
- GitHub Check: Prettier Check (23)
🔇 Additional comments (4)
apps/web/src/app/[locale]/layout.tsx (1)
122-122
: Good coordination with Structure component changes.Changing from
overflow-y-scroll
tooverflow-y-auto
properly consolidates page-level scrolling at the body element and ensures scrollbars only appear when needed, improving the user experience.apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx (2)
521-576
: Well-implemented drag and drop functionality.The drag and drop implementation correctly handles nested elements using a counter ref, provides good user feedback, and has proper error handling for the dropped data.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 521-576: apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx#L521-L576
Added lines #L521 - L576 were not covered by tests
641-689
: Excellent dropdown positioning implementation.The dropdown positioning logic properly calculates available viewport space and uses
requestAnimationFrame
for reliable DOM measurements, addressing the previous concern aboutsetTimeout
reliability.apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx (1)
1631-1636
:key={task.id}
risks React warnings
ExtendedWorkspaceTask
marksid
as optional (Partial<WorkspaceTask>
).
If any task comes without anid
, React will warn about missing keys and re-render list items inefficiently.-key={task.id} +key={task.id ?? task.name ?? String(index)}Or guarantee
id
is present by filtering beforehand.Also applies to: 1654-1661
const handleRetry = useCallback(() => { | ||
fetchData(true, true); | ||
}, [fetchData]); | ||
}, []); // Remove fetchData dependency | ||
|
||
// Quick Actions Carousel |
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.
handleRetry
captures a frozen fetchData
useCallback(() => { fetchData(true, true); }, [])
freezes whichever fetchData
existed on first render.
Depend on fetchData
, or memoise fetchData
with useStableCallback
/ref so that the latest implementation is always used.
🤖 Prompt for AI Agents
In
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx
around lines 468 to 472, the handleRetry callback captures a stale version of
fetchData due to an empty dependency array. Fix this by either adding fetchData
to the dependency array of useCallback or memoizing fetchData itself with
useStableCallback or a ref to ensure handleRetry always uses the latest
fetchData implementation.
Summary by CodeRabbit
New Features
UI Improvements
Bug Fixes