Conversation
WalkthroughThe changes introduce a new translation key ( Changes
Sequence Diagram(s)sequenceDiagram
participant G as IssueGanttBlock
participant U as Utils:findTotalDaysInRange
participant S as IssueStats
G->>U: Calculate duration (start_date, target_date)
U-->>G: Return duration
alt duration ≥ 2 and isEpic true
G->>S: Render IssueStats (with showProgressText true)
else
G->>G: Skip rendering IssueStats
end
sequenceDiagram
participant K as KanbanIssueDetailsBlock
participant H as WithDisplayPropertiesHOC
participant S as IssueStats
K->>K: Compute subIssueCount & isEpic flag
alt isEpic true and displayProperties available
K->>H: Wrap IssueStats component
H->>S: Render IssueStats with displayProperties
else
K->>K: Render without IssueStats
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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:
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: 0
🧹 Nitpick comments (3)
packages/i18n/src/locales/ru/translations.json (1)
985-986: New Translation Key for Issue Count AddedThe addition of the
"issue_count": "Количество рабочих элементов"key is implemented correctly and fits well with the surrounding translation entries. The naming is consistent with the expected functionality in the UI, and the translation appears appropriate for a count of work items. Please verify that this term aligns with similar keys in other locale files to maintain consistency across the application.packages/i18n/src/locales/en/translations.json (1)
818-819: New Translation Key Addition for Issue Count.
The new key"issue_count": "Work item count"has been added alongside the"sub_issue"entry, which retains its previous value. Please verify that this key and its value remain consistent with the application's overall terminology and styling, and that similar changes in other locale files follow the same conventions. Additionally, if this count is meant to support pluralization or dynamic rendering, consider using an ICU message format (e.g.,{count, plural, one{# work item} other{# work items}}) to handle different cases gracefully.web/core/components/issues/issue-layouts/list/block.tsx (1)
273-281: Inconsistency in shouldRenderProperty condition compared to kanban block.While the implementation is generally good, there's a slight inconsistency between this implementation and the one in the kanban block. In the kanban block, the
shouldRenderPropertyfunction checks both!!properties.sub_issue_count && !!subIssueCount, but here it only checks!!properties.sub_issue_count.For consistency, consider updating the condition to also check for the existence of sub-issues:
- shouldRenderProperty={(properties) => !!properties.sub_issue_count} + shouldRenderProperty={(properties) => !!properties.sub_issue_count && !!subIssuesCount}This would ensure consistent behavior across different view types.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
packages/i18n/src/locales/en/translations.json(1 hunks)packages/i18n/src/locales/es/translations.json(1 hunks)packages/i18n/src/locales/fr/translations.json(1 hunks)packages/i18n/src/locales/it/translations.json(1 hunks)packages/i18n/src/locales/ja/translations.json(1 hunks)packages/i18n/src/locales/ru/translations.json(1 hunks)packages/i18n/src/locales/zh-CN/translations.json(1 hunks)web/ce/components/issues/issue-layouts/issue-stats.tsx(1 hunks)web/core/components/issues/issue-layouts/filters/header/display-filters/display-properties.tsx(1 hunks)web/core/components/issues/issue-layouts/gantt/blocks.tsx(4 hunks)web/core/components/issues/issue-layouts/kanban/block.tsx(3 hunks)web/core/components/issues/issue-layouts/list/block.tsx(2 hunks)web/core/components/issues/issue-layouts/properties/all-properties.tsx(1 hunks)web/core/components/issues/issue-layouts/spreadsheet/columns/sub-issue-column.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (24)
web/ce/components/issues/issue-layouts/issue-stats.tsx (2)
7-10: Good addition of optional props for flexibility.The addition of these optional properties enhances the component's flexibility and customization options. This allows the component to be used in various contexts with different styling and display behavior requirements.
13-13:Details
❓ Verification inconclusive
Implementation simplified to an empty fragment.
The component implementation has been greatly simplified from what was likely a more complex implementation to just returning an empty fragment. This suggests this might be part of a refactoring process where the actual implementation will be provided elsewhere or in a future update.
This empty implementation seems intentional as part of a refactoring, but I'd suggest verifying that:
- The actual implementation exists elsewhere in the codebase
- This placeholder doesn't cause any rendering issues where it's being used
🏁 Script executed:
#!/bin/bash # Search for any other implementations or imports of IssueStats echo "Searching for implementations of IssueStats component..." rg "IssueStats" -A 5 -B 5 --glob "*.{tsx,jsx}"Length of output: 9367
Placeholder Implementation for IssueStats: Verify Usage & Future Implementation
The
IssueStatscomponent inweb/ce/components/issues/issue-layouts/issue-stats.tsxcurrently returns an empty fragment. A repository-wide search confirms that this component is imported and used in several places (e.g., kanban, list, gantt, spreadsheet). This suggests that the simplified implementation is intentional as part of a refactoring process.
- Verify that the placeholder is not causing any rendering or functionality gaps where it’s used.
- Ensure that the intended, fully implemented version of
IssueStatswill replace this placeholder in a timely manner.packages/i18n/src/locales/zh-CN/translations.json (1)
986-987: Translation key added for issue count.Adding the "issue_count" translation key enhances localization support for the application, making it more accessible to Chinese-speaking users. This addition aligns with the changes in the display properties filter component.
web/core/components/issues/issue-layouts/filters/header/display-filters/display-properties.tsx (1)
55-57: Enhanced localization for epic issue counts.The code now uses the translation key "issue.display.properties.issue_count" for epics instead of a hardcoded string. This change improves internationalization support by ensuring that text is properly translated based on the user's locale settings.
This change correctly integrates with the new translation key added to the localization files, ensuring a consistent user experience across different languages.
packages/i18n/src/locales/it/translations.json (1)
984-985: New Translation Key Added: "issue_count"
The"issue_count": "Conteggio degli elementi di lavoro"entry has been added to the display section as part of the issue stats refactor. This addition aligns well with the UI changes and maintains consistency with similar keys in other locale files. Please ensure that all locale files are updated similarly for a consistent user experience.packages/i18n/src/locales/es/translations.json (1)
988-989: Translation Key Addition VerificationThe new
"issue_count": "Recuento de elementos de trabajo"key has been added to the"display"properties of the"issue"object. The translation is clear and consistent with the structure used in other locale files. Please verify that the UI components referencing this key have been updated accordingly.packages/i18n/src/locales/fr/translations.json (1)
986-987: New Translation Key Addition: "issue_count"The new key
"issue_count": "Nombre d'éléments de travail"is clearly added in the display section of the issue entity. The translation is concise and consistent with the existing terminology (e.g.,"sub_issue": "Sous-élément de travail"). Please double-check that its naming and usage align with the other locale files to ensure uniformity across the application.packages/i18n/src/locales/ja/translations.json (1)
986-987: Translation addition looks good.The new translation key "issue_count" with the Japanese translation "作業項目数" (work item count) has been properly added to the display section of the issue object.
web/core/components/issues/issue-layouts/properties/all-properties.tsx (1)
432-463: Improved conditional rendering logic for issue statistics.The code now properly conditionally renders the sub-issue count component only for non-epic issues. This change improves the control flow by ensuring that this component is only rendered when appropriate.
The implementation uses the
WithDisplayPropertiesHOCwith appropriate checks to only render when both:
- The display property is enabled (
!!properties.sub_issue_count)- There are actual sub-issues to display (
!!subIssueCount)This is a good pattern that prevents showing empty UI elements when there's no data to display.
web/core/components/issues/issue-layouts/kanban/block.tsx (2)
67-67: Good addition to calculate subIssueCount.Adding this derived value makes the code more clear and consistent with the issue handling in other components.
114-122: Well-implemented conditional rendering for epic issue stats.The code correctly renders the
IssueStatscomponent for epics with appropriate conditional logic. The HOC pattern withWithDisplayPropertiesHOCensures that stats are only shown when display properties allow it and when there are actual sub-issues to display.The consistent use of the same HOC pattern as in other files maintains code cohesion across the application.
web/core/components/issues/issue-layouts/gantt/blocks.tsx (7)
8-8: Improvement: Added utility function import for date range calculationsThe addition of the
findTotalDaysInRangeutility function from@plane/utilsenables proper calculation of the duration between start and target dates, which is used for conditional rendering logic.
20-20: Enhancement: Added IssueStats component importThe IssueStats component is properly imported to display work item statistics for epics on the Gantt chart view.
52-53: Improvement: Added duration calculation for conditional renderingThe duration calculation provides a metric to determine when to show progress text in the IssueStats component, improving the UI by only showing progress text when there's sufficient duration (≥ 2 days).
72-72: LGTM - Background overlay adjustmentThe background overlay provides proper contrast for content visibility.
74-74: Proper flex layout adjustmentAdding
flex-1to this element enables it to properly expand within the flex container, working together with the newly added IssueStats component.
79-85: Enhancement: Conditionally render IssueStats for epicsGood implementation of conditional rendering for the IssueStats component. The component is only shown for epics and includes appropriate styling and configuration:
- Proper passing of the issue ID
- Appropriate styling with
className- Smart conditional showing of progress text based on duration
This enhances the Gantt view by providing additional contextual information for epic work items.
68-68:Details
❓ Verification inconclusive
Verify styling classes
The addition of
space-betweenandbg-red-500classes may have unintended visual effects. Thebg-red-500class applies a red background which might conflict with the intended dynamic background fromblockStyle.
🏁 Script executed:
#!/bin/bash # Check if bg-red-500 is used elsewhere in the codebase rg "bg-red-500" --type tsx --type jsxLength of output: 65
I'll run a new script that searches for the
"bg-red-500"class across.tsxand.jsxfiles by explicitly specifying file globs. This should help us verify whether that class is being used elsewhere:Once we have the output from this command, we can better assess if the usage of
"bg-red-500"is intentional or might be conflicting with the dynamic background styles (fromblockStyle) as originally noted. Please run the above script and share the results if further manual verification is needed.
🏁 Script executed:
#!/bin/bash # Search for "bg-red-500" in all .tsx and .jsx files rg "bg-red-500" --glob "*.tsx" --glob "*.jsx"Length of output: 5749
Verify and Confirm Dynamic Background and Flex Alignment
Please ensure that the hardcoded styling in the Gantt block is intentional:
- Background Conflict: The static
bg-red-500class is applied here even though the component seems designed to support a dynamic background viablockStyle. Given thatbg-red-500is also widely used across the codebase (albeit sometimes with opacity modifiers), verify that this does not inadvertently override or conflict with the dynamic background logic.- Flex Alignment Utility: The use of
space-betweenraises a question. If the intent is to distribute child elements within a flex container, confirm whether this class is a custom utility or if it should instead bejustify-betweenper standard Tailwind CSS conventions.Please manually verify the visual effects in this component to ensure that the intended dynamic styling is preserved.
web/core/components/issues/issue-layouts/spreadsheet/columns/sub-issue-column.tsx (6)
11-11: Enhancement: Added IssueStats component importThe IssueStats component is properly imported to display work item statistics in the spreadsheet view.
22-22: Refactor: Simplified parameter destructuringRemoved unused
epicIdfrom parameter destructuring, simplifying the code.
24-24: Improvement: Derived isEpic from issue propertiesGood approach to determine if an issue is an epic directly from its properties rather than from URL parameters. This makes the component more reusable and less dependent on routing structure.
29-29: Refactor: Updated URL construction based on issue typeUpdated URL construction to use the
isEpicflag for proper navigation. This ensures correct routing to either "epics" or "issues" based on the issue type.
33-33: Simplification: Consistent terminology for sub-work itemsSimplified label construction to consistently use "sub-work item" terminology regardless of issue type, improving consistency in the UI.
45-45: Enhancement: Conditional rendering based on issue typeGood implementation of conditional rendering that shows either:
- The IssueStats component with appropriate props for epics
- The simple text label for regular issues
This enhances the spreadsheet view by providing richer information for epic work items while keeping the interface clean for regular issues.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/core/components/issues/issue-layouts/gantt/blocks.tsx (2)
52-53: Consider documenting the significance of the duration value.The duration calculation is implemented correctly with a proper fallback to 0. However, it would be helpful to add a comment explaining why this calculation is needed and its significance in the context of the component.
+ // Calculate the duration between start and target dates to determine if progress text should be shown const duration = findTotalDaysInRange(issueDetails?.start_date, issueDetails?.target_date) || 0;
79-85: Well-implemented conditional rendering with appropriate props.The conditional rendering of the
IssueStatscomponent is well implemented with the following observations:
- It only shows for epics (
isEpic && (...)), which aligns with the component's purpose.- The
showProgressTextprop uses a threshold of duration >= 2, which appears to be a design decision to only show progress text when there's a meaningful duration.- The styling is consistent with the rest of the component.
Consider documenting why the threshold of 2 days was chosen, as this might not be immediately obvious to other developers.
{isEpic && ( <IssueStats issueId={issueId} className="sticky mx-2 font-medium text-custom-text-100 overflow-hidden truncate w-auto justify-end flex-shrink-0" - showProgressText={duration >= 2} + showProgressText={duration >= 2} // Only show progress text when duration is at least 2 days /> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/core/components/issues/issue-layouts/gantt/blocks.tsx(4 hunks)web/core/components/issues/issue-layouts/spreadsheet/columns/sub-issue-column.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/core/components/issues/issue-layouts/spreadsheet/columns/sub-issue-column.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
web/core/components/issues/issue-layouts/gantt/blocks.tsx (5)
8-8: Good addition of utility function for date range calculation.The import of
findTotalDaysInRangeis a good choice for calculating the duration between dates, which is used appropriately later in the code.
20-20: Good component reuse for issue statistics.The import of the
IssueStatscomponent follows good modular design principles, allowing for reuse across different layouts.
68-68: Good flex layout adjustment for the new component.The addition of
space-betweento the class list improves the layout by ensuring proper spacing between the issue name and the newly added stats component.
74-74: Appropriate flex property for better text handling.Adding
flex-1to the issue name container ensures it takes available space while still allowing the stats component to fit properly.
52-85:Details
❓ Verification inconclusive
Verify component behavior with edge cases.
Since the
IssueStatscomponent is conditionally rendered based on theisEpicflag and shows progress text based on the duration, ensure it behaves correctly when:
- Start date or target date is missing
- The dates are invalid or in the wrong order
- Other data that might be used by the
IssueStatscomponent is missing
🏁 Script executed:
#!/bin/bash # Find how IssueStats handles missing or invalid dates # Check IssueStats implementation echo "Checking IssueStats implementation..." fd -t f "issue-stats.(tsx|js)" -x cat {} # Check how findTotalDaysInRange handles edge cases echo "Checking findTotalDaysInRange implementation..." fd -t f -e ts -e js -x grep -l "findTotalDaysInRange" {} \; | xargs catLength of output: 37881
Edge Case Testing for IssueStats Rendering and Date Calculation
Please verify that the Gantt block component handles edge cases properly when computing and using the duration for conditionally rendering IssueStats:
- Missing Dates: Confirm that if either
issueDetails.start_dateorissueDetails.target_dateis undefined or missing, thefindTotalDaysInRangehelper returns undefined (falling back to 0 via|| 0), so thatshowProgressTextremains false.- Invalid or Reversed Dates: Test scenarios where the provided dates are invalid (e.g., wrong format) or in the wrong order (start date after target date). Ensure that the calculated duration is appropriately 0 (or negative, resulting in
showProgressTextbeing false) so no misleading progress text is shown.- Missing Additional Data: Verify that if other related properties (such as
issueDetails.nameormessage) are missing, the component still renders without errors—even though the IssueStats component (currently an empty placeholder) won’t display progress details.
* fix: issue stats refactor (#6705) * fix: issue stats refactor * fix: refactor * fix: ui color * fix: translation key * refactor: issue list modal refactor (#6702) --------- Co-authored-by: Akshita Goyal <36129505+gakshita@users.noreply.github.com> Co-authored-by: Vamsi Krishna <46787868+vamsikrishnamathala@users.noreply.github.com> Co-authored-by: sriram veeraghanta <veeraghanta.sriram@gmail.com>
* fix: issue stats refactor * fix: refactor * fix: ui color * fix: translation key
3 participants
Description
This PR has some refactoring changes for rendering issue stats
Summary by CodeRabbit