chore: code refactor and build fix #6285
Conversation
|
Warning Rate limit exceeded@anmolsinghbhatia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 29 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces comprehensive modifications across multiple components to enhance epic issue handling. The changes primarily involve adding an optional Changes
Suggested labels
Suggested reviewers
Poem
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
web/core/components/issues/issue-layouts/properties/all-properties.tsx
Outdated
Show resolved
Hide resolved
![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 (19)
web/core/services/issue/issue.service.ts (1)
425-437: Leverage stronger typing for new bulk subscription method.
ReturningPromise<any>is flexible but omits type safety. Consider specifying a return interface to improve maintainability.web/core/components/issues/issue-detail/issue-activity/sort-root.tsx (2)
12-13: Add JSDoc for newly introduced props.
AlthoughclassNameandiconClassNameare straightforward, adding short descriptions in either code comments or JSDoc helps maintain clarity for future contributors.
27-29: Verify icon styling across different screen sizes.
It may be worth verifying whethersize-4remains consistent on high-DPI displays or in responsive layouts, especially when combined withprops.iconClassName.web/core/components/issues/issue-layouts/filters/header/display-filters/issue-grouping.tsx (2)
14-14: Optional boolean naming suggestion.
isEpicis sufficiently descriptive, but consider naming booleans more contextually (e.g.,groupingEpic) if it needs to convey an action or state more explicitly.
38-38: Maintain consistent naming for user-facing text.
Appending "Epics" or "Issues" is clear. Verify that the same logic applies to other screens or modals if they also display these titles.web/core/components/issues/issue-layouts/spreadsheet/columns/sub-issue-column.tsx (3)
27-27: Prevent confusion in route definition.
When building a long route string, consider splitting into smaller, more readable pieces or using a utility function. This helps readability and reduces the chance of missing slash characters.- router.push(`/${workspaceSlug?.toString()}/projects/${issue.project_id}/${issue.archived_at ? "archives/" : ""}${epicId ? "epics" : "issues"}/${issue.id}#sub-issues`); + const archivePart = issue.archived_at ? "archives/" : ""; + const entityPart = epicId ? "epics" : "issues"; + router.push(`/${workspaceSlug}/projects/${issue.project_id}/${archivePart}${entityPart}/${issue.id}#sub-issues`);
31-32: Simplify label calculation if appropriate.
The creation ofissueLabelis fine for clarity. If usage evolves, consider extracting to a separate helper function or constant for reusability across the codebase.
44-44: Consider adding a tooltip or hover state.
If the sub-issue count is zero, the label is static text. Providing user feedback (e.g., "No sub-issues") might improve clarity, especially if the rest of the row is interactive.web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-header-column.tsx (1)
18-25: Avoid blowing uppropswith unstructured arguments.
Consider grouping new booleans or related parameters into configuration objects if you anticipate more toggles. This keeps the top-level ofpropstidy.web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-header.tsx (2)
24-24: Add clarity to theisEpicnaming.
While addingisEpic?: boolean;is straightforward, consider whether the naming could be more descriptive, for exampleisEpicVieworshowEpic. This would more clearly indicate how the prop is used.
67-67: String literal can be extracted for maintainability.
Consider extracting"Epics"/"Issues"to constants for localization or future changes.web/core/components/issues/delete-issue-modal.tsx (1)
99-106: Well-handled dynamic modal content.
Conditionally displaying text for "epic" vs. "issue" offers clarity to end-users. Consider extracting repeated strings to constants if multiple modals use this logic.web/core/components/issues/issue-layouts/spreadsheet/columns/header-column.tsx (1)
50-50: Conditional label renaming.
Replacing "Sub-issue" with "Issues" whenisEpicistrueis a neat approach to maintaining clarity. However, if future states require more dynamic labeling (e.g., localization), centralizing label management in a constants file or i18n solution may be preferable.web/core/components/issues/issue-detail-widgets/sub-issues/content.tsx (1)
56-59: Duplicate hook call usage.
Here,useIssueDetail()is called twice with different signatures—one retains the default usage, and the other receivesissueServiceType. If the first call is purely to destructure outtoggleCreateIssueModalandtoggleDeleteIssueModal, consider merging them into a single call to reduce cognitive load.- const { toggleCreateIssueModal, toggleDeleteIssueModal } = useIssueDetail(); - const { - subIssues: { subIssueHelpersByIssueId, setSubIssueHelpers }, - } = useIssueDetail(issueServiceType); + const { + toggleCreateIssueModal, + toggleDeleteIssueModal, + subIssues: { subIssueHelpersByIssueId, setSubIssueHelpers }, + } = useIssueDetail(issueServiceType);web/core/components/issues/issue-layouts/gantt/base-gantt-root.tsx (1)
124-125: Use of conditional titles.
SettingtitleandloaderTitleto"Epics"or"Issues"depending onisEpicis clear and straightforward. If more states arise, consider abstracting the logic into a helper function or config to avoid scattering string checks.web/core/store/issue/issue-details/sub_issues.store.ts (1)
68-68: Missing visibility / type annotation
Consider adding an explicit type toserviceTypefor better readability, for instance:public serviceType: EIssueServiceType;web/core/components/issues/issue-layouts/properties/all-properties.tsx (1)
367-406: Double check the!isEpiccondition
There’s a nested!isEpic &&block within another!isEpic &&. This seems redundant. You can remove the inner duplication for clarity:- {!isEpic && ( - {!isEpic && ( - ... + {!isEpic && ( + ...web/core/store/issue/helpers/base-issues.store.ts (1)
675-675: Typo in comment
Change “Male API call” to “Make API call” for clarity.- // Male API call + // Make API callweb/core/components/issues/issue-layouts/spreadsheet/spreadsheet-table.tsx (1)
Line range hint
35-52: Consider extracting scroll handling logic into a custom hookThe scroll handling logic could be moved to a custom hook (e.g.,
useTableScroll) to improve code organization and reusability. This would make the component more focused on its primary responsibility of rendering the spreadsheet table.Example implementation:
// hooks/use-table-scroll.ts export const useTableScroll = (containerRef: MutableRefObject<HTMLTableElement | null>) => { const isScrolled = useRef(false); const handleScroll = useCallback(() => { // ... existing scroll handling logic ... }, [containerRef]); useEffect(() => { const currentContainerRef = containerRef.current; if (currentContainerRef) currentContainerRef.addEventListener("scroll", handleScroll); return () => { if (currentContainerRef) currentContainerRef.removeEventListener("scroll", handleScroll); }; }, [handleScroll, containerRef]); return { isScrolled }; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
web/core/components/issues/delete-issue-modal.tsx(3 hunks)web/core/components/issues/filters.tsx(2 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/content.tsx(1 hunks)web/core/components/issues/issue-detail-widgets/sub-issues/title.tsx(1 hunks)web/core/components/issues/issue-detail/issue-activity/sort-root.tsx(1 hunks)web/core/components/issues/issue-layouts/calendar/base-calendar-root.tsx(1 hunks)web/core/components/issues/issue-layouts/calendar/issue-blocks.tsx(1 hunks)web/core/components/issues/issue-layouts/filters/header/display-filters/display-filters-selection.tsx(3 hunks)web/core/components/issues/issue-layouts/filters/header/display-filters/display-properties.tsx(3 hunks)web/core/components/issues/issue-layouts/filters/header/display-filters/issue-grouping.tsx(3 hunks)web/core/components/issues/issue-layouts/filters/header/filters/filters-selection.tsx(3 hunks)web/core/components/issues/issue-layouts/gantt/base-gantt-root.tsx(2 hunks)web/core/components/issues/issue-layouts/group-drag-overlay.tsx(3 hunks)web/core/components/issues/issue-layouts/list/list-group.tsx(2 hunks)web/core/components/issues/issue-layouts/properties/all-properties.tsx(12 hunks)web/core/components/issues/issue-layouts/quick-add/button/gantt.tsx(2 hunks)web/core/components/issues/issue-layouts/quick-add/button/kanban.tsx(1 hunks)web/core/components/issues/issue-layouts/quick-add/button/list.tsx(1 hunks)web/core/components/issues/issue-layouts/quick-add/button/spreadsheet.tsx(2 hunks)web/core/components/issues/issue-layouts/spreadsheet/columns/header-column.tsx(2 hunks)web/core/components/issues/issue-layouts/spreadsheet/columns/sub-issue-column.tsx(2 hunks)web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-header-column.tsx(2 hunks)web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-header.tsx(4 hunks)web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-table.tsx(1 hunks)web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-view.tsx(1 hunks)web/core/components/issues/parent-issues-list-modal.tsx(1 hunks)web/core/components/issues/sub-issues/issues-list.tsx(1 hunks)web/core/constants/empty-state.ts(6 hunks)web/core/constants/issue.ts(1 hunks)web/core/services/issue/issue.service.ts(9 hunks)web/core/store/issue/helpers/base-issues.store.ts(1 hunks)web/core/store/issue/issue-details/issue.store.ts(3 hunks)web/core/store/issue/issue-details/sub_issues.store.ts(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- web/core/constants/issue.ts
🔇 Additional comments (77)
web/core/components/issues/issue-layouts/filters/header/filters/filters-selection.tsx (3)
45-45: Introduce isEpic property.
The newly added optional isEpic property provides a clear and concise way to differentiate between epic and standard issues. This addition appears consistent with the rest of the props definition.
60-60: Default value for isEpic.
Setting the default value of isEpic to false ensures backward compatibility for existing usages. Great way to handle an optional prop.
239-239: Passing isEpic to FilterIssueGrouping.
Using the isEpic prop here is a neat approach to control conditional rendering. Confirm that FilterIssueGrouping gracefully handles isEpic in all cases.
web/core/components/issues/parent-issues-list-modal.tsx (1)
72-73: Ensure conditional parameters align with intended search logic.
When searchEpic is true, parent is omitted while epic is set to true. Conversely, when searchEpic is false, parent is true and epic is omitted. Confirm that the projectIssuesSearch method expects these param values for epic vs. parent searches, and that this logic accurately reflects the intended behavior.
Run the following script to search for usage of projectIssuesSearch elsewhere in the codebase and validate the parameter expectations:
✅ Verification successful
Let me check the type definition for the search parameters to fully verify the logic.
The parameter logic for epic vs. parent searches is correctly implemented
Based on the type definition and usage analysis:
TProjectIssuesSearchParamsshows bothparentandepicare optional boolean parameters- The current implementation correctly sets:
- For epic search:
epic: trueandparent: undefined - For parent search:
parent: trueandepic: undefined
- For epic search:
- Other usages of
projectIssuesSearchin the codebase confirm this pattern of mutually exclusive parameters
The conditional logic accurately reflects the intended behavior by ensuring only one of these parameters is set at a time, preventing any ambiguity in the search criteria.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Explore the usage of projectIssuesSearch across the codebase to confirm parameter usage.
rg -A 10 "projectIssuesSearch"
Length of output: 5078
Script:
#!/bin/bash
# Search for the type definition of TProjectIssuesSearchParams
ast-grep --pattern 'type TProjectIssuesSearchParams = {
$$$
}'
# Also search for interface definition as an alternative
ast-grep --pattern 'interface TProjectIssuesSearchParams {
$$$
}'
Length of output: 941
web/core/services/issue/issue.service.ts (8)
46-46: Use consistent approach for the detail endpoint.
Using a separate -detail/ endpoint when issue_relation is expanded is a good approach for clarity. However, confirm that additional query combinations (e.g., group_by with issue_relation) do not require special handling.
79-81: Fallback logic is clear and effective.
Falling back to server data when serviceType isn't ISSUES or when offline caching doesn't apply is a sensible approach.
116-117: Local update skip ensures consistency for epics.
By skipping updateIssue for epics, you avoid stale local data. This is correct for maintaining separate data flows for epics and issues.
132-132: Bulk-adding local issues is well integrated.
Conditionally adding issues to the local DB only for ISSUES fits your specialized data partitioning logic.
238-240: Conditional local deletion is correct.
Removing the issue locally only when serviceType is ISSUES aligns with the separation of epic data storage.
342-344: Bulk operations sync for issues is appropriate.
Only syncing local issues for EIssueServiceType.ISSUES helps avoid conflicts with epic data.
361-363: Bulk delete sync is scoped properly.
Synchronizing local issues after bulk deletions is essential when handling ISSUES.
382-384: Bulk archive sync is consistently handled.
Limiting local sync to ISSUES maintains the distinction between issues and epics.
web/core/components/issues/issue-layouts/quick-add/button/kanban.tsx (1)
7-15: Conditional label for epic creation is effective.
Using isEpic with a default of false and displaying “New Epic” vs. “New Issue” is clear and maintains UI consistency.
web/core/components/issues/issue-layouts/quick-add/button/list.tsx (1)
8-16: Dynamic button label refactor is straightforward.
The introduction of isEpic ensures clarity when adding either epics or standard issues, maintaining a cohesive UI.
web/core/components/issues/issue-layouts/quick-add/button/spreadsheet.tsx (1)
Line range hint 7-17: Spreadsheet add button handles epics gracefully.
Applying isEpic to render “New Epic” or “New Issue” aligns with the approach seen in other quick-add components.
web/core/components/issues/issue-layouts/quick-add/button/gantt.tsx (1)
Line range hint 8-18: Gantt quick-add supports epic creation seamlessly.
This addition improves consistency across all quick-add buttons by using the isEpic prop.
web/core/components/issues/issue-detail/issue-activity/sort-root.tsx (1)
17-21: Validate container styling behavior.
Ensure that combining getButtonStyling("neutral-primary", "sm") with any externally passed props.className won't create conflicting styles. A potential pitfall is having both utility classes and overrides, which can be confusing if not documented.
web/core/components/issues/issue-layouts/filters/header/display-filters/issue-grouping.tsx (2)
18-18: Leverage default assignment in destructuring.
Good use of isEpic = false. This pattern keeps default values top of mind for future maintainers. Looks good.
27-27: Confirm consistent label usage.
Switching the title between "Epic Grouping" and "Issue Grouping" is beneficial for clarity. Double-check that all references to the grouping label (e.g., help text, tooltips) also dynamically adjust.
web/core/components/issues/issue-layouts/spreadsheet/columns/sub-issue-column.tsx (1)
21-21: Confirm presence of epicId across routes.
Ensure that all routes setting epicId in the URL params are standardized. Inconsistent route naming could break navigation.
web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-header-column.tsx (2)
15-15: Check for naming parity across components.
isEpic is introduced here. Validate that parallel components or relevant context use the same naming or pass state in a consistent manner (e.g., isEpic, issueIsEpic).
50-50: Check child component synergy.
isEpic is forwarded to HeaderColumn. Confirm that HeaderColumn indeed uses it and includes any fallback logic or default behavior.
web/core/components/issues/sub-issues/issues-list.tsx (1)
63-63: Prop usage is consistent with the default assignment.
The addition of issueServiceType aligns well with the default value of EIssueServiceType.ISSUES and ensures each IssueListItem is aware of the correct service type. This is a clean approach that maintains backward compatibility.
web/core/components/issues/issue-detail-widgets/sub-issues/title.tsx (1)
41-41: Dynamic title for Epics looks good.
Conditionally rendering "Issues" vs. "Sub-issues" based on the issueServiceType offers clarity in distinguishing epics from regular issues.
web/core/components/issues/issue-layouts/group-drag-overlay.tsx (3)
19-19: New isEpic prop introduced.
Making the isEpic prop optional retains flexibility. This is consistent with the overall epic-issue distinction.
31-31: Default value set to false.
Providing a safe default ensures that existing usage isn’t disrupted and sets clear expectations for when epic behavior is toggled on.
73-73: Nice user-facing feedback for epics.
Displaying "epic" vs. "issue" in the drop overlay gives clear context to users when reordering items.
web/core/components/issues/issue-layouts/filters/header/display-filters/display-properties.tsx (3)
19-19: isEpic prop for flexible display.
Adding isEpic?: boolean continues the pattern of conditionally handling epic-specific logic in the component.
29-29: Defaulting to false ensures backward compatibility.
This approach keeps the existing behavior unchanged unless explicitly set to true.
50-54: Conditional renaming of “sub_issue_count” property.
Replacing “Sub-issue count” with “Issue count” when isEpic is true improves clarity by matching the epic context.
web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-header.tsx (2)
36-36: Good default for isEpic.
Having a default of false ensures backward compatibility and prevents errors if the prop is omitted. Great job!
79-79: Prop forwarding is consistent.
Forwarding isEpic to child components helps maintain a single source of truth for epic/issue contexts. Ensure child components fully cover edge cases.
web/core/components/issues/delete-issue-modal.tsx (4)
21-21: isEpic?: boolean extends functionality seamlessly.
Adding this optional prop aligns with the rest of the epic-based changes.
25-25: Defaulting isEpic to false prevents unintended behavior.
This default helps avoid errors if the implementation forgets to pass isEpic.
73-73: Clear success message for epics.
Conditionally displaying "Sub-issue", "Epic", or "Issue" is user-friendly and clarifies the type of deletion.
78-80: Permission error message ensures user awareness.
Explicitly mentioning "epic" in the permission error helps users troubleshoot and understand the requirements for deletion.
web/core/components/issues/issue-layouts/calendar/issue-blocks.tsx (1)
90-90: Prop usage for epic context is consistent.
Forwarding isEpic to CalendarQuickAddIssueActions ensures a consistent approach to epic/issue handling throughout the calendar layout.
web/core/components/issues/issue-layouts/filters/header/display-filters/display-filters-selection.tsx (3)
28-28: isEpic property aligns with filtering needs.
Introducing isEpic?: boolean; accommodates specialized filtering or display for epic entities without impacting standard issues.
41-41: Good default assignment for optional prop.
Initializing isEpic to false ensures components function correctly in non-epic contexts.
66-66: Prop usage for sub-component flexibility.
Passing isEpic to FilterDisplayProperties extends epic context without major refactors. Verify child logic for all possible states.
web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-view.tsx (1)
120-120: Prop propagation looks appropriate.
Passing the isEpic prop into QuickAddIssueRoot nicely keeps the logic consistent throughout. Ensure that wherever QuickAddIssueRoot is used, isEpic is handled uniformly for potential epic-specific behaviors.
web/core/components/issues/issue-layouts/spreadsheet/columns/header-column.tsx (1)
18-22: Optional prop addition is consistent.
Introducing isEpic in the Props interface and destructuring it with a default value of false aligns with the rest of the codebase. This ensures that if the prop is not passed, the component gracefully defaults to standard issues handling without throwing errors.
web/core/components/issues/issue-layouts/gantt/base-gantt-root.tsx (1)
114-114: Prop forwarding for epic context.
Forwarding isEpic to QuickAddIssueRoot is consistent with other layout updates. This is crucial for epic-specific quick-add functionalities.
web/core/components/issues/filters.tsx (2)
126-126: Prop usage looks correct.
Using storeType === EIssuesStoreType.EPIC to set isEpic is consistent with the rest of the application logic. This allows the FilterSelection component to handle epic-related filtering conditions seamlessly.
138-138: Consistent isEpic forwarding.
Forwarding the same logical check to DisplayFiltersSelection ensures both components remain synchronized about the epic state. No issues identified.
web/core/components/issues/issue-layouts/calendar/base-calendar-root.tsx (1)
53-53: Conditional storeType assignment.
This conditional assignment to storeType is a neat approach to force the epic store type when isEpic is true, while otherwise using the value from useIssueStoreType(). It’s clear and maintainable.
web/core/store/issue/issue-details/issue.store.ts (3)
52-52: New epicService property introduced.
Defining a dedicated epicService field is a clean approach that keeps epic-related operations separate. Ensure to remove it if future refactors no longer require dedicated epic logic.
66-66: Initialization of epicService.
Instantiating the epicService with EIssueServiceType.EPICS aligns with the chosen design. This clarifies the intended usage and helps with consistent code organization.
98-100: Restricting local DB fetch to non-epic issues.
This conditional ensures that epics do not trigger local DB fetches. It cleanly separates epic retrieval logic from standard issue logic.
web/core/components/issues/issue-layouts/list/list-group.tsx (2)
287-287: Overlay epic state alignment.
Injecting isEpic into the GroupDragOverlay helps maintain a consistent epic vs. non-epic state. No performance or logical issues observed.
316-316: Quick add button epic support.
Passing isEpic to QuickAddIssueRoot is a straightforward enhancement that allows the list layout quick add functionality to differentiate between epics and standard issues when necessary.
web/core/store/issue/issue-details/sub_issues.store.ts (5)
7-7: Import statement clarity
Good call extracting EIssueServiceType from @plane/constants; it helps disambiguate service usage.
89-89: Initializing service type
Looks good for clarity in the constructor. This ensures IssueService creation aligns with the appropriate service type.
188-191: Conditional persistence
The condition if (this.serviceType === EIssueServiceType.ISSUES) ensures local updates only for standard issues. The usage of updatePersistentLayer is consistent with the rest of the codebase.
289-291: Consistent logic for removing sub-issue
Repeating the updatePersistentLayer logic within this condition is consistent with the approach in createSubIssues. No issues found.
326-328: Maintaining local updates for deleted sub-issues
This aligns with the existing pattern to only handle standard “issues” for local DB updates. Nicely done.
web/core/components/issues/issue-layouts/properties/all-properties.tsx (13)
3-3: Extended import for SyntheticEvent
Including SyntheticEvent clarifies the event type for handleEventPropagation. Good improvement for type safety.
248-248: Conditional epic path
Appending "epics" vs. "issues" in the URL clarifies the routing logic. Ensure all routes calling redirectToIssueDetail pass the correct isEpic flag.
268-268: Stopping propagation
Using SyntheticEvent<HTMLDivElement> is more precise, and calling both e.stopPropagation() and e.preventDefault() prevents unintended clicks or focusing. Good approach.
278-278: Focus vs. click events
Applying onFocus and onClick to the same handler is helpful for accessibility. Verify no undesired double handler triggers.
294-294: Priority dropdown
Same pattern here—good consistency across different property dropdowns.
309-309: Date dropdown
Maintaining uniform event handling approach.
327-327: Due date focus
Again, symmetrical approach with onFocus and onClick.
347-347: Assignees
The onFocus + onClick approach is consistent. No issues found.
414-414: Estimate dropdown
Identical event handling. Looks consistent.
436-436: Tooltip heading
Switching title to “Issues” for epics is a nice contextual detail.
442-442: Inline event
In this inline onClick handler, you’re again preventing the default. This is consistent with the rest of the code. No issues found.
475-475: Attachment count
Same approach—no concerns.
498-498: Link count
Same approach—no concerns.
web/core/constants/empty-state.ts (6)
32-34: Extended enum for workspace
Good expansions for teams and initiatives. No issues found.
102-102: Active cycles
Adding WORKSPACE_ACTIVE_CYCLES clarifies the new scenario for cycles at workspace level.
117-121: Added new TEAM enumerations
Defines team-based empty states. Ensure usage is consistent in the UI.
736-742: Workspace active cycles entry
Properly addresses scenario for “No active cycles” at workspace level.
843-845: Add epic creation
Expanding the empty state to direct the user to create an epic is in line with the rest of the epic changes.
849-904: Team states
New placeholders for “Team” concepts (no issues, filtering, views, pages). Matches consistent usage with other empty states.
web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-table.tsx (2)
Line range hint 20-33: LGTM: Props type definition is well-structured
The addition of the optional isEpic boolean prop maintains backward compatibility while enabling epic-specific functionality.
Line range hint 114-134: Verify isEpic handling in child components
The isEpic prop is passed to both SpreadsheetHeader and SpreadsheetIssueRow. Please ensure these components properly handle the prop for epic-specific rendering.
Additionally, consider using React Context if this prop needs to be passed down multiple levels, as this could reduce prop drilling.
✅ Verification successful
isEpic prop is properly handled in both components
Based on the verification results, both SpreadsheetHeader and SpreadsheetIssueRow components properly handle the isEpic prop:
-
In
SpreadsheetHeader:- The prop is properly typed in the interface
- It's used to conditionally render "Epics" or "Issues" in the header text
- It's passed down to
SpreadsheetHeaderColumnfor further customization
-
In
SpreadsheetIssueRow:- The prop is properly typed and defaulted to
false - It affects the issue service type selection (
EIssueServiceType.EPICSvsEIssueServiceType.ISSUES) - It's used for navigation URLs (
/epicsvs/issues) - It prevents sub-issue expansion for epics (
!isEpic && subIssues?.map) - It's properly passed down to child components
- The prop is properly typed and defaulted to
The prop drilling concern is not significant here as:
- The prop is used at each level where it's passed
- The component hierarchy is not deep enough to warrant React Context
- The prop affects core functionality and routing logic
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for SpreadsheetHeader and SpreadsheetIssueRow components to verify isEpic prop handling
ast-grep --pattern 'interface Props {
$$$
isEpic?: boolean
$$$
}' web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-header.tsx web/core/components/issues/issue-layouts/spreadsheet/issue-row.tsx
# Check for conditional rendering based on isEpic
rg -A 5 'isEpic[^a-zA-Z]' web/core/components/issues/issue-layouts/spreadsheet/spreadsheet-header.tsx web/core/components/issues/issue-layouts/spreadsheet/issue-row.tsx
Length of output: 12293
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
web/core/components/issues/issue-layouts/properties/all-properties.tsx (1)
Line range hint
248-256: Remove commented-out codeThe routing logic update looks good, but the commented-out code should be removed as it adds unnecessary noise. If needed, this code can be retrieved from version control history.
- // router.push({ - // pathname: `/${workspaceSlug}/projects/${issue.project_id}/${issue.archived_at ? "archives/" : ""}issues/${ - // issue.id - // }`, - // hash: "sub-issues", - // });
🧹 Nitpick comments (1)
web/core/components/issues/issue-layouts/properties/all-properties.tsx (1)
365-407: Consider simplifying the conditional structureWhile the Fragment usage is valid here, the nested conditionals could be simplified for better readability.
- <> - {!isEpic && ( - <> - {/* modules */} - {projectDetails?.module_view && ( - <WithDisplayPropertiesHOC>...</WithDisplayPropertiesHOC> - )} - {/* cycles */} - {projectDetails?.cycle_view && ( - <WithDisplayPropertiesHOC>...</WithDisplayPropertiesHOC> - )} - </> - )} - </> + {!isEpic && ( + <> + {/* modules */} + {projectDetails?.module_view && ( + <WithDisplayPropertiesHOC>...</WithDisplayPropertiesHOC> + )} + {/* cycles */} + {projectDetails?.cycle_view && ( + <WithDisplayPropertiesHOC>...</WithDisplayPropertiesHOC> + )} + </> + )}🧰 Tools
🪛 Biome (1.9.4)
[error] 365-407: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/core/components/issues/issue-layouts/properties/all-properties.tsx(12 hunks)web/core/constants/empty-state.ts(6 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
web/core/components/issues/issue-layouts/properties/all-properties.tsx
[error] 365-407: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (4)
web/core/constants/empty-state.ts (1)
32-34: LGTM! Well-structured empty state configurations.
The new empty state configurations for workspace and team-related features are well-organized, maintain consistent naming conventions, and provide clear, user-friendly messages. The access controls are properly set, and the descriptions effectively guide users.
Also applies to: 102-102, 117-121, 177-188, 190-205, 736-742, 850-893
web/core/components/issues/issue-layouts/properties/all-properties.tsx (3)
3-3: LGTM! Type enhancement for event handling
The addition of SyntheticEvent import and type change provides better type coverage for both mouse and focus events.
268-271: LGTM! Improved event handling
The event handler correctly prevents event propagation and default behavior for both mouse and focus events.
278-278: LGTM! Enhanced accessibility with focus handling
The addition of onFocus handlers alongside onClick improves keyboard navigation and accessibility.
Also applies to: 294-294, 309-309, 327-327, 347-347, 371-371, 391-391, 412-412, 440-440, 473-473, 496-496
web/core/constants/empty-state.ts
Outdated
| access: [EUserPermissions.ADMIN], | ||
| }, | ||
| [EmptyStateType.WORKSPACE_INITIATIVES]: { | ||
| key: EmptyStateType.WORKSPACE_TEAMS, |
There was a problem hiding this comment.
Fix incorrect key in WORKSPACE_INITIATIVES empty state.
The key is incorrectly set to EmptyStateType.WORKSPACE_TEAMS instead of EmptyStateType.WORKSPACE_INITIATIVES.
Apply this fix:
- key: EmptyStateType.WORKSPACE_TEAMS,
+ key: EmptyStateType.WORKSPACE_INITIATIVES,📝 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.
| key: EmptyStateType.WORKSPACE_TEAMS, | |
| key: EmptyStateType.WORKSPACE_INITIATIVES, |
| key: EmptyStateType.TEAM_PAGE, | ||
| title: "Team pages are coming soon!", | ||
| description: | ||
| "Write a note, a doc, or a full knowledge base. Get Galileo, Plane’s AI assistant, to help you get started. Pages are thoughts potting space in Plane. Take down meeting notes, format them easily, embed issues, lay them out using a library of components, and keep them all in your project’s context. To make short work of any doc, invoke Galileo, Plane’s AI, with a shortcut or the click of a button.", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider revising the team pages description.
The title indicates "coming soon" but the description suggests the feature is already available. This might confuse users.
Consider updating the description to align with the "coming soon" status:
- title: "Team pages are coming soon!",
- description:
- "Write a note, a doc, or a full knowledge base. Get Galileo, Plane's AI assistant, to help you get started. Pages are thoughts potting space in Plane. Take down meeting notes, format them easily, embed issues, lay them out using a library of components, and keep them all in your project's context. To make short work of any doc, invoke Galileo, Plane's AI, with a shortcut or the click of a button.",
+ title: "Team pages are coming soon!",
+ description:
+ "Soon you'll be able to create and manage pages specifically for your team. Stay tuned for updates on this exciting feature!",📝 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.
| key: EmptyStateType.TEAM_PAGE, | |
| title: "Team pages are coming soon!", | |
| description: | |
| "Write a note, a doc, or a full knowledge base. Get Galileo, Plane’s AI assistant, to help you get started. Pages are thoughts potting space in Plane. Take down meeting notes, format them easily, embed issues, lay them out using a library of components, and keep them all in your project’s context. To make short work of any doc, invoke Galileo, Plane’s AI, with a shortcut or the click of a button.", | |
| key: EmptyStateType.TEAM_PAGE, | |
| title: "Team pages are coming soon!", | |
| description: | |
| "Soon you'll be able to create and manage pages specifically for your team. Stay tuned for updates on this exciting feature!", |
Description
This PR includes following changes:
Type of Change
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores