feat(web): show estimate point sums in list view group headers#9214
Open
laurin-eichberger wants to merge 1 commit into
Open
feat(web): show estimate point sums in list view group headers#9214laurin-eichberger wants to merge 1 commit into
laurin-eichberger wants to merge 1 commit into
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds a user-facing "Show estimate totals" toggle in Display Filters that computes and displays the sum of estimate points next to each grouped list view's header. The feature follows the existing "Show sub-work items" pattern: optional type definitions, UI selection, prop propagation through list components, estimate calculations using the project estimates hook, and conditional badge rendering in group headers. ChangesEstimate totals display in list group headers
🎯 2 (Simple) | ⏱️ ~12 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new “show estimates” display option and surfaces estimate totals in list group headers when enabled.
Changes:
- Added
show_estimatesto issue display filter types, computed filters, constants, and i18n. - Wired the new option through list root → list → list group, and rendered an estimate total badge in the group header.
- Implemented client-side estimate total aggregation for each visible group.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/utils/src/work-item/base.ts | Includes show_estimates in computed display filters. |
| packages/types/src/view-props.ts | Extends filter option unions/interfaces to include show_estimates. |
| packages/i18n/src/locales/en/work-item.json | Adds label for the new extra option. |
| packages/constants/src/issue/filter.ts | Enables show_estimates as a selectable extra option for relevant pages. |
| apps/web/core/components/issues/issue-layouts/list/list-group.tsx | Computes per-group estimate totals and passes them to the header. |
| apps/web/core/components/issues/issue-layouts/list/headers/group-by-card.tsx | Renders estimate total badge (with partial indicator). |
| apps/web/core/components/issues/issue-layouts/list/default.tsx | Threads showEstimates prop into list groups. |
| apps/web/core/components/issues/issue-layouts/list/base-list-root.tsx | Reads show_estimates from display filters and passes it down. |
| apps/web/core/components/issues/issue-layouts/filters/header/display-filters/extra-options.tsx | Adds show_estimates to UI extra options config / types. |
| apps/web/core/components/issues/issue-layouts/filters/header/display-filters/display-filters-selection.tsx | Initializes show_estimates in selected options state. |
Comment on lines
+120
to
+151
| const groupIssueCount = getGroupIssueCount(group.id, undefined, false) ?? 0; | ||
| const nextPageResults = getPaginationData(group.id, undefined)?.nextPageResults; | ||
| const isPaginating = !!getIssueLoader(group.id); | ||
| const isPartialGroup = groupIssueIds ? groupIssueIds.length < groupIssueCount || !!nextPageResults : false; | ||
| const estimateSum = (() => { | ||
| if (!showEstimates) return null; | ||
| if (!groupIssueIds || groupIssueIds.length === 0) return null; | ||
|
|
||
| const firstIssue = issuesMap[groupIssueIds[0]]; | ||
| const issueProjectId = firstIssue?.project_id; | ||
| if (!issueProjectId) return null; | ||
|
|
||
| if (!areEstimateEnabledByProjectId(issueProjectId)) return null; | ||
|
|
||
| const activeEstimateId = currentActiveEstimateIdByProjectId(issueProjectId); | ||
| if (!activeEstimateId) return null; | ||
| const activeEstimate = estimateById(activeEstimateId); | ||
| if (!activeEstimate?.type || activeEstimate.type === EEstimateSystem.CATEGORIES) return null; | ||
|
|
||
| let sum = 0; | ||
| for (const issueId of groupIssueIds) { | ||
| const issue = issuesMap[issueId]; | ||
| if (!issue?.estimate_point) continue; | ||
| const point = activeEstimate.estimatePointById(issue.estimate_point); | ||
| if (!point?.value) continue; | ||
| const numericValue = Number(point.value); | ||
| if (!Number.isNaN(numericValue)) { | ||
| sum += numericValue; | ||
| } | ||
| } | ||
| return sum; | ||
| })(); |
Author
There was a problem hiding this comment.
I would argue that the current logic is the simplest approach and performance impact is limited, because groups are capped at 50 entries, correct? After that pagination kicks in. But I am ofc open for different perspectives and suggestions here.
Comment on lines
+274
to
+280
| group, | ||
| orderBy, | ||
| getGroupIndex, | ||
| setDragColumnOrientation, | ||
| setIsDraggingOverColumn, | ||
| isWorkflowDropDisabled | ||
| ]); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/core/components/issues/issue-layouts/list/list-group.tsx`:
- Around line 124-151: The group total in estimateSum currently uses the first
issue's project context; change the logic to compute per-issue project
estimates: inside the loop over groupIssueIds (using variables groupIssueIds and
issuesMap), for each issue read its project_id, skip if missing or if
areEstimateEnabledByProjectId(project_id) is false, fetch activeEstimateId via
currentActiveEstimateIdByProjectId(project_id) and resolve activeEstimate via
estimateById(activeEstimateId), skip if missing or of type
EEstimateSystem.CATEGORIES, then lookup the point with
activeEstimate.estimatePointById(issue.estimate_point) and add its numeric value
to sum; remove dependence on firstIssue/issueProjectId outside the loop so
mixed-project groups are handled correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 84f3368b-89b1-493f-a420-a3e0e23c8f36
📒 Files selected for processing (10)
apps/web/core/components/issues/issue-layouts/filters/header/display-filters/display-filters-selection.tsxapps/web/core/components/issues/issue-layouts/filters/header/display-filters/extra-options.tsxapps/web/core/components/issues/issue-layouts/list/base-list-root.tsxapps/web/core/components/issues/issue-layouts/list/default.tsxapps/web/core/components/issues/issue-layouts/list/headers/group-by-card.tsxapps/web/core/components/issues/issue-layouts/list/list-group.tsxpackages/constants/src/issue/filter.tspackages/i18n/src/locales/en/work-item.jsonpackages/types/src/view-props.tspackages/utils/src/work-item/base.ts
Comment on lines
+124
to
+151
| const estimateSum = (() => { | ||
| if (!showEstimates) return null; | ||
| if (!groupIssueIds || groupIssueIds.length === 0) return null; | ||
|
|
||
| const firstIssue = issuesMap[groupIssueIds[0]]; | ||
| const issueProjectId = firstIssue?.project_id; | ||
| if (!issueProjectId) return null; | ||
|
|
||
| if (!areEstimateEnabledByProjectId(issueProjectId)) return null; | ||
|
|
||
| const activeEstimateId = currentActiveEstimateIdByProjectId(issueProjectId); | ||
| if (!activeEstimateId) return null; | ||
| const activeEstimate = estimateById(activeEstimateId); | ||
| if (!activeEstimate?.type || activeEstimate.type === EEstimateSystem.CATEGORIES) return null; | ||
|
|
||
| let sum = 0; | ||
| for (const issueId of groupIssueIds) { | ||
| const issue = issuesMap[issueId]; | ||
| if (!issue?.estimate_point) continue; | ||
| const point = activeEstimate.estimatePointById(issue.estimate_point); | ||
| if (!point?.value) continue; | ||
| const numericValue = Number(point.value); | ||
| if (!Number.isNaN(numericValue)) { | ||
| sum += numericValue; | ||
| } | ||
| } | ||
| return sum; | ||
| })(); |
Contributor
There was a problem hiding this comment.
Use per-issue project estimate context when calculating group totals.
The current sum logic uses the first issue’s project_id for the whole group. In mixed-project groups, this can undercount or miscalculate totals for issues from other projects.
Suggested fix
-import { useEffect, useRef, useState } from "react";
+import { useEffect, useMemo, useRef, useState } from "react";
@@
- const estimateSum = (() => {
+ const estimateSum = useMemo(() => {
if (!showEstimates) return null;
if (!groupIssueIds || groupIssueIds.length === 0) return null;
-
- const firstIssue = issuesMap[groupIssueIds[0]];
- const issueProjectId = firstIssue?.project_id;
- if (!issueProjectId) return null;
-
- if (!areEstimateEnabledByProjectId(issueProjectId)) return null;
-
- const activeEstimateId = currentActiveEstimateIdByProjectId(issueProjectId);
- if (!activeEstimateId) return null;
- const activeEstimate = estimateById(activeEstimateId);
- if (!activeEstimate?.type || activeEstimate.type === EEstimateSystem.CATEGORIES) return null;
-
let sum = 0;
+ let hasEstimatedItems = false;
+
for (const issueId of groupIssueIds) {
const issue = issuesMap[issueId];
- if (!issue?.estimate_point) continue;
- const point = activeEstimate.estimatePointById(issue.estimate_point);
- if (!point?.value) continue;
+ if (!issue?.project_id || !issue?.estimate_point) continue;
+ if (!areEstimateEnabledByProjectId(issue.project_id)) continue;
+
+ const activeEstimateId = currentActiveEstimateIdByProjectId(issue.project_id);
+ if (!activeEstimateId) continue;
+ const activeEstimate = estimateById(activeEstimateId);
+ if (!activeEstimate?.type || activeEstimate.type === EEstimateSystem.CATEGORIES) continue;
+
+ const point = activeEstimate.estimatePointById(issue.estimate_point);
+ if (point?.value == null) continue;
const numericValue = Number(point.value);
if (!Number.isNaN(numericValue)) {
sum += numericValue;
+ hasEstimatedItems = true;
}
}
- return sum;
- })();
+ return hasEstimatedItems ? sum : null;
+ }, [
+ showEstimates,
+ groupIssueIds,
+ issuesMap,
+ areEstimateEnabledByProjectId,
+ currentActiveEstimateIdByProjectId,
+ estimateById,
+ ]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const estimateSum = (() => { | |
| if (!showEstimates) return null; | |
| if (!groupIssueIds || groupIssueIds.length === 0) return null; | |
| const firstIssue = issuesMap[groupIssueIds[0]]; | |
| const issueProjectId = firstIssue?.project_id; | |
| if (!issueProjectId) return null; | |
| if (!areEstimateEnabledByProjectId(issueProjectId)) return null; | |
| const activeEstimateId = currentActiveEstimateIdByProjectId(issueProjectId); | |
| if (!activeEstimateId) return null; | |
| const activeEstimate = estimateById(activeEstimateId); | |
| if (!activeEstimate?.type || activeEstimate.type === EEstimateSystem.CATEGORIES) return null; | |
| let sum = 0; | |
| for (const issueId of groupIssueIds) { | |
| const issue = issuesMap[issueId]; | |
| if (!issue?.estimate_point) continue; | |
| const point = activeEstimate.estimatePointById(issue.estimate_point); | |
| if (!point?.value) continue; | |
| const numericValue = Number(point.value); | |
| if (!Number.isNaN(numericValue)) { | |
| sum += numericValue; | |
| } | |
| } | |
| return sum; | |
| })(); | |
| const estimateSum = useMemo(() => { | |
| if (!showEstimates) return null; | |
| if (!groupIssueIds || groupIssueIds.length === 0) return null; | |
| let sum = 0; | |
| let hasEstimatedItems = false; | |
| for (const issueId of groupIssueIds) { | |
| const issue = issuesMap[issueId]; | |
| if (!issue?.project_id || !issue?.estimate_point) continue; | |
| if (!areEstimateEnabledByProjectId(issue.project_id)) continue; | |
| const activeEstimateId = currentActiveEstimateIdByProjectId(issue.project_id); | |
| if (!activeEstimateId) continue; | |
| const activeEstimate = estimateById(activeEstimateId); | |
| if (!activeEstimate?.type || activeEstimate.type === EEstimateSystem.CATEGORIES) continue; | |
| const point = activeEstimate.estimatePointById(issue.estimate_point); | |
| if (point?.value == null) continue; | |
| const numericValue = Number(point.value); | |
| if (!Number.isNaN(numericValue)) { | |
| sum += numericValue; | |
| hasEstimatedItems = true; | |
| } | |
| } | |
| return hasEstimatedItems ? sum : null; | |
| }, [ | |
| showEstimates, | |
| groupIssueIds, | |
| issuesMap, | |
| areEstimateEnabledByProjectId, | |
| currentActiveEstimateIdByProjectId, | |
| estimateById, | |
| ]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/core/components/issues/issue-layouts/list/list-group.tsx` around
lines 124 - 151, The group total in estimateSum currently uses the first issue's
project context; change the logic to compute per-issue project estimates: inside
the loop over groupIssueIds (using variables groupIssueIds and issuesMap), for
each issue read its project_id, skip if missing or if
areEstimateEnabledByProjectId(project_id) is false, fetch activeEstimateId via
currentActiveEstimateIdByProjectId(project_id) and resolve activeEstimate via
estimateById(activeEstimateId), skip if missing or of type
EEstimateSystem.CATEGORIES, then lookup the point with
activeEstimate.estimatePointById(issue.estimate_point) and add its numeric value
to sum; remove dependence on firstIssue/issueProjectId outside the loop so
mixed-project groups are handled correctly.
Author
There was a problem hiding this comment.
This seems like a valid point, I did not think of multi-project views. I will adapt the function to handle this correctly.
Contributor
There was a problem hiding this comment.
@laurin-eichberger, sounds great! The suggested useMemo-based fix in the comment above should cover multi-project groups correctly by resolving the estimate context per-issue rather than anchoring on the first issue's project. Feel free to ping if you run into anything while adapting it.
(🐰 ✨)
🧠 Learnings used
Learnt from: aaryan610
Repo: makeplane/plane PR: 8537
File: apps/web/core/components/issues/preview-card/root.tsx:51-51
Timestamp: 2026-03-05T07:50:17.119Z
Learning: In Tailwind CSS v4, wrap-break-word is a built-in overflow-wrap utility (not a custom class) and should not be flagged as invalid. The v4 naming convention uses wrap-* for overflow-wrap utilities (e.g., wrap-break-word, wrap-anywhere, wrap-normal) instead of the older break-words class. When reviewing TSX files, recognize wrap-break-word as valid Tailwind usage and prefer wrap-* utilities for overflow-wrap; avoid flagging these as errors and consider updating any older references to break-words to the corresponding wrap-* form where appropriate.
Learnt from: dkumankov
Repo: makeplane/plane PR: 9216
File: apps/web/core/components/dropdowns/layout.tsx:77-77
Timestamp: 2026-06-05T06:06:41.884Z
Learning: In makeplane/plane, Tailwind CSS v4 uses the important modifier (`!`) as a **suffix** placed immediately after the utility class (e.g., `w-auto!`, `bg-layer-transparent-hover!`, `px-2!`). When reviewing files in this repo, do **not** flag these suffix-important utilities as invalid Tailwind syntax. The v3 prefix form `!utility` / `!classname` (e.g., `!w-auto`) is considered invalid for Tailwind v4—flag that form instead.
Author
|
@sriramveeraghanta while working on this I noticed that the pre-commit hooks identified 14 warnings via eslint plugins, but AFAIK all were pre-existing and unrelated to the changes in this PR. How should I handle these? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds estimate point sum badges to list view group headers when using a summable estimate system (Points or Time).
≥prefix when the group has more items not yet loaded (partial sum)Type of Change
Screenshots and Media (if applicable)
Activated estimations for groups show group estimation totals
Activated estimations for paginated groups (>50) show a lower bound based on loaded issues
Test Scenarios
≥prefixReferences
Resolves #9213
Summary by CodeRabbit