Skip to content

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

Merged
merged 8 commits into from
Jun 15, 2025
Merged

Conversation

Adinorio
Copy link
Collaborator

@Adinorio Adinorio commented Jun 15, 2025

  • it has some unfinished stuff, but it is improved.
    image

image

Summary by CodeRabbit

  • New Features

    • Introduced a fully interactive, searchable, and filterable task selection dropdown with drag-and-drop support in the timer controls.
    • Added keyboard navigation and shortcuts for task selection and timer actions.
    • Implemented a sidebar with switchable views for Analytics, Tasks, Reports, and Settings, including a Quick Actions Carousel and enhanced stats overview.
    • Enhanced task management with advanced filtering, searching, and drag-to-timer functionality.
  • UI Improvements

    • Redesigned layout with a sidebar and reorganized main content for improved navigation and clarity.
    • Updated visual feedback for drag-and-drop, empty states, and status indicators.
    • Adjusted scroll behavior to prevent double scrollbars and streamline page scrolling.
  • Bug Fixes

    • Improved state handling to avoid unnecessary re-renders and ensure a smoother user experience.

Adinorio added 2 commits June 15, 2025 17:56
- 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)
Copy link
Contributor

coderabbitai bot commented Jun 15, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This 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

File(s) Change Summary
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/components/timer-controls.tsx Replaced static task select with a custom, searchable, filterable, and keyboard-navigable dropdown; added drag-and-drop support; enhanced session/task control logic; updated props.
apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx Refactored layout to include a sidebar with switchable views, a quick actions carousel, improved stats and task filtering, and updated drag-and-drop state management.
apps/web/src/app/[locale]/layout.tsx Changed body overflow from overflow-y-scroll to overflow-y-auto for improved scroll behavior.
packages/ui/src/components/ui/custom/structure.tsx Removed overflow-y-auto from <main> to prevent double scrollbars; added comments explaining scroll handling.

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
Loading

Suggested labels

enhancement

Poem

A sidebar blooms, the tasks now dance,
With drag and drop, they take their chance.
The dropdown sings with search and flair,
And shortcuts whisk you here and there.
No double scrolls—just smooth delight,
A rabbit’s hop makes work feel light! 🐇✨

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx

Oops! Something went wrong! :(

ESLint: 9.29.0

Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs
at Object.getPackageJSONURL (node:internal/modules/package_json_reader:255:9)
at packageResolve (node:internal/modules/esm/resolve:767:81)
at moduleResolve (node:internal/modules/esm/resolve:853:18)
at defaultResolve (node:internal/modules/esm/resolve:983:11)
at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:801:12)
at #cachedDefaultResolve (node:internal/modules/esm/loader:725:25)
at ModuleLoader.resolve (node:internal/modules/esm/loader:708:38)
at ModuleLoader.getModuleJobForImport (node:internal/modules/esm/loader:309:38)
at #link (node:internal/modules/esm/module_job:201:49)


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ff0392a and 65cac17.

📒 Files selected for processing (1)
  • apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx (6 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate Unit Tests
  • Create PR with Unit Tests
  • Commit Unit Tests in branch feat/time-tracker/ux-improvement
  • Post Copyable Unit Tests in Comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

graphite-app bot commented Jun 15, 2025

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • merge-queue - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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.

Copy link

@gemini-code-assist gemini-code-assist bot left a 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

  1. 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.

Copy link

codecov bot commented Jun 15, 2025

Codecov Report

Attention: Patch coverage is 0% with 2190 lines in your changes missing coverage. Please review.

Project coverage is 0.83%. Comparing base (a24c21a) to head (ff0392a).

Files with missing lines Patch % Lines
...oard)/[wsId]/time-tracker/time-tracker-content.tsx 0.00% 1309 Missing ⚠️
.../[wsId]/time-tracker/components/timer-controls.tsx 0.00% 879 Missing ⚠️
apps/web/src/app/[locale]/layout.tsx 0.00% 1 Missing ⚠️
packages/ui/src/components/ui/custom/structure.tsx 0.00% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

@gemini-code-assist gemini-code-assist bot left a 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.

@vhpx vhpx changed the title Feat/time tracker/ux improvement Improve Time Tracker UX and layout Jun 15, 2025
@Adinorio Adinorio requested a review from vhpx June 15, 2025 18:26
@vhpx vhpx marked this pull request as ready for review June 15, 2025 18:29
vhpx
vhpx previously approved these changes Jun 15, 2025
Copy link
Member

@vhpx vhpx left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @Adinorio.

@vhpx vhpx merged commit 5625f33 into main Jun 15, 2025
15 checks passed
@vhpx vhpx deleted the feat/time-tracker/ux-improvement branch June 15, 2025 18:33
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 issue

Stale fetchData reference inside auto-refresh interval

useEffect creates an interval that calls fetchData, yet fetchData is not part of the dependency array.
If selectedUserId, wsId, etc. change, a new fetchData 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 a useRef that you update on every render (fetchRef.current = fetchData) and call fetchRef.current() inside the interval.
Same issue re-appears in the online/offline effect below.


424-442: ⚠️ Potential issue

handleOnline may call an outdated fetchData

Because fetchData is omitted from the deps, switching users/workspaces after the effect mounts will have handleOnline invoking a stale closure.
Add fetchData to the dependency list or refactor with a mutable ref as suggested above.


104-1800: 🛠️ Refactor suggestion

Component 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 -50

Also 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 to true. 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

📥 Commits

Reviewing files that changed from the base of the PR and between a24c21a and ff0392a.

⛔ 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 to overflow-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 about setTimeout reliability.

apps/web/src/app/[locale]/(dashboard)/[wsId]/time-tracker/time-tracker-content.tsx (1)

1631-1636: key={task.id} risks React warnings

ExtendedWorkspaceTask marks id as optional (Partial<WorkspaceTask>).
If any task comes without an id, 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

Comment on lines 468 to +472
const handleRetry = useCallback(() => {
fetchData(true, true);
}, [fetchData]);
}, []); // Remove fetchData dependency

// Quick Actions Carousel
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants