[WEB-3040] chore: project breadcrumb improvement #6322
Conversation
WalkthroughThe pull request introduces a comprehensive refactoring of breadcrumb navigation across multiple project-related components. The primary change involves replacing individual project breadcrumb implementations with a new, centralized Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
![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 (7)
web/ce/components/breadcrumbs/project.tsx (1)
23-29: Remove redundant check in JSX.
You havecurrentProjectDetails ? (currentProjectDetails && (...)) : (...), which makes thecurrentProjectDetailscheck redundant. Consider simplifying the conditional logic for a cleaner approach.- currentProjectDetails ? ( - currentProjectDetails && ( - <span className="grid place-items-center flex-shrink-0 h-4 w-4"> - <Logo logo={currentProjectDetails?.logo_props} size={16} /> - </span> - ) - ) : ( + currentProjectDetails ? ( + <span className="grid place-items-center flex-shrink-0 h-4 w-4"> + <Logo logo={currentProjectDetails?.logo_props} size={16} /> + </span> + ) : (web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(list)/header.tsx (1)
25-25: Project breadcrumb usage.
AdoptingProjectBreadcrumbhelps maintain consistency across various headers. Keep an eye on potential future expansions (e.g., environment badges or project states) to ensureProjectBreadcrumbremains flexible.web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/header.tsx (1)
61-61: Add Workspace-Specific Tests
After adding<ProjectBreadcrumb />, consider adding or updating unit/integration tests for workspace-specific behavior (Pro vs. non-Pro). This helps ensure correctness for all workspace tiers.web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/draft-issues/header.tsx (1)
22-23: Remove or rename the inline comment for consistency.
The inline comment// plane webcan be safely removed or updated to match the code style guidelines across the project.-// plane web +// Plane webweb/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(detail)/[viewId]/header.tsx (1)
36-37: Remove or rename the inline comment for consistency.
Similar to the previous file, the// plane webcomment can be removed or standardized.-// plane web +// Plane webweb/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(detail)/header.tsx (1)
41-42: Remove or rename the inline comment for consistency.
Like the other files, consider removing or updating the// plane webcomment to match code style.-// plane web +// Plane webweb/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/header.tsx (1)
40-41: Remove or rename the inline comment for consistency.
The repeated// plane webannotation can be standardized or removed altogether.-// plane web +// Plane web
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/archives/header.tsx(3 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/archives/issues/(detail)/header.tsx(2 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/header.tsx(3 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(list)/header.tsx(2 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/draft-issues/header.tsx(3 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/issues/(detail)/header.tsx(2 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(detail)/header.tsx(3 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(list)/header.tsx(2 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/header.tsx(2 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/header.tsx(2 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/settings/header.tsx(2 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(detail)/[viewId]/header.tsx(2 hunks)web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(list)/header.tsx(1 hunks)web/ce/components/breadcrumbs/index.ts(1 hunks)web/ce/components/breadcrumbs/project.tsx(1 hunks)web/ce/components/issues/header.tsx(3 hunks)web/ce/components/projects/settings/intake/header.tsx(2 hunks)web/ee/components/breadcrumbs/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- web/ce/components/breadcrumbs/index.ts
- web/ee/components/breadcrumbs/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint-web
🔇 Additional comments (42)
web/ce/components/breadcrumbs/project.tsx (1)
21-21: Confirm fallback name for project.
Using"Project"as a default name is fine, but verify whether you need a more specific placeholder to better differentiate multiple projects or check if your product design calls for a different fallback text.web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(list)/header.tsx (2)
8-13: Imports realigned successfully.
These changes remove the unnecessary route parameters while adding the newProjectBreadcrumb. This looks good and consistent with the updated approach to handle project breadcrumb logic in a central component.
18-18: Verify loading states.
You are usingloaderfrom theuseProjecthook to indicate a loading state inBreadcrumbs. Confirm this aligns with the rest of the codebase’s loading patterns (e.g., other components might have separate loading indicators).web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(list)/header.tsx (2)
13-13: Good use of centralizedProjectBreadcrumb.
Replacing direct references tocurrentProjectDetailswith the newProjectBreadcrumbhelps ensure modularity and consistent breadcrumb styling across the app.Also applies to: 38-38
25-25: Check fallback behavior on loader.
You passloadertoBreadcrumbs. Make sure that whenloaderistrue, the fallback or skeleton for the breadcrumb is displayed correctly, matching user expectations.web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(list)/header.tsx (1)
14-14: Consistent breadcrumb refactoring.
The introduction ofProjectBreadcrumbin the cycles header aligns with the rest of the project’s standardized breadcrumb approach. No further issues found here.Also applies to: 36-36
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/issues/(detail)/header.tsx (3)
13-13: Use Named Imports for Clarity
SinceProjectBreadcrumbis a central element of your new breadcrumb structure, consider keeping it cleanly separated to highlight its importance. This is already well done, just a recommendation to maintain it in future refactors.
33-33: Conditionally Render for Non-Pro Workspaces
Given the PR objective specifies different behavior for Pro vs. non-Pro workspaces, confirm whetherProjectBreadcrumbhandles this internally or if an additional conditional check is needed to disable the breadcrumb link for non-Pro workspaces.
8-8: Remove Unused Import Check
If you're no longer using any other elements from@/components/commonaside fromBreadcrumbLink, ensure that the import statement is kept minimal. Otherwise, if other exports from@/components/commonare still in use elsewhere, this is fine.web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/settings/header.tsx (4)
14-14: Full Adoption ofProjectBreadcrumb
Good call importingProjectBreadcrumbto standardize the breadcrumb behavior. This fosters consistency across different headers.
25-25: Loader State Usage
You're now using onlyloaderfromuseProject()instead of also destructuring project details. This likely reduces overhead. Just a note that it might affect any future code that relies oncurrentProjectDetails.
33-33: Ensure Non-Pro Workspaces Logic
Double-check ifProjectBreadcrumbincorporates the necessary logic to disable navigation or redirect for non-Pro workspaces. If not included internally, wrap<ProjectBreadcrumb />with a conditional guard as needed.
10-10: Bookmark Dependency
BreadcrumbLinkappears repeatedly across the codebase. IfProjectBreadcrumbfully replaces these references, consider removingBreadcrumbLinkif it's no longer needed.web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/header.tsx (2)
17-17: Consistent Import Style
Your approach to addingProjectBreadcrumbis consistent with other places in the PR. This centralization will make future enhancements easier.
12-12: Library Sweeps
Good to see the import from@/components/commonreduced. If you plan to removeLogoentirely from your codebase, make sure you’re not missing it in any leftover references across other files.web/ce/components/projects/settings/intake/header.tsx (3)
10-10: Minimal Import
BreadcrumbLinkis imported but used minimally, ensuring there's no leftover references from older breadcrumb logic. If you plan on removing or refactoringBreadcrumbLinkglobally, track usage carefully.
14-14: Smooth Transition toProjectBreadcrumb
It’s good to see the newProjectBreadcrumbapproach consistently introduced. This reduces custom breadcrumb code duplication across components.
40-40: Hybrid Breadcrumb Logic
IfProjectBreadcrumbis meant to handle both Pro and non-Pro cases, ensure the logic to disable redirection for non-Pro workspaces is covered, either in the component or via props. This aligns with the PR objective.web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/archives/issues/(detail)/header.tsx (3)
9-9: No concerns with the import from "@/components/common".
You are correctly importingBreadcrumbLinkhere, and references in subsequent lines confirm active usage.
15-16: Project hook and breadcrumb import alignment looks good.
These imports (useProjectandProjectBreadcrumb) are consistent with the new architecture. No issues noted here.
39-39: Confirm workspace type logic for non-Pro usage.
The new<ProjectBreadcrumb />simplifies breadcrumb rendering. However, per the PR objectives, ensure the logic for disabling breadcrumb clicks for non-Pro workspaces is handled upstream or within this component if applicable.web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/archives/header.tsx (4)
10-10: Import forBreadcrumbLinkis actively used.
Confirmed usage in the subsequent breadcrumb items. This is fine.
17-18: New import ofProjectBreadcrumbis aligned with the refactoring.
The approach centralizes the project breadcrumb usage and appears consistent with the PR objective.
33-33: Retainingloaderusage fromuseProject.
No issues identified with the loader usage. It ensures the breadcrumb is rendered with proper loading states.
47-47:<ProjectBreadcrumb />usage is consistent.
This is harmonious with the overall breadcrumb refactoring, replacing explicit references to project details with a concise component.web/ce/components/issues/header.tsx (4)
6-6: Icons from lucide-react are correctly imported.
The inclusion ofCircleandExternalLinkicons does not conflict with the new breadcrumb structure.
12-12:BreadcrumbLinkandCountChipusage is valid.
The new import supports the updated breadcrumb structure and issue count display.
22-23:ProjectBreadcrumbimport aligns with refactored approach.
These lines replace the older, more manual breadcrumb approach to unify the UI logic.
56-56: Proper integration of<ProjectBreadcrumb />.
The breadcrumb usage references the project context. Ensure that any potential non-Pro workspace logic is handled within or around this component.web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/header.tsx (2)
21-21:ProjectBreadcrumbimport is appropriate.
Replacing scattered breadcrumb references with a singular import helps maintain consistency.
72-72: Responsive breadcrumb handling for mobile vs. desktop.
The fallbackBreadcrumbLinkwith label"..."ensures good UX on mobile screens, while<ProjectBreadcrumb />is displayed on larger screens. This approach is well-balanced.web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/draft-issues/header.tsx (3)
13-13: Check ifBreadcrumbLinkis actually necessary.
BreadcrumbLinkis being used later in the code, so it isn't redundant. However, ensure no references to the removedLogocomponent remain and that no dead imports linger elsewhere.
97-97: Breadcrumb usage looks good.
Using<ProjectBreadcrumb />within<Breadcrumbs>here simplifies the breadcrumb logic and improves consistency with the rest of the codebase.
22-23: Verify the Pro vs. non-Pro workspace logic.
Ensure the newly importedProjectBreadcrumbhandles redirect behavior for Pro workspaces and remains inert for non-Pro workspaces, as described in the PR objectives.web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(detail)/[viewId]/header.tsx (2)
142-142: Usage of<ProjectBreadcrumb />is consistent.
Replacing the manualBreadcrumbs.BreadcrumbItemfor project references with the standardized component improves maintainability.
36-37: Confirm conditional workspace redirects inProjectBreadcrumb.
Since the PR is meant to differentiate between Pro and non-Pro workspaces, ensure thatProjectBreadcrumbindeed applies that logic for this file's route as well.web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(detail)/header.tsx (3)
17-17: Validate continued usage ofBreadcrumbLink.
This import remains required for module navigation links. Confirm that no references to the removedLogoremain.
171-171: Breadcrumb usage is correct.
The<ProjectBreadcrumb />usage is consistent with the pattern established in the other files.
41-42: Confirm thatProjectBreadcrumbmeets the PR objective.
Ensure pro-workspace behavior is handled properly here as well.✅ Verification successful
ProjectBreadcrumb implementation aligns with PR objectives
The component is a simple presentational element that displays project information (name and logo) without any workspace-tier specific behavior, which is the correct implementation as these are basic features available to all workspace types.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash #!/bin/bash # Combining pattern matches to see if there's any mention of pro workspace logic or references fd ProjectBreadcrumb --extension ts --extension tsx --exec rg "Pro|NonPro|PLAN_TYPE|workspaceTier"Length of output: 100
Script:
#!/bin/bash # Find ProjectBreadcrumb component implementation ast-grep --pattern 'export const ProjectBreadcrumb = $_' # Find any workspace-related props or logic ast-grep --pattern 'interface ProjectBreadcrumb$_ { $$$ workspace$_?: $_; $$$ }' # Find all imports and usages rg -g '*.{ts,tsx}' "ProjectBreadcrumb" -A 5Length of output: 25596
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/header.tsx (3)
17-17: Validate removal ofLogoreferences.
No direct references toLogoremain here, so this import set continues to be relevant. Just keep an eye out for unused imports.
172-172: Consistency in<ProjectBreadcrumb />usage.
Retrofitting the breadcrumb logic with a single standardized component helps reduce duplication. Nicely done.
40-41: Confirm business logic for breadcrumb.
As with the other headers, verify that the Pro vs. non-Pro workspace logic is correctly encapsulated byProjectBreadcrumb.
Description
This PR includes the following changes:
Type of Change
Affected Areas
References
[WEB-3040]
Summary by CodeRabbit
Refactor
ProjectBreadcrumbcomponent to standardize project navigation across multiple header componentsLogoanduseParamsin various header componentsChores