-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[WEB-2848] refactor: enhance components modularity and introduce new UI componenets #6192
Conversation
…ion functionality
…props for mentions
WalkthroughThe pull request introduces various enhancements across multiple files, primarily focusing on expanding the options for issue management and improving component functionality. Key changes include the addition of new enum values, types, and properties related to team projects, as well as the introduction of several new components and modifications to existing ones. These updates enhance the flexibility and usability of the application, particularly in managing issues and team-related functionalities. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 16
🧹 Outside diff range and nitpick comments (46)
web/core/components/project/index.ts (1)
21-22
: Consider grouping related exportsThe new exports for project selection and search functionality could be grouped with similar functionality. Consider moving these exports next to related ones like
member-select
orsearch-projects
near other UI interaction exports.web/core/components/project/search-projects.tsx (2)
54-62
: Enhance input accessibility and performanceThe search input should have proper ARIA attributes and debounced search updates for better performance.
<input ref={inputRef} + role="searchbox" + aria-label="Search projects" className="w-full max-w-[234px] border-none bg-transparent text-sm text-custom-text-100 placeholder:text-custom-text-400 focus:outline-none" placeholder="Search" value={searchQuery} - onChange={(e) => updateSearchQuery(e.target.value)} + onChange={debounce((e) => updateSearchQuery(e.target.value), 300)} onKeyDown={handleInputKeyDown} />Consider adding the following import at the top:
import { debounce } from "lodash";
13-78
: Consider extracting UI components and constantsThe component could benefit from better organization:
- Extract the search button and input into separate components
- Move magic numbers and classes to constants
- Consider using a custom hook for search logic
Example structure:
// hooks/useSearch.ts export const useSearch = (initialQuery = "") => { const { searchQuery, updateSearchQuery } = useProjectFilter(); const [isOpen, setIsOpen] = useState(false); // ... rest of the logic return { searchQuery, isOpen, handleKeyDown, handleToggle, handleClose }; }; // components/SearchButton.tsx export const SearchButton = ({ onClick }: { onClick: () => void }) => ( <button aria-label="Toggle search" ...> <Search className="h-3.5 w-3.5" /> </button> ); // components/SearchInput.tsx export const SearchInput = ({ value, onChange, ... }) => ( <input role="searchbox" ... /> );web/core/components/project/header.tsx (2)
5-16
: Organize imports by categoryConsider organizing imports into clear categories with consistent spacing:
import { observer } from "mobx-react"; import { usePathname } from "next/navigation"; import { Briefcase } from "lucide-react"; + // ui import { Breadcrumbs, Button, Header } from "@plane/ui"; + // components import { BreadcrumbLink } from "@/components/common"; +import HeaderFilters from "./filters"; +import { ProjectSearch } from "./search-projects"; + // hooks import { useCommandPalette, useEventTracker, useUserPermissions } from "@/hooks/store"; + // constants import { EUserPermissions, EUserPermissionsLevel } from "@/plane-web/constants/user-permissions"; - -// components -import HeaderFilters from "./filters"; -import { ProjectSearch } from "./search-projects";
44-44
: Consider composition pattern for header itemsThe ProjectSearch component could be wrapped in a Header.SearchItem component for better composition and consistency.
-<ProjectSearch /> +<Header.SearchItem> + <ProjectSearch /> +</Header.SearchItem>web/ce/constants/dashboard.ts (2)
73-80
: Alignkey
andlabel
for consistencyThe menu item with
key: "all-issues"
has alabel: "Views"
. This may cause confusion. Consider updating either the key to"views"
or the label to"All issues"
for consistency and clarity.
99-104
: Simplify the creation ofSIDEBAR_WORKSPACE_MENU_ITEMS
Since
SIDEBAR_WORKSPACE_MENU
properties are defined and not expected to beundefined
, the optional chaining (?.
) may be unnecessary. Removing it can simplify the code.Apply this diff to remove optional chaining:
export const SIDEBAR_WORKSPACE_MENU_ITEMS: TSidebarWorkspaceMenuItems[] = [ - SIDEBAR_WORKSPACE_MENU?.projects, - SIDEBAR_WORKSPACE_MENU?.["all-issues"], - SIDEBAR_WORKSPACE_MENU?.["active-cycles"], - SIDEBAR_WORKSPACE_MENU?.analytics, + SIDEBAR_WORKSPACE_MENU.projects, + SIDEBAR_WORKSPACE_MENU["all-issues"], + SIDEBAR_WORKSPACE_MENU["active-cycles"], + SIDEBAR_WORKSPACE_MENU.analytics, ].filter((item): item is TSidebarWorkspaceMenuItems => item !== undefined);packages/ui/src/tabs/tabs.tsx (1)
53-57
: OptimizeuseEffect
dependency arrayThe
storageKey
is included in the dependency array but not used inside the effect. This could trigger unnecessary re-renders. RemovestorageKey
from the dependency array.Apply this diff:
useEffect(() => { if (storeInLocalStorage) { setValue(selectedTab); } - }, [selectedTab, setValue, storeInLocalStorage, storageKey]); + }, [selectedTab, setValue, storeInLocalStorage]);web/core/components/dropdowns/project.tsx (1)
145-151
: Limit the number of icons displayed for multiple selectionsDisplaying all project icons when multiple projects are selected may clutter the UI. Consider limiting the number of icons shown and indicating the total count.
Apply this diff to limit displayed icons:
{Array.isArray(value) && ( <div className="flex items-center gap-0.5"> - {value.map((projectId) => { + {value.slice(0, 3).map((projectId) => { const projectDetails = getProjectById(projectId); return projectDetails ? renderIcon(projectDetails) : null; })} + {value.length > 3 && ( + <span className="text-xs">+{value.length - 3}</span> + )} </div> )}web/core/hooks/use-issues-actions.tsx (1)
Line range hint
70-71
: Resolve TypeScript type mismatch instead of suppressing itUsing
//@ts-expect-error
to suppress a type mismatch hides potential issues and may lead to runtime errors. It's important to address the underlying type incompatibility betweenworkspaceDraftIssueActions
and the expected return typeIssueActions
. Please ensure thatworkspaceDraftIssueActions
conforms to theIssueActions
interface or adjust the interface to accommodate necessary differences.Apply this diff to remove the type suppression and fix the mismatch:
case EIssuesStoreType.WORKSPACE_DRAFT: - //@ts-expect-error type mismatch return workspaceDraftIssueActions;
web/ce/components/issues/filters/team-project.tsx (1)
6-10
: Add JSDoc documentation for Props interface.Consider adding JSDoc documentation to describe the purpose and usage of each prop.
+/** + * Props for the FilterTeamProjects component + * @param appliedFilters - Currently applied team project filters + * @param handleUpdate - Callback to update the selected filter + * @param searchQuery - Current search query for filtering team projects + */ type Props = { appliedFilters: string[] | null; handleUpdate: (val: string) => void; searchQuery: string; };web/ce/helpers/dashboard.helper.ts (1)
7-8
: Add return type and documentation for the helper function.The function's purpose and return value should be documented for better maintainability.
+/** + * Checks if a specific feature is enabled for a workspace + * @param featureKey - The feature to check + * @param workspaceSlug - The workspace identifier + * @returns boolean indicating if the feature is enabled + */ // eslint-disable-next-line @typescript-eslint/no-unused-vars -export const isWorkspaceFeatureEnabled = (featureKey: TSidebarWorkspaceMenuItemKeys, workspaceSlug: string) => true; +export const isWorkspaceFeatureEnabled = ( + featureKey: TSidebarWorkspaceMenuItemKeys, + workspaceSlug: string +): boolean => true;packages/types/src/common.d.ts (1)
26-26
: LGTM! Consider adding error state.The new type
TNameDescriptionLoader
effectively captures the common submission states. However, consider adding an "error" state to handle failed submissions more explicitly.-export type TNameDescriptionLoader = "submitting" | "submitted" | "saved"; +export type TNameDescriptionLoader = "submitting" | "submitted" | "saved" | "error";web/ce/store/issue/team-views/issue.store.ts (1)
5-13
: Consider architectural alternatives to inheritanceThis file follows the same pattern as
team/issue.store.ts
, suggesting a systemic architectural concern. Both files:
- Extend base classes that "will never be used"
- Use @ts-nocheck to bypass type checking
- Have unnecessary constructors
Consider:
- Using composition over inheritance
- Creating a proper abstraction layer
- Implementing interfaces directly instead of extending unused classes
Example architectural alternative:
interface IssueStore { // Define common interface } class BaseIssueStore implements IssueStore { // Implement common functionality } export class TeamViewIssues implements IssueStore { private baseStore: BaseIssueStore; constructor(_rootStore: IIssueRootStore, teamViewFilterStore: ITeamViewIssuesFilter) { this.baseStore = new BaseIssueStore(_rootStore, teamViewFilterStore); } // Implement interface methods using composition }Would you like me to create a detailed proposal for this architectural change?
🧰 Tools
🪛 Biome (1.9.4)
[error] 10-12: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
web/ce/store/command-palette.store.ts (1)
19-24
: Consider enhancing the documentationWhile the documentation explains what the method does, it would be helpful to include:
- The return value from
getCoreModalsState()
- Example scenarios when this would return true/false
packages/ui/src/icons/lead-icon.tsx (1)
5-26
: Enhance accessibility and customization of LeadIcon componentWhile the SVG implementation is clean, consider the following improvements:
export const LeadIcon: React.FC<ISvgIcons> = ({ className = "text-current", ...rest }) => ( <svg className={className} viewBox="0 0 19 18" fill="none" xmlns="http://www.w3.org/2000/svg" + role="img" + aria-label="Lead icon" {...rest} > + <title>Lead Icon</title> <path d="M0.571533 9C0.571533 4.02944 4.60097 0 9.57153 0C14.5421 0 18.5715 4.02944 18.5715 9C18.5715 13.9706 14.5421 18 9.57153 18C4.60097 18 0.571533 13.9706 0.571533 9Z" - fill="#3372FF" + className="fill-current text-blue-500" /> <g clip-path="url(#clip0_8992_2377)"> <circle cx="9.57153" cy="6.5" r="2.5" fill="#F5F5FF" /> <path fill-rule="evenodd" clip-rule="evenodd" d="M8.94653 9.625C6.53029 9.625 4.57153 11.5838 4.57153 14H9.57153H14.5715C14.5715 11.5838 12.6128 9.625 10.1965 9.625H9.82153L10.8215 13.0278L9.57153 14L8.32153 13.0278L9.32153 9.625H8.94653Z" - fill="#F5F5FF" + className="fill-current text-white" /> </g> <defs> - <clipPath id="clip0_8992_2377"> + <clipPath id={`lead-icon-clip-${Math.random().toString(36).substr(2, 9)}`}> <rect width="10" height="10" fill="white" transform="translate(4.57153 4)" /> </clipPath> </defs> </svg> );web/core/hooks/use-issue-layout-store.ts (2)
29-32
: Consider reorganizing store type conditions for better maintainabilityThe team-related conditions should be grouped with similar conditions for better code organization.
if (globalViewId) return EIssuesStoreType.GLOBAL; if (userId) return EIssuesStoreType.PROFILE; + // Group all view-related conditions together + if (teamId && viewId) return EIssuesStoreType.TEAM_VIEW; if (viewId) return EIssuesStoreType.PROJECT_VIEW; if (cycleId) return EIssuesStoreType.CYCLE; if (moduleId) return EIssuesStoreType.MODULE; if (projectId) return EIssuesStoreType.PROJECT; if (teamId) return EIssuesStoreType.TEAM; - if (teamId && viewId) return EIssuesStoreType.TEAM_VIEW; return EIssuesStoreType.PROJECT;
11-11
: Add type safety for router parametersConsider adding type safety for the destructured parameters from useParams.
- const { globalViewId, viewId, projectId, cycleId, moduleId, userId, teamId } = useParams(); + const { globalViewId, viewId, projectId, cycleId, moduleId, userId, teamId } = useParams() as { + globalViewId?: string; + viewId?: string; + projectId?: string; + cycleId?: string; + moduleId?: string; + userId?: string; + teamId?: string; + };web/ce/constants/issues.ts (1)
24-24
: Consider inlining the TActivityFilters type aliasSince
TActivityFilters
is just an alias forEActivityFilterType
, consider usingEActivityFilterType
directly for better code clarity.- key: TActivityFilters; + key: EActivityFilterType;web/core/components/cycles/list/cycle-list-project-group-header.tsx (2)
12-17
: Props interface looks good, consider adding validation.The props interface is well-typed, but consider adding runtime validation for the
count
prop to ensure it's non-negative.type Props = { projectId: string; - count?: number; + count?: number; // Should be validated to ensure non-negative showCount?: boolean; isExpanded?: boolean; };
19-26
: Consider adding error boundary and memoization.The component correctly uses the observer pattern, but could benefit from:
- Error boundary to handle potential errors from
getProjectById
- Memoization of the project lookup
-export const CycleListProjectGroupHeader: FC<Props> = observer((props) => { +export const CycleListProjectGroupHeader: FC<Props> = observer(React.memo((props) => { const { projectId, count, showCount = false, isExpanded = false } = props; // store hooks const { getProjectById } = useProject(); // derived values - const project = getProjectById(projectId); + const project = React.useMemo(() => getProjectById(projectId), [getProjectById, projectId]);web/core/components/cycles/analytics-sidebar/root.tsx (1)
49-50
: Consider memoizing child components for performance optimization.While the props propagation is correct, consider wrapping child components with React.memo to prevent unnecessary re-renders when other props change.
+ const MemoizedCycleSidebarHeader = React.memo(CycleSidebarHeader); + const MemoizedCycleSidebarDetails = React.memo(CycleSidebarDetails); + const MemoizedCycleAnalyticsProgress = React.memo(CycleAnalyticsProgress); return ( <div className="relative pb-2"> <div className="flex flex-col gap-5 w-full"> - <CycleSidebarHeader + <MemoizedCycleSidebarHeader workspaceSlug={workspaceSlug} projectId={projectId} cycleDetails={cycleDetails} isArchived={isArchived} handleClose={handleClose} /> - <CycleSidebarDetails + <MemoizedCycleSidebarDetails projectId={projectId} cycleDetails={cycleDetails} /> </div> {workspaceSlug && projectId && cycleDetails?.id && ( - <CycleAnalyticsProgress + <MemoizedCycleAnalyticsProgress workspaceSlug={workspaceSlug} projectId={projectId} cycleId={cycleDetails?.id} /> )} </div> );Also applies to: 55-55, 59-59
web/core/hooks/use-local-storage.tsx (2)
24-24
: Track the TODO comment for migration.The TODO comment indicates a planned migration. Let's ensure this is tracked properly.
Would you like me to create a GitHub issue to track this migration task?
Line range hint
24-53
: Consider enhancing type safety for stored values.While the hook uses generics, we could improve type safety for stored values.
- const useLocalStorage = <T,>(key: string, initialValue: T) => { + const useLocalStorage = <T extends JsonValue,>(key: string, initialValue: T) => { + type JsonValue = string | number | boolean | null | JsonValue[] | { [key: string]: JsonValue };web/core/components/issues/issue-modal/components/project-select.tsx (1)
Line range hint
33-57
: Consider extracting form validation rules.The form validation rules could be extracted into a constant for better maintainability and reuse.
+ const PROJECT_VALIDATION_RULES = { + required: true, + } as const; return ( <Controller control={control} name="project_id" - rules={{ - required: true, - }} + rules={PROJECT_VALIDATION_RULES} render={({ field: { value, onChange } }) =>web/core/components/editor/rich-text-editor/rich-text-editor.tsx (1)
25-25
: Consider memoizing memberDetails transformationThe
memberDetails
array is recreated on every render by mapping over memberIds. Consider memoizing this transformation to prevent unnecessary recalculations.- const memberDetails = memberIds?.map((id) => getUserDetails(id) as IUserLite); + const memberDetails = useMemo( + () => memberIds?.map((id) => getUserDetails(id) as IUserLite), + [memberIds, getUserDetails] + );Also applies to: 28-28, 32-32, 37-37
web/core/components/cycles/cycle-peek-overview.tsx (1)
Line range hint
45-60
: Consider extracting sidebar styles to a CSS moduleThe inline styles and className could be moved to a CSS module for better maintainability and reusability.
Consider creating a new file
cycle-peek-overview.module.css
:.sidebarContainer { @apply flex h-full w-full max-w-[21.5rem] flex-shrink-0 flex-col gap-3.5 overflow-y-auto border-l border-custom-border-100 bg-custom-sidebar-background-100 px-4 duration-300 fixed md:relative right-0 z-[9]; box-shadow: 0px 1px 4px 0px rgba(0, 0, 0, 0.06), 0px 2px 4px 0px rgba(16, 24, 40, 0.06), 0px 1px 8px -1px rgba(16, 24, 40, 0.06); }web/core/components/dropdowns/layout.tsx (1)
19-22
: Consider adding type assertion for ISSUE_LAYOUT_MAP valuesThe filter operation assumes ISSUE_LAYOUT_MAP values have a 'key' property. Consider adding type assertion or interface to ensure type safety.
+ interface IssueLayout { + key: EIssueLayoutTypes; + label: string; + icon: React.FC; + } const availableLayouts = useMemo( - () => Object.values(ISSUE_LAYOUT_MAP).filter((layout) => !disabledLayouts.includes(layout.key)), + () => Object.values(ISSUE_LAYOUT_MAP as Record<string, IssueLayout>) + .filter((layout) => !disabledLayouts.includes(layout.key)), [disabledLayouts] );packages/ui/src/icons/teams.tsx (2)
5-13
: Consider moving props spreading to the SVG elementCurrently, props are spread on the first path element. For better maintainability and to ensure all SVG-related props are properly handled, consider moving the props spreading to the SVG element itself.
- <svg className={className} viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"> + <svg className={className} viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg" {...rest}> <path fillRule="evenodd" clipRule="evenodd" d="M8.25 6.75C8.25 5.75544 8.64509 4.80161 9.34835 4.09835C10.0516 3.39509 11.0054 3 12 3C12.9946 3 13.9484 3.39509 14.6517 4.09835C15.3549 4.80161 15.75 5.75544 15.75 6.75C15.75 7.74456 15.3549 8.69839 14.6517 9.40165C13.9484 10.1049 12.9946 10.5 12 10.5C11.0054 10.5 10.0516 10.1049 9.34835 9.40165C8.64509 8.69839 8.25 7.74456 8.25 6.75ZM15.75 9.75C15.75 8.95435 16.0661 8.19129 16.6287 7.62868C17.1913 7.06607 17.9544 6.75 18.75 6.75C19.5456 6.75 20.3087 7.06607 20.8713 7.62868C21.4339 8.19129 21.75 8.95435 21.75 9.75C21.75 10.5456 21.4339 11.3087 20.8713 11.8713C20.3087 12.4339 19.5456 12.75 18.75 12.75C17.9544 12.75 17.1913 12.4339 16.6287 11.8713C16.0661 11.3087 15.75 10.5456 15.75 9.75ZM2.25 9.75C2.25 8.95435 2.56607 8.19129 3.12868 7.62868C3.69129 7.06607 4.45435 6.75 5.25 6.75C6.04565 6.75 6.80871 7.06607 7.37132 7.62868C7.93393 8.19129 8.25 8.95435 8.25 9.75C8.25 10.5456 7.93393 11.3087 7.37132 11.8713C6.80871 12.4339 6.04565 12.75 5.25 12.75C4.45435 12.75 3.69129 12.4339 3.12868 11.8713C2.56607 11.3087 2.25 10.5456 2.25 9.75ZM6.31 15.117C6.91995 14.161 7.76108 13.3743 8.75562 12.8294C9.75016 12.2846 10.866 11.9994 12 12C12.9498 11.9991 13.8891 12.1989 14.7564 12.5862C15.6237 12.9734 16.3994 13.5395 17.0327 14.2474C17.6661 14.9552 18.1428 15.7888 18.4317 16.6936C18.7205 17.5985 18.815 18.5541 18.709 19.498C18.696 19.6153 18.6556 19.7278 18.591 19.8265C18.5263 19.9252 18.4393 20.0073 18.337 20.066C16.4086 21.1725 14.2233 21.7532 12 21.75C9.695 21.75 7.53 21.138 5.663 20.066C5.56069 20.0073 5.47368 19.9252 5.40904 19.8265C5.34441 19.7278 5.30396 19.6153 5.291 19.498C5.12305 17.9646 5.48246 16.4198 6.31 15.118V15.117Z" fill="currentColor" - {...rest} />
5-19
: Add accessibility attributes to the SVGTo improve accessibility, consider adding
role="img"
andaria-label
attributes to the SVG element.- <svg className={className} viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"> + <svg + className={className} + viewBox="0 0 24 24" + fill="none" + xmlns="http://www.w3.org/2000/svg" + role="img" + aria-label="Teams icon" + {...rest} + >web/core/components/project/card-list.tsx (2)
16-19
: Add prop validation for project IDsConsider adding runtime validation for the project IDs arrays to ensure they contain valid string values.
type TProjectCardListProps = { - totalProjectIds?: string[]; - filteredProjectIds?: string[]; + totalProjectIds?: string[] & { [K in number]: string }; + filteredProjectIds?: string[] & { [K in number]: string }; };
34-35
: Consider memoizing derived valuesThe derived project IDs could be memoized using
useMemo
to prevent unnecessary recalculations on re-renders.+ const workspaceProjectIds = useMemo( + () => totalProjectIdsProps ?? storeWorkspaceProjectIds, + [totalProjectIdsProps, storeWorkspaceProjectIds] + ); + const filteredProjectIds = useMemo( + () => filteredProjectIdsProps ?? storeFilteredProjectIds, + [filteredProjectIdsProps, storeFilteredProjectIds] + ); - const workspaceProjectIds = totalProjectIdsProps ?? storeWorkspaceProjectIds; - const filteredProjectIds = filteredProjectIdsProps ?? storeFilteredProjectIds;web/core/components/inbox/modals/create-modal/issue-description.tsx (2)
Line range hint
37-37
: Implement or remove TODO commentThe TODO comment about GPT Assistance implementation should either be addressed or removed if no longer relevant.
Would you like me to help implement the GPT Assistance feature or create a GitHub issue to track this task?
Line range hint
85-97
: Enhance error handling in uploadFileThe current error handling only logs the error and throws a generic message. Consider:
- Adding specific error types
- Providing more detailed error messages
- Implementing retry logic for transient failures
uploadFile={async (file) => { try { const { asset_id } = await fileService.uploadProjectAsset( workspaceSlug, projectId, { entity_identifier: data.id ?? "", entity_type: EFileAssetType.ISSUE_DESCRIPTION, }, file ); onAssetUpload?.(asset_id); return asset_id; } catch (error) { - console.log("Error in uploading issue asset:", error); - throw new Error("Asset upload failed. Please try again later."); + console.error("Error uploading issue asset:", error); + if (error instanceof TypeError) { + throw new Error("Network error while uploading. Please check your connection."); + } + if (error.response?.status === 413) { + throw new Error("File size exceeds the maximum limit."); + } + throw new Error(`Upload failed: ${error.message || 'Please try again later.'}`); } }}web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/[cycleId]/page.tsx (1)
80-85
: LGTM! Consider destructuring params earlierThe addition of required props to
CycleDetailsSidebar
improves component modularity. However, we could optimize the code by destructuring and converting params to strings once, at the top of the component.- const { workspaceSlug, projectId, cycleId } = useParams(); + const params = useParams(); + const workspaceSlug = params.workspaceSlug.toString(); + const projectId = params.projectId.toString(); + const cycleId = params.cycleId.toString(); // Then use these variables directly without toString() calls <CycleDetailsSidebar handleClose={toggleSidebar} - cycleId={cycleId.toString()} - projectId={projectId.toString()} - workspaceSlug={workspaceSlug.toString()} + cycleId={cycleId} + projectId={projectId} + workspaceSlug={workspaceSlug} />packages/ui/src/breadcrumbs/navigation-dropdown.tsx (2)
10-14
: Consider adding documentation for the props interfaceThe type definition would benefit from JSDoc comments explaining the purpose of each prop and providing examples.
+/** + * Props for the BreadcrumbNavigationDropdown component + * @property {string} selectedItemKey - The key of the currently selected item + * @property {TContextMenuItem[]} navigationItems - Array of navigation items to display + * @property {boolean} [navigationDisabled] - Whether navigation is disabled + */ type TBreadcrumbNavigationDropdownProps = { selectedItemKey: string; navigationItems: TContextMenuItem[]; navigationDisabled?: boolean; };
16-26
: Add error boundary protectionThe component should be wrapped in an error boundary to gracefully handle runtime errors.
+ import { ErrorBoundary } from 'react-error-boundary'; export const BreadcrumbNavigationDropdown = (props: TBreadcrumbNavigationDropdownProps) => { const { selectedItemKey, navigationItems, navigationDisabled = false } = props; + const fallbackComponent = () => ( + <span className="text-custom-text-200">Navigation unavailable</span> + ); + return ( + <ErrorBoundary FallbackComponent={fallbackComponent}> {/* existing component code */} + </ErrorBoundary> + ); };web/ce/components/cycles/active-cycle/root.tsx (2)
42-82
: Optimize useMemo dependenciesThe memoization of ActiveCyclesComponent is a good optimization, but the dependencies array could be more specific:
cycleIssueDetails
is an object and might cause unnecessary re-renders- Consider deriving specific values from
cycleIssueDetails
that are actually used in the component- [cycleId, activeCycle, workspaceSlug, projectId, handleFiltersUpdate, cycleIssueDetails] + [cycleId, activeCycle?.id, workspaceSlug, projectId, handleFiltersUpdate, + cycleIssueDetails?.completed_issues, cycleIssueDetails?.total_issues]
86-99
: Consider adding error boundaryThe conditional rendering looks good, but consider wrapping the component with an error boundary to gracefully handle potential runtime errors.
+ import { ErrorBoundary } from '@/components/error-boundary'; return ( <> {showHeader ? ( + <ErrorBoundary fallback={<div>Error loading cycle details</div>}> <Disclosure as="div" className="flex flex-shrink-0 flex-col" defaultOpen> {/* ... */} </Disclosure> + </ErrorBoundary> ) : ( + <ErrorBoundary fallback={<div>Error loading cycle details</div>}> {ActiveCyclesComponent} + </ErrorBoundary> )} </> );web/app/[workspaceSlug]/(projects)/sidebar.tsx (1)
104-107
: Consider handling empty teams list stateWhile the favorites menu has an empty state check, the teams list doesn't have similar handling. Consider adding an empty state check for consistency.
- <SidebarTeamsList /> + {canPerformWorkspaceMemberActions && teams.length > 0 && <SidebarTeamsList />}web/core/components/issues/description-input.tsx (2)
Line range hint
93-97
: Consider optimizing the debounce implementationThe current implementation recreates the debounced function on every render due to
handleSubmit
being in the dependency array. Consider moving the debounce wrapper outside the component or usinguseCallback
withuseMemo
for better performance.- const debouncedFormSave = useCallback( - debounce(async () => { - handleSubmit(handleDescriptionFormSubmit)().finally(() => setIsSubmitting("submitted")); - }, 1500), - [handleSubmit, issueId] - ); + const debouncedFormSave = useMemo( + () => + debounce(async () => { + handleSubmit(handleDescriptionFormSubmit)().finally(() => setIsSubmitting("submitted")); + }, 1500), + [handleSubmit, handleDescriptionFormSubmit, setIsSubmitting] + );
Line range hint
130-143
: LGTM: Good error handling with room for improvementThe error handling for file uploads is well implemented. Consider adding more specific error types and user-friendly error messages.
try { const { asset_id } = await fileService.uploadProjectAsset(/*...*/); return asset_id; } catch (error) { - console.log("Error in uploading issue asset:", error); - throw new Error("Asset upload failed. Please try again later."); + console.error("Error uploading issue asset:", error); + if (error instanceof TypeError) { + throw new Error("Network error while uploading. Please check your connection."); + } + throw new Error("Failed to upload file. Please try again or contact support if the issue persists."); }web/core/store/base-command-palette.store.ts (1)
Line range hint
82-96
: LGTM: Well-implemented modal state managementThe new
getCoreModalsState
method:
- Is well-documented with clear visibility modifiers
- Provides a comprehensive check of all modal states
- Enables better extensibility through protected access
Consider caching the result if this method is called frequently, as it checks multiple properties.
web/core/components/inbox/content/issue-root.tsx (1)
Based on the search results, I can now provide a final response about the state transition timing consistency:
State transition timing is inconsistent across components
The codebase shows different timeout durations for the same state transition pattern:
issue-root.tsx
: Uses 3000ms (3 seconds) delayissue-detail.tsx
andmain-content.tsx
: Use 2000ms (2 seconds) delayThese components should maintain consistent timing for better user experience predictability.
🔗 Analysis chain
Line range hint
48-58
: Verify state transition timingThe useEffect hook handles state transitions with a 3-second delay. Ensure this timing is consistent with other components using the same pattern.
Let me search for more instances of setTimeout with state transitions to get a comprehensive view.
Let me search for more specific state transition patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar setTimeout patterns in state transitions rg -A 2 'setTimeout.*setIsSubmitting.*saved'Length of output: 398
Script:
#!/bin/bash # Search for all setTimeout patterns with state transitions rg -B 2 -A 2 'setTimeout.*set\w+'Length of output: 65572
Script:
#!/bin/bash # Search for all setIsSubmitting patterns with their timing context rg -B 3 -A 3 'setIsSubmitting.*saved|setIsSubmitting.*submitting'Length of output: 10077
web/core/components/cycles/list/cycle-list-item-action.tsx (1)
203-203
: Consider documenting the progress bar behaviorThe addition of
showProgressBar: false
is consistent but could benefit from a brief comment explaining why the progress bar is disabled during navigation.- router.push(`${pathname}?${query}`, {}, { showProgressBar: false }); + // Disable progress bar for smoother navigation within cycle overview + router.push(`${pathname}?${query}`, {}, { showProgressBar: false });Also applies to: 205-205
web/core/store/issue/root.store.ts (1)
4-11
: LGTM: Well-structured team issue management integrationThe addition of team-related stores follows the established pattern and maintains consistency with existing issue management structures.
Consider adding unit tests to verify the integration of team-level issue management with the existing store hierarchy.
Also applies to: 76-77, 88-89
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (82)
packages/constants/src/issue.ts
(1 hunks)packages/types/src/common.d.ts
(1 hunks)packages/types/src/enums.ts
(1 hunks)packages/types/src/issues.d.ts
(1 hunks)packages/types/src/view-props.d.ts
(3 hunks)packages/ui/src/breadcrumbs/index.ts
(1 hunks)packages/ui/src/breadcrumbs/navigation-dropdown.tsx
(1 hunks)packages/ui/src/icons/index.ts
(2 hunks)packages/ui/src/icons/lead-icon.tsx
(1 hunks)packages/ui/src/icons/teams.tsx
(1 hunks)packages/ui/src/tabs/tabs.tsx
(4 hunks)space/core/store/helpers/base-issues.store.ts
(1 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/[cycleId]/page.tsx
(1 hunks)web/app/[workspaceSlug]/(projects)/sidebar.tsx
(4 hunks)web/ce/components/cycles/active-cycle/root.tsx
(2 hunks)web/ce/components/issues/filters/index.ts
(1 hunks)web/ce/components/issues/filters/team-project.tsx
(1 hunks)web/ce/components/issues/issue-layouts/utils.tsx
(1 hunks)web/ce/components/workspace/sidebar/teams-sidebar-list.tsx
(1 hunks)web/ce/constants/dashboard.ts
(2 hunks)web/ce/constants/issues.ts
(3 hunks)web/ce/helpers/dashboard.helper.ts
(1 hunks)web/ce/helpers/issue-action-helper.ts
(1 hunks)web/ce/store/command-palette.store.ts
(1 hunks)web/ce/store/issue/team-views/filter.store.ts
(1 hunks)web/ce/store/issue/team-views/index.ts
(1 hunks)web/ce/store/issue/team-views/issue.store.ts
(1 hunks)web/ce/store/issue/team/filter.store.ts
(1 hunks)web/ce/store/issue/team/index.ts
(1 hunks)web/ce/store/issue/team/issue.store.ts
(1 hunks)web/ce/types/dashboard.ts
(1 hunks)web/core/components/cycles/analytics-sidebar/root.tsx
(2 hunks)web/core/components/cycles/cycle-peek-overview.tsx
(3 hunks)web/core/components/cycles/form.tsx
(1 hunks)web/core/components/cycles/list/cycle-list-item-action.tsx
(3 hunks)web/core/components/cycles/list/cycle-list-project-group-header.tsx
(1 hunks)web/core/components/cycles/list/cycles-list-item.tsx
(2 hunks)web/core/components/cycles/list/index.ts
(1 hunks)web/core/components/dropdowns/layout.tsx
(1 hunks)web/core/components/dropdowns/project.tsx
(6 hunks)web/core/components/editor/rich-text-editor/rich-text-editor.tsx
(1 hunks)web/core/components/inbox/content/inbox-issue-header.tsx
(4 hunks)web/core/components/inbox/content/inbox-issue-mobile-header.tsx
(3 hunks)web/core/components/inbox/content/issue-root.tsx
(2 hunks)web/core/components/inbox/content/root.tsx
(2 hunks)web/core/components/inbox/modals/create-modal/issue-description.tsx
(3 hunks)web/core/components/issues/description-input.tsx
(5 hunks)web/core/components/issues/issue-detail/main-content.tsx
(3 hunks)web/core/components/issues/issue-layouts/utils.tsx
(3 hunks)web/core/components/issues/issue-modal/components/description-editor.tsx
(3 hunks)web/core/components/issues/issue-modal/components/project-select.tsx
(1 hunks)web/core/components/issues/issue-update-status.tsx
(2 hunks)web/core/components/issues/peek-overview/header.tsx
(4 hunks)web/core/components/modules/form.tsx
(1 hunks)web/core/components/project/card-list.tsx
(1 hunks)web/core/components/project/header.tsx
(2 hunks)web/core/components/project/index.ts
(1 hunks)web/core/components/project/multi-select-modal.tsx
(1 hunks)web/core/components/project/search-projects.tsx
(1 hunks)web/core/components/workspace/sidebar/workspace-menu.tsx
(3 hunks)web/core/constants/dashboard.ts
(0 hunks)web/core/constants/issue.ts
(5 hunks)web/core/hooks/store/use-issues.ts
(5 hunks)web/core/hooks/use-group-dragndrop.ts
(1 hunks)web/core/hooks/use-issue-layout-store.ts
(2 hunks)web/core/hooks/use-issues-actions.tsx
(3 hunks)web/core/hooks/use-local-storage.tsx
(1 hunks)web/core/store/base-command-palette.store.ts
(2 hunks)web/core/store/issue/helpers/base-issues.store.ts
(2 hunks)web/core/store/issue/helpers/issue-filter-helper.store.ts
(2 hunks)web/core/store/issue/root.store.ts
(12 hunks)web/core/store/pages/page.ts
(5 hunks)web/core/store/router.store.ts
(3 hunks)web/ee/components/issues/filters/index.ts
(1 hunks)web/ee/components/issues/filters/team-project.tsx
(1 hunks)web/ee/components/issues/issue-layouts/utils.tsx
(1 hunks)web/ee/components/workspace/sidebar/teams-sidebar-list.tsx
(1 hunks)web/ee/constants/dashboard.ts
(1 hunks)web/ee/helpers/issue-action-helper.ts
(1 hunks)web/ee/store/issue/team-views/index.ts
(1 hunks)web/ee/store/issue/team/index.ts
(1 hunks)web/helpers/issue.helper.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- web/core/constants/dashboard.ts
✅ Files skipped from review due to trivial changes (12)
- web/ce/components/issues/issue-layouts/utils.tsx
- web/ee/components/issues/filters/team-project.tsx
- web/ee/store/issue/team-views/index.ts
- web/ee/components/issues/issue-layouts/utils.tsx
- packages/ui/src/breadcrumbs/index.ts
- web/ce/store/issue/team-views/index.ts
- web/ee/store/issue/team/index.ts
- web/ce/components/workspace/sidebar/teams-sidebar-list.tsx
- web/ee/constants/dashboard.ts
- web/ee/helpers/issue-action-helper.ts
- web/ee/components/workspace/sidebar/teams-sidebar-list.tsx
- web/ce/store/issue/team/index.ts
👮 Files not reviewed due to content moderation or server errors (3)
- space/core/store/helpers/base-issues.store.ts
- web/core/components/issues/issue-layouts/utils.tsx
- web/core/store/issue/helpers/base-issues.store.ts
🧰 Additional context used
🪛 Biome (1.9.4)
web/ce/store/issue/team-views/filter.store.ts
[error] 9-11: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
web/ce/store/issue/team-views/issue.store.ts
[error] 10-12: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
web/ce/store/issue/team/issue.store.ts
[error] 10-12: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
web/ce/store/issue/team/filter.store.ts
[error] 9-11: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
🔇 Additional comments (59)
web/core/components/issues/peek-overview/header.tsx (3)
7-8
: LGTM! Import changes align with modularity goals.
The switch to TNameDescriptionLoader
type and NameDescriptionUpdateStatus
component shows good progress towards more generic, reusable components.
Also applies to: 21-21
162-162
: LGTM! Clean integration of NameDescriptionUpdateStatus.
The component is well-placed in the header's right section and properly integrated with the existing layout.
63-63
: Verify consistent usage of TNameDescriptionLoader type.
The type change improves type safety. Let's verify that this type is used consistently across related components.
✅ Verification successful
Let me gather more information to verify the usage of this type.
Based on the search results, I can now provide a conclusive response:
TNameDescriptionLoader type is used consistently across the codebase
The type TNameDescriptionLoader
is used consistently in the following contexts:
- Issue components for tracking submission states
- Page store for managing page submission states
- Components that handle name and description updates
The type change in PeekOverviewHeaderProps
aligns with the existing usage pattern across the codebase, particularly in similar components like IssueDetail
and PageStore
where it's used to track the submission state of name/description updates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other components using TNameDescriptionLoader
ast-grep --pattern 'import { $$$, TNameDescriptionLoader, $$$ } from "@plane/types"'
# Search for components with isSubmitting prop
ast-grep --pattern 'isSubmitting: $_'
Length of output: 249
Script:
#!/bin/bash
# Search for TNameDescriptionLoader usage in type definitions and interfaces
ast-grep --pattern 'TNameDescriptionLoader'
# Search for files containing isSubmitting with more context
rg "isSubmitting" -A 2 -B 2
Length of output: 66654
web/ce/constants/dashboard.ts (1)
15-23
: Type definition for TSidebarMenuItems
enhances flexibility
Defining TSidebarMenuItems
as a generic type improves the reusability and maintains type safety for both user and workspace menu items.
web/core/components/project/multi-select-modal.tsx (1)
37-40
: Verify dependencies in useMemo
Including getProjectById
in the dependency array may cause unnecessary recomputations if getProjectById
is unstable. Ensure that getProjectById
is memoized or stable to prevent performance issues.
Run the following script to check the stability of getProjectById
:
web/ce/components/issues/filters/index.ts (1)
3-3
: Verify that ./team-project
module exists and is properly exported
The export statement export * from "./team-project";
has been added. Please ensure that the team-project
module exists at the specified path and that it correctly exports the intended components or functions to prevent runtime errors.
Run the following script to check for the existence of the team-project.tsx
file:
✅ Verification successful
Let me verify the exports in the team-project.tsx file to ensure it has proper exports.
Module exists and exports are properly defined
The team-project.tsx
file exists at the correct location and properly exports the FilterTeamProjects
component. The export statement in index.ts
correctly re-exports this component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the team-project.tsx file exists in the CE filters directory
fd 'team-project.tsx' web/ce/components/issues/filters
Length of output: 104
Script:
#!/bin/bash
# Check the content of team-project.tsx for exports
cat web/ce/components/issues/filters/team-project.tsx
Length of output: 326
web/ee/components/issues/filters/index.ts (1)
3-3
: Verify that ./team-project
module exists and is properly exported
The export statement export * from "./team-project";
has been added. Please ensure that the team-project
module exists at the specified path and that it correctly exports the intended components or functions to prevent runtime errors.
Run the following script to check for the existence of the team-project.tsx
file:
#!/bin/bash
# Description: Verify that the team-project.tsx file exists in the EE filters directory
fd 'team-project.tsx' web/ee/components/issues/filters
web/ce/types/dashboard.ts (1)
3-3
: New type TSidebarWorkspaceMenuItemKeys
added successfully
The addition of TSidebarWorkspaceMenuItemKeys
enhances the sidebar menu structure by introducing workspace-specific menu items. This improves modularity and organization within the application.
web/core/components/cycles/list/index.ts (1)
6-6
: LGTM! Verify the exported component exists.
The export statement follows the established pattern and naming convention.
✅ Verification successful
Export statement is valid and component file exists
The exported component cycle-list-project-group-header
exists at the correct location in the codebase at web/core/components/cycles/list/cycle-list-project-group-header.tsx
.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the exported component file exists
fd "cycle-list-project-group-header.tsx" --type f
Length of output: 117
web/ce/helpers/dashboard.helper.ts (1)
7-8
: Verify the always-true implementation is intentional.
Both feature flag helpers always return true. If this is the CE (Community Edition) version, this might be intentional, but it should be verified.
✅ Verification successful
Always-true implementation is intentional for CE edition
This is the Community Edition (CE) implementation where all workspace features (projects
, all-issues
, active-cycles
, analytics
) are enabled by default. The implementation is located in the web/ce/
directory and the helper is specifically designed for the CE version, making the always-true return value intentional.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for different implementations in other editions
rg "isWorkspaceFeatureEnabled.*=.*{" --type ts
Length of output: 174
Script:
#!/bin/bash
# Search for isWorkspaceFeatureEnabled implementations
rg "isWorkspaceFeatureEnabled.*=" --type ts -A 2
Length of output: 202
Script:
#!/bin/bash
# Look for any feature flag related files
fd -e ts feature
Length of output: 18
Script:
#!/bin/bash
# Check if this is indeed CE edition by looking at package.json
cat package.json | jq '.name'
Length of output: 37
Script:
#!/bin/bash
# Look for any workspace feature related constants or types
rg "TSidebarWorkspaceMenuItemKeys" --type ts -A 3
Length of output: 1607
web/ce/store/issue/team/filter.store.ts (1)
4-5
:
Remove @ts-nocheck and clarify intent.
The comment stating "This class will never be used" raises concerns about the purpose of this code. If this is temporary scaffolding, it should be documented with a TODO and timeline for proper implementation.
web/ce/store/command-palette.store.ts (1)
5-8
: LGTM! Clean implementation of MobX computed property
The extension of the base store and implementation of the computed property follows MobX best practices. The observable setup in the constructor is correct.
Also applies to: 13-16
web/core/components/issues/issue-update-status.tsx (2)
4-8
: LGTM! Improved component naming and type safety
The rename from IssueUpdateStatus
to NameDescriptionUpdateStatus
better reflects the component's purpose. The use of TNameDescriptionLoader
type enhances type safety.
Also applies to: 11-11
22-22
: Clean implementation of spinner animation
The use of animate-spin
and size-3.5
utility classes provides a clean, maintainable approach to styling the loading indicator.
packages/constants/src/issue.ts (1)
14-14
:
Verify the duplicate mapping to project_id
The new team_project
option maps to "project_id"
, which is the same as the existing project
option. This could lead to confusion or inconsistent behavior.
Additionally, consider adding corresponding entries in EIssueGroupBYServerToProperty
and EServerGroupByToFilterOptions
for consistency.
web/ce/constants/issues.ts (1)
38-40
: Verify the empty ADDITIONAL_ISSUE_DISPLAY_FILTERS_BY_LAYOUT initialization
The constant is initialized as an empty object. Please verify if this is intentional or if default configurations should be added.
✅ Verification successful
Let me gather more context about the usage and implementation of display filters.
Based on the search results, I can now provide a final response:
Empty ADDITIONAL_ISSUE_DISPLAY_FILTERS_BY_LAYOUT initialization is intentional
The empty object initialization is correct as this constant is used as an extension point in the Community Edition (CE) to be spread into the main ISSUE_DISPLAY_FILTERS_BY_LAYOUT
configuration in web/core/constants/issue.ts
. The core application already has default configurations, and this empty object allows for additional configurations to be merged without overriding the existing ones.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any usage of this constant to ensure it's populated elsewhere
rg "ADDITIONAL_ISSUE_DISPLAY_FILTERS_BY_LAYOUT" -A 3
Length of output: 746
Script:
#!/bin/bash
# Search for ILayoutDisplayFiltersOptions interface definition and usage
rg "ILayoutDisplayFiltersOptions" -A 5
# Search for any other display filter configurations
rg "DISPLAY_FILTERS" -A 3
Length of output: 47614
packages/types/src/enums.ts (1)
62-62
: LGTM! Verify usage of new enum value.
The new TEAM_SPACE_DESCRIPTION
enum value follows the established naming pattern and is properly typed.
Let's verify its usage across the codebase:
packages/ui/src/icons/index.ts (1)
19-19
:
New icon exports look good, but remove duplicate info-icon export.
The new icon exports follow the established pattern, but there's a duplicate export of info-icon
in the file (see line 33).
Remove the duplicate export:
export * from "./info-icon";
export * from "./dropdown-icon";
-export * from "./info-icon";
Let's verify the new icon components exist:
Also applies to: 42-43
✅ Verification successful
All new icon components exist and are properly exported
The verification confirms that all three new icon components (info-fill-icon.tsx
, teams.tsx
, and lead-icon.tsx
) exist in the correct location at packages/ui/src/icons/
. The exports follow the established pattern and the files are present in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the new icon components exist
fd -e tsx -e ts "info-fill-icon|teams|lead-icon" packages/ui/src/icons/
Length of output: 182
web/core/components/cycles/analytics-sidebar/root.tsx (2)
15-17
: LGTM! Props type enhancement improves component modularity.
Making these props mandatory improves type safety and makes the component more reusable by explicitly defining its dependencies.
21-21
: LGTM! Clean props destructuring and usage.
The direct usage of props instead of useParams makes the component more modular and easier to test.
Also applies to: 25-27
web/core/hooks/use-local-storage.tsx (1)
Line range hint 47-52
: LGTM! Proper event listener cleanup.
The event listener setup and cleanup is handled correctly using the useEffect cleanup function.
web/core/components/issues/issue-modal/components/project-select.tsx (1)
49-49
: LGTM! Explicit multiple selection behavior.
The addition of multiple={false}
prop makes the component's behavior more explicit and aligns with the PR's modularity goals.
web/core/components/editor/rich-text-editor/rich-text-editor.tsx (1)
19-20
: Props interface changes improve component modularity
Making projectId
optional and adding memberIds
as a direct prop improves the component's reusability and decouples it from project-specific logic.
Let's verify all consumers handle the optional projectId:
✅ Verification successful
Props interface changes are safe and properly handled by consumers
Based on the codebase analysis, all consumers of the RichTextEditor component are correctly handling both the optional projectId
and required memberIds
props:
issue-description.tsx
: Passes bothprojectId
andmemberIds
description-editor.tsx
: Passes bothprojectId
andmemberIds
description-input.tsx
: Passes bothprojectId
andmemberIds
The changes improve component modularity while maintaining backward compatibility, and all existing implementations have been properly updated to handle the new props structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for RichTextEditor usage to ensure consumers handle optional projectId
ast-grep --pattern 'RichTextEditor $props' | rg -A 5 'projectId'
Length of output: 64
Script:
#!/bin/bash
# Search for RichTextEditor usage with broader pattern and context
rg -l "RichTextEditor" | xargs rg -A 10 "RichTextEditor"
Length of output: 17604
Script:
#!/bin/bash
# Search for RichTextEditor imports to find consumers
rg "import.*RichTextEditor"
Length of output: 867
Script:
#!/bin/bash
# Search for RichTextEditor component definition to understand default props
ast-grep --pattern 'export $_ function RichTextEditor($props) {
$$$
}'
Length of output: 74
web/core/components/cycles/cycle-peek-overview.tsx (1)
38-41
: LGTM: Effect dependencies properly updated
The useEffect dependencies array correctly includes all used variables, and the early return condition prevents unnecessary API calls.
web/core/components/dropdowns/layout.tsx (1)
25-30
: LGTM: Proper dependency management
The options array correctly depends on availableLayouts, and the mapping operation is straightforward.
web/core/store/router.store.ts (1)
12-12
: LGTM! Clean implementation of teamId support
The addition of the teamId property follows the established patterns in the router store. The implementation is consistent with other query parameters and includes proper documentation.
Also applies to: 38-38, 71-78
web/core/hooks/use-group-dragndrop.ts (1)
20-22
: Verify handling of new store types
The addition of TEAM
and TEAM_VIEW
store types expands the drag-and-drop functionality to team-related views. However, we should ensure that these new types are properly handled in the updateIssueOnDrop
function.
✅ Verification successful
Let me gather more information about the drag and drop handling logic.
Let me check how the drag and drop operations are handled for different store types.
Based on the gathered information, I can now provide the final response:
The new store types are properly handled in the drag-and-drop implementation
The drag-and-drop functionality is store-type agnostic and works through a common interface. The implementation in useGroupIssuesDragNDrop
hook:
- Uses
useIssuesActions
which has specific handlers for TEAM and TEAM_VIEW store types - Leverages
useIssues
store which has proper type definitions and handlers for team-related stores - Doesn't require special handling for TEAM and TEAM_VIEW as the drag-and-drop operations are abstracted through common interfaces
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any special handling of TEAM and TEAM_VIEW store types
rg -A 5 "EIssuesStoreType.TEAM|EIssuesStoreType.TEAM_VIEW" --type ts
# Check for any team-specific issue update logic
ast-grep --pattern 'function updateIssueOnDrop($_) {
$$$
if ($store === EIssuesStoreType.TEAM) {
$$$
}
$$$
}'
Length of output: 4238
Script:
#!/bin/bash
# Check the implementation of updateIssueOnDrop function
ast-grep --pattern 'function updateIssueOnDrop($_) {
$$$
}'
# Search for any drag and drop related handlers for team stores
rg -A 10 "updateIssueOnDrop" --type ts
Length of output: 3399
Script:
#!/bin/bash
# Check the drag and drop handler implementation
rg -A 20 "useGroupIssuesDragNDrop" --type ts
# Check if there are any store-specific conditions in the drag and drop logic
rg -A 10 "storeType.*TEAM" --type ts
Length of output: 10557
web/ce/components/cycles/active-cycle/root.tsx (1)
26-27
: LGTM: Props interface enhancement
The addition of optional props cycleId
and showHeader
improves component flexibility while maintaining backward compatibility.
web/core/components/inbox/content/root.tsx (1)
4-4
: Verify loader state handling
The change from union type to TNameDescriptionLoader
suggests standardization of loader states. Ensure that all possible states from TNameDescriptionLoader
are properly handled in the UI components that consume this state.
Also applies to: 36-36
✅ Verification successful
Based on the shell script results, I can now generate the final response:
Loader state handling is properly implemented
The TNameDescriptionLoader
type is consistently used across components and has a well-defined set of states: "submitting" | "submitted" | "saved". The codebase shows proper implementation:
- All components consuming
isSubmitting
prop use the correct type - The state is properly managed through useState and state management stores
- Components like
NameDescriptionUpdateStatus
handle the loading states appropriately
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of TNameDescriptionLoader type
rg -A 5 "TNameDescriptionLoader" --type ts
# Check for components consuming isSubmitting prop
ast-grep --pattern 'interface $_ {
$$$
isSubmitting: TNameDescriptionLoader;
$$$
}'
Length of output: 10851
web/app/[workspaceSlug]/(projects)/sidebar.tsx (2)
24-24
: LGTM: New teams list component import
The addition of the SidebarTeamsList component aligns with the PR's objective of improving component modularity.
80-81
: Verify permission check implementation
The SidebarAppSwitcher is now conditionally rendered based on workspace member permissions, which is a security improvement. However, we should verify the permission check implementation.
✅ Verification successful
Let me verify the implementation of allowPermissions
to ensure it's correctly checking workspace member permissions.
Let me search for the actual implementation of allowPermissions
function since it wasn't found in the previous search.
Let me try one more search pattern to find the allowPermissions implementation.
Let me try one final search pattern to locate the implementation.
Based on the search results, I can now provide a final response since I have found the complete implementation of the permission checks:
Permission check implementation is correctly implemented
The canPerformWorkspaceMemberActions
check is properly implemented using the allowPermissions
utility which:
- Takes an array of allowed permissions ([ADMIN, MEMBER])
- Checks against the WORKSPACE permission level
- Is consistently used across the codebase for workspace-level member actions
- Is implemented in the permissions store with proper workspace and project context handling
The SidebarAppSwitcher's conditional rendering is following the same pattern used throughout the application for workspace-level member access control.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of canPerformWorkspaceMemberActions to ensure consistent permission checks
rg -A 2 "canPerformWorkspaceMemberActions"
Length of output: 4613
Script:
#!/bin/bash
# Search for allowPermissions implementation
ast-grep --pattern 'const allowPermissions = $_'
# Also search for EUserPermissions enum definition
rg "enum EUserPermissions" -A 10
Length of output: 1727
Script:
#!/bin/bash
# Search for allowPermissions function implementation
rg -A 5 "function allowPermissions"
# Also try searching for allowPermissions export
rg -A 5 "export.*allowPermissions"
Length of output: 74
Script:
#!/bin/bash
# Search for allowPermissions with more context and different patterns
rg -A 10 "allowPermissions" web/core/hooks/
rg -A 10 "allowPermissions" web/core/utils/
Length of output: 143
Script:
#!/bin/bash
# Search for allowPermissions in all files, including potential utility files
fd . -t f -e ts -e tsx -e js -e jsx --exec rg -l "allowPermissions"
# Get contents of any file that might contain the implementation
rg -A 10 "allowPermissions.*="
Length of output: 72405
web/core/components/cycles/list/cycles-list-item.tsx (1)
75-75
: Verify impact of disabling progress bar
The addition of showProgressBar: false
might affect user experience during navigation. Please ensure this is a deliberate UX decision and verify its impact.
Also applies to: 77-77
✅ Verification successful
Let me search for more router.push calls to get a complete picture of the progress bar behavior across the codebase.
Based on the search results, I can now provide a conclusive response about the progress bar behavior:
Progress bar is intentionally disabled for specific navigation patterns
The showProgressBar: false
option is consistently used in specific navigation scenarios:
- Cycle-related navigation (peek views and list items)
- Project settings navigation
- Module-related navigation
This appears to be a deliberate UX pattern where certain quick navigation actions within the same context don't need to show a loading indicator. The default behavior (without the option) shows the progress bar, which is appropriate for major navigation changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other router.push calls to check for consistency in progress bar behavior
rg -A 2 "router\.push.*showProgressBar"
Length of output: 1536
Script:
#!/bin/bash
# Search for all router.push calls to check if there's a mix of progress bar behaviors
rg "router\.push\(" -A 2
Length of output: 29920
packages/types/src/issues.d.ts (1)
214-215
: Verify consistent usage of new team_project grouping
The addition of "team_project" to GroupByColumnTypes expands issue grouping capabilities. Let's verify its implementation across the codebase.
✅ Verification successful
The team_project grouping is properly integrated across the codebase
The verification shows that the "team_project" grouping option is consistently implemented:
- It's properly defined in the type system (
GroupByColumnTypes
) - It's integrated into the grouping functionality in multiple components
- It has corresponding column getters and handlers in the codebase
- It's properly mapped to "project_id" in the backend integration
- It's included in the filter helpers and store implementations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for GroupByColumnTypes usage and related team_project implementations
ast-grep --pattern 'type $_ = $$$| "team_project" |$$$'
# Search for any direct usage of "team_project" string
rg "team_project" --type typescript
Length of output: 841
Script:
#!/bin/bash
# Search for team_project usage with ripgrep
rg "team_project" -t ts -t tsx -A 2 -B 2
# Search for GroupByColumnTypes usage
rg "GroupByColumnTypes" -t ts -t tsx -A 2 -B 2
# Search for any group by related implementations
rg "groupBy|group_by" -t ts -t tsx
Length of output: 208
Script:
#!/bin/bash
# Search for team_project and related terms without file type restrictions
rg "team_project" -A 2 -B 2
# Search for GroupByColumnTypes usage
rg "GroupByColumnTypes" -A 2 -B 2
# Look for files that might contain group by implementations
fd -e ts -e tsx -e js -e jsx --exec grep -l "groupBy\|group_by" {} \;
Length of output: 16979
packages/types/src/view-props.d.ts (1)
21-21
: LGTM: Consistent addition of team project support
The changes consistently integrate team project support across the type definitions, maintaining type safety and following existing patterns.
Also applies to: 73-73, 97-97
web/core/components/issues/issue-detail/main-content.tsx (1)
5-5
: LGTM: Clean transition to NameDescriptionUpdateStatus
The changes consistently implement the new TNameDescriptionLoader type and NameDescriptionUpdateStatus component, improving modularity while maintaining type safety.
Also applies to: 9-9, 42-42, 91-91
web/core/hooks/store/use-issues.ts (3)
8-9
: LGTM: Clean import structure for new team-related types
The new imports for team-related interfaces are well-organized and follow the existing pattern.
38-41
: LGTM: Well-structured type extensions
The TStoreIssues type is cleanly extended with team-related cases, maintaining consistency with existing patterns.
Also applies to: 54-57
100-104
: Verify team-related store access patterns
The implementation for team and team view cases follows the established pattern. However, we should verify that the context properties (teamIssues
, teamViewIssues
) are properly initialized.
Also applies to: 120-124
✅ Verification successful
Based on the search results, I can now verify the team-related store properties. The code shows that:
teamIssues
andteamIssuesFilter
are properly initialized in the root store (web/core/store/issue/root.store.ts)- These properties are correctly typed and integrated into the issues store type system (web/core/hooks/store/use-issues.ts)
- The
EIssuesStoreType.TEAM
case is properly handled in the store access patterns - The properties are correctly merged with the default store in the use-issues hook
Team-related store properties are properly initialized and integrated
The implementation follows the established pattern consistently across the codebase. The team issues store properties are properly initialized in the root store and correctly accessed through the store context.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if team-related store properties are properly initialized
ast-grep --pattern 'class $_ {
$$$
teamIssues: $_
$$$
}'
ast-grep --pattern 'class $_ {
$$$
teamViewIssues: $_
$$$
}'
Length of output: 133
Script:
#!/bin/bash
# Let's try a different approach to find team-related store properties
# First, let's check the context/issue file structure
fd "issue" --type f
# Then search for team-related properties initialization
rg "teamIssues" -A 3
rg "teamViewIssues" -A 3
# Also search for the store type enum to understand the structure
rg "EIssuesStoreType" -A 5
Length of output: 66252
web/core/components/cycles/form.tsx (1)
Line range hint 78-82
: LGTM: Improved ProjectDropdown configuration
Good improvements:
- Explicitly setting
multiple={false}
clarifies the single-project selection behavior - Proper tab index management maintains accessibility
web/core/store/base-command-palette.store.ts (1)
1-1
: Optimize imports
The computed
import has been removed as it's no longer needed after the architectural change.
web/core/components/inbox/content/inbox-issue-mobile-header.tsx (2)
18-18
: LGTM: Type system enhancement improves code maintainability
The migration to TNameDescriptionLoader
type and the new NameDescriptionUpdateStatus
component aligns with the PR's objective of improving component modularity.
Also applies to: 22-22, 34-34
121-121
: Verify component behavior consistency
The replacement of IssueUpdateStatus
with NameDescriptionUpdateStatus
should maintain the same user experience.
✅ Verification successful
Component replacement maintains consistent behavior
The NameDescriptionUpdateStatus
component is consistently used across the application for showing save status with identical behavior. It displays a loading spinner and "Saving..." text while submitting, and "Saved" when complete. The component is properly typed with TNameDescriptionLoader
and follows the same pattern in all its implementations (inbox, issue detail, and peek overview). No references to the old IssueUpdateStatus
component remain in the codebase, indicating a complete migration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of NameDescriptionUpdateStatus to verify consistent implementation
ast-grep --pattern 'NameDescriptionUpdateStatus isSubmitting={$_}'
Length of output: 68
Script:
#!/bin/bash
# Search for both components to understand the changes
echo "Searching for IssueUpdateStatus component:"
rg "IssueUpdateStatus" -A 3
echo -e "\nSearching for NameDescriptionUpdateStatus component:"
rg "NameDescriptionUpdateStatus" -A 3
echo -e "\nSearching for component definitions:"
ast-grep --pattern 'const $_ = ({ isSubmitting }: { isSubmitting: boolean }) => {'
Length of output: 4071
Script:
#!/bin/bash
# Let's look at the component definition and its usage
echo "Checking the component implementation:"
cat web/core/components/issues/issue-update-status.tsx
echo -e "\nChecking if there are any references to the old IssueUpdateStatus component:"
fd -e tsx -e ts -e jsx -e js | xargs rg "IssueUpdateStatus"
Length of output: 1218
web/core/components/modules/form.tsx (1)
89-89
: LGTM: Explicit single project selection
Setting multiple={false}
explicitly enforces that a module can only belong to one project, which is the correct business logic.
web/core/components/inbox/content/issue-root.tsx (1)
7-7
: LGTM: Consistent type system enhancement
The adoption of TNameDescriptionLoader
type improves type safety and maintains consistency across components.
Also applies to: 37-38
web/core/components/workspace/sidebar/workspace-menu.tsx (2)
25-29
: LGTM: Import restructuring and feature flag integration
The reorganization of imports and addition of feature flag functionality improves the code's modularity and feature management capabilities.
Line range hint 164-196
: Verify feature flag implementation for all menu items
The feature flag check has been added correctly, but let's ensure all menu items are properly configured with feature flags.
✅ Verification successful
Let me gather more information about the menu items and their feature flag configuration.
Let me search for the actual menu items array definition to verify each item's configuration.
Let me check the SIDEBAR_WORKSPACE_MENU definition to see the complete menu item configurations.
Let me check one final thing to see how the feature flags are implemented in the CE (Community Edition) version.
Feature flag implementation is correctly configured but always enabled in CE
The feature flag implementation is properly structured across all menu items ("projects", "all-issues", "active-cycles", "analytics"), but in the Community Edition (CE) the isWorkspaceFeatureEnabled
helper always returns true
. This is an intentional design choice for the CE version, where all features are enabled by default.
- The feature flag check is applied uniformly to all menu items through the
link.key
parameter - The CE implementation in
web/ce/helpers/dashboard.helper.ts
explicitly returnstrue
for all features - All menu item keys are properly typed via
TSidebarWorkspaceMenuItemKeys
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify feature flag configuration for menu items
# Check for any menu items that might be missing feature flag definitions
# Search for menu item definitions
ast-grep --pattern 'SIDEBAR_WORKSPACE_MENU_ITEMS = [
$$$
]'
# Search for feature flag definitions
rg "isWorkspaceFeatureEnabled.*\(" -A 2
Length of output: 659
Script:
#!/bin/bash
# Search for the menu items definition and their keys
ast-grep --pattern 'export const SIDEBAR_WORKSPACE_MENU_ITEMS: TSidebarWorkspaceMenuItem[] = [
$$$
]'
# Search for the type definition of TSidebarWorkspaceMenuItemKeys
ast-grep --pattern 'type TSidebarWorkspaceMenuItemKeys = $$$'
Length of output: 300
Script:
#!/bin/bash
# Search for the menu items array content
rg "SIDEBAR_WORKSPACE_MENU_ITEMS.*=.*\[" -A 20
Length of output: 540
Script:
#!/bin/bash
# Search for SIDEBAR_WORKSPACE_MENU definition
rg "SIDEBAR_WORKSPACE_MENU\s*=\s*{" -A 30
# Also search for any other potential menu item definitions
rg "projects:|all-issues:|active-cycles:|analytics:" -A 5 web/ce/
Length of output: 1103
Script:
#!/bin/bash
# Search for the complete SIDEBAR_WORKSPACE_MENU definition
rg "export const SIDEBAR_WORKSPACE_MENU = {" -A 50
# Also check for any feature-specific configurations
rg "isWorkspaceFeatureEnabled" --type ts
Length of output: 678
web/core/components/issues/issue-modal/components/description-editor.tsx (2)
79-83
: LGTM: Member integration enhances editor functionality
The addition of member information through the useMember
hook and memberIds
improves the editor's capabilities, particularly for features like user mentions.
187-187
: Verify member IDs usage in RichTextEditor
Ensure the RichTextEditor component properly handles the memberIds prop for features like mentions.
✅ Verification successful
memberIds prop is properly handled in RichTextEditor
The RichTextEditor component correctly handles the memberIds prop. It's used to:
- Map member IDs to user details via getUserDetails
- Pass the member details to useMention hook for mention functionality
- Support mentions through the mentionHandler with highlights and suggestions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify RichTextEditor's handling of memberIds
# Check RichTextEditor implementation
ast-grep --pattern 'RichTextEditor = $$$({
$$$
memberIds,
$$$
}) => {
$$$
}'
Length of output: 86
Script:
#!/bin/bash
# Let's try a different pattern to find RichTextEditor component and its usage of memberIds
ast-grep --pattern 'const RichTextEditor = $_'
# Also search for any file containing RichTextEditor and memberIds
rg "RichTextEditor.*memberIds" -A 5
Length of output: 9058
web/core/components/cycles/list/cycle-list-item-action.tsx (1)
71-73
: LGTM: Enhanced permission check granularity
The permission check now correctly includes workspace and project context, improving security and modularity.
web/core/store/issue/root.store.ts (1)
139-140
: LGTM: Proper store initialization and state sync
The initialization of team stores and router state synchronization is implemented correctly, maintaining consistency with the store's reactive patterns.
Also applies to: 151-152, 192-192
web/core/store/issue/helpers/issue-filter-helper.store.ts (1)
96-96
: LGTM: Clean integration of team project filtering
The addition of team project filtering maintains consistency with existing filter patterns and includes proper null handling.
Let's verify the team project filter usage:
Also applies to: 146-146
✅ Verification successful
LGTM: Team project filter implementation is consistent
The verification confirms that the team project filter is consistently implemented in the issue-filter-helper.store.ts file, with proper null/undefined handling in both the filter mapping (line 96) and filter state (line 146). No inconsistencies or issues were found in the implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent team project filter usage across components
# Search for team_project filter usage
rg "team_project.*filter" -A 3
# Check for any potential filter mapping inconsistencies
ast-grep --pattern 'filters?.team_project'
Length of output: 1102
web/core/constants/issue.ts (3)
27-27
: LGTM: New enum values enhance team functionality.
The addition of TEAM
and TEAM_VIEW
to EIssuesStoreType
aligns with the PR's objective of improving team-related functionality.
Also applies to: 31-31
472-472
: LGTM: Extensible filter configuration.
The spread operator usage for incorporating additional display filters follows good practices for maintaining extensible configurations.
86-86
: Verify team project grouping implementation.
The addition of the team project grouping option looks good, but let's verify its implementation across the codebase.
✅ Verification successful
Based on the search results, I can now verify the team project grouping implementation. The codebase shows comprehensive integration of the team project grouping feature:
Team project grouping is properly integrated across the codebase
- It's defined as a valid grouping option in the issue constants (
web/core/constants/issue.ts
) - It's integrated into the filter and store logic (
web/core/store/issue/helpers/issue-filter-helper.store.ts
) - It's properly mapped in the server options (
packages/constants/src/issue.ts
) - It's handled in the base issues store types (
space/core/store/helpers/base-issues.store.ts
) - The grouping functionality follows the same pattern as other grouping options in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of team project grouping.
# Test: Search for team_project usage in grouping logic.
# Check for team_project references in TypeScript/JavaScript files
rg -t ts -t js "team_project.*group" -A 5
Length of output: 43
Script:
#!/bin/bash
# Let's try a broader search for team_project usage
rg -t ts -t js "team_project" -A 5
# Also search for grouping related implementations
rg -t ts -t js "groupBy|GroupBy" -A 5
Length of output: 85933
web/core/components/inbox/content/inbox-issue-header.tsx (3)
18-18
: LGTM: Type imports and component updates.
The changes improve type safety by using TNameDescriptionLoader
and enhance semantic clarity by renaming to NameDescriptionUpdateStatus
.
Also applies to: 29-29
45-45
: LGTM: Type definition update.
The isSubmitting
property type update in TInboxIssueActionsHeader
maintains consistency with the new type system.
286-286
: LGTM: Component usage update.
The component usage is correctly updated to use NameDescriptionUpdateStatus
.
web/core/store/pages/page.ts (2)
4-4
: LGTM: Interface updates enhance type safety.
The changes to the IPage
interface properly integrate the new TNameDescriptionLoader
type, improving type safety while maintaining functionality.
Also applies to: 15-15, 29-29
48-48
: LGTM: Class implementation aligns with interface.
The implementation in the Page
class correctly reflects the interface changes, maintaining consistency in type usage.
Also applies to: 325-325
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
TNameDescriptionLoader
,ITeamIssuesFilter
, andITeamViewIssuesFilter
.BreadcrumbNavigationDropdown
,FilterTeamProjects
,ProjectMultiSelectModal
,ProjectSearch
, andCycleListProjectGroupHeader
.SidebarTeamsList
.Bug Fixes
Documentation
Chores