Skip to content

Feature: Show Task Details For Approved Tasks#1164

Merged
b1ink0 merged 4 commits intofeat/redesignfrom
fix/show-inline-popover
Apr 6, 2026
Merged

Feature: Show Task Details For Approved Tasks#1164
b1ink0 merged 4 commits intofeat/redesignfrom
fix/show-inline-popover

Conversation

@b1ink0
Copy link
Copy Markdown
Collaborator

@b1ink0 b1ink0 commented Apr 1, 2026

Description

This PR fixes missing task details on hover for approved time entries and refactors the code to reuse existing task data instead of making a new API request.

Screenshot/Screencast

Screen.Recording.2026-04-01.at.11.35.28.PM.mov

Checklist

  • I have carefully reviewed the code before submitting it for review.
  • This code is adequately covered by unit tests to validate its functionality.
  • I have conducted thorough testing to ensure it functions as intended.
  • A member of the QA team has reviewed and tested this PR (To be checked by QA or code reviewer)

@b1ink0
Copy link
Copy Markdown
Collaborator Author

b1ink0 commented Apr 1, 2026

Depends on #1163.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes missing inline time-entry popover details for approved (disabled) cells in the timesheet UI and refactors the inline popover to reuse already-fetched task/time-entry data instead of issuing an extra details API request.

Changes:

  • Updated the design-system timesheet TaskRow popover trigger so hover can work even when the underlying cell button is disabled (approved entries).
  • Refactored the app’s InlineTimeEntry popover to accept precomputed time-entry data (tasks) and a disabled flag rather than fetching details on demand.
  • Extended the app TaskRow row-data computation to provide per-day task entries to the inline popover.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
frontend/packages/design-system/src/components/timesheet/rows/task/taskRow.tsx Adjusts popover trigger rendering to enable hover behavior for disabled/approved cells.
frontend/packages/app/src/components/timesheet-row/components/row/taskRow.tsx Computes and passes per-day task entries + disabled state into the inline popover.
frontend/packages/app/src/components/timesheet-row/components/inline-time-entry/types.ts Updates inline popover prop types to include taskKey, tasks, and disabled.
frontend/packages/app/src/components/timesheet-row/components/inline-time-entry/index.tsx Removes details fetching and renders inline popover from provided task entry data; gates editing UI via disabled.
Comments suppressed due to low confidence (1)

frontend/packages/app/src/components/timesheet-row/components/row/taskRow.tsx:67

  • tasksForDate is filtered using entry.from_time.includes(date), which is inconsistent with the rest of the timesheet calculations (e.g., calculateTotalHours uses getDateFromDateAndTimeString(from_time) === date). includes can produce false matches and may incorrectly mark a day as approved/disabled. Use the same date-extraction helper for an exact match.
      const currentTotal = calculateTotalHours(tasks, date);
      // Check if the time entry for the day is approved or not.
      const tasksForDate = tasks[taskKey].data.filter((entry) =>
        entry.from_time.includes(date),
      );
      const isApproved = tasksForDate.some((entry) => entry.docstatus === 1);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@b1ink0 b1ink0 marked this pull request as ready for review April 2, 2026 10:50
@b1ink0 b1ink0 self-assigned this Apr 3, 2026
@b1ink0 b1ink0 merged commit 965f21b into feat/redesign Apr 6, 2026
4 of 5 checks passed
@b1ink0 b1ink0 deleted the fix/show-inline-popover branch April 6, 2026 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants