[WEB-4050] feat: breadcrumbs revamp#7188
Conversation
WalkthroughThis update introduces a comprehensive refactor and enhancement of breadcrumb navigation components across the codebase. It standardizes breadcrumb item usage, introduces reusable components for project and feature breadcrumbs, and adds new dropdown and search-select capabilities for navigation. The changes also streamline header and quick action components, improve UI flexibility, and centralize navigation logic for project features. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Breadcrumbs
participant CommonProjectBreadcrumbs
participant ProjectFeatureBreadcrumb
participant ProjectBreadcrumb
participant Dropdown
User->>Breadcrumbs: Render with items/components
Breadcrumbs->>CommonProjectBreadcrumbs: Render with workspaceSlug, projectId, featureKey
CommonProjectBreadcrumbs->>ProjectBreadcrumb: Render project breadcrumb (dropdown)
CommonProjectBreadcrumbs->>ProjectFeatureBreadcrumb: (if featureKey) Render feature breadcrumb (dropdown)
ProjectBreadcrumb->>Dropdown: Show project switch options
ProjectFeatureBreadcrumb->>Dropdown: Show feature navigation options
User->>Dropdown: Select item
Dropdown->>Breadcrumbs: Trigger navigation to selected route
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🔇 Additional comments (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (3)
web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/archives/header.tsx (1)
69-75: Avoid unnecessary.toString()casts
workspaceSlugandprojectIdare already typed asstring | undefined.
Calling.toString()on a possibly-undefined value risks convertingundefinedinto the literal string"undefined"and leaking into URLs.-<ProjectBreadcrumb workspaceSlug={workspaceSlug?.toString()} projectId={projectId?.toString()} /> +{workspaceSlug && projectId && ( + <ProjectBreadcrumb workspaceSlug={workspaceSlug} projectId={projectId} /> +)}Guarding also prevents the breadcrumb component from rendering with invalid params.
web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/header.tsx (1)
42-60: Guard againstundefinedbefore mapping overprojectPageIds.
getCurrentProjectPageIds()can legally returnundefinedwhile the data is loading.
Calling.mapunconditionally will raise a runtime TypeError and break the header.- const switcherOptions = projectPageIds + const switcherOptions = (projectPageIds ?? [])A tiny guard is enough and keeps the component resilient during the initial loading phase.
web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/header.tsx (1)
143-154: Handle potential undefined cycle detailsSimilar to the workspace views file, the cycle details retrieval might return undefined values.
Apply this diff to handle undefined values properly:
const switcherOptions = currentProjectCycleIds ?.map((id) => { const _cycle = id === cycleId ? cycleDetails : getCycleById(id); - if (!_cycle) return; + if (!_cycle) return null; return { value: _cycle.id, query: _cycle.name, content: <SwitcherLabel name={_cycle.name} LabelIcon={ContrastIcon} />, }; }) - .filter((option) => option !== undefined) as ICustomSearchSelectOption[]; + .filter(Boolean) as ICustomSearchSelectOption[];
🧹 Nitpick comments (23)
packages/constants/src/project.ts (1)
152-160: Consider using aconstobject + union type instead of a runtime enumTS enums emit extra JS at runtime; you only need a string-literal union for typing. A lighter pattern is:
export const PROJECT_FEATURE_KEY = { WORK_ITEMS: "work_items", CYCLES: "cycles", MODULES: "modules", VIEWS: "views", PAGES: "pages", INTAKE: "intake", } as const; export type EProjectFeatureKey = typeof PROJECT_FEATURE_KEY[keyof typeof PROJECT_FEATURE_KEY];This keeps the ergonomics, removes runtime cost, and is fully tree-shakable.
web/core/components/common/switcher-label.tsx (1)
6-29: Accessibility nit: makealttext descriptive
alt="logo"gives screen-reader users no context. Consider deriving it from the entity name:- alt="logo" + alt={name ? `${name} logo` : "logo"}(You can pass
nameas a prop or compute it higher up.)web/core/components/workspace/views/header.tsx (1)
40-40: Minor type-safety nit – avoid?.toString()when value is already guaranteedBoth
ViewTabandDefaultViewTabbail out early if!workspaceSlug, so at this pointworkspaceSlugis truthy and already a string.
Using optional-chaining plustoString()widens the type tostring | undefinedand is unnecessary.- <WorkspaceViewQuickActions workspaceSlug={workspaceSlug?.toString()} view={view} /> + <WorkspaceViewQuickActions workspaceSlug={workspaceSlug} view={view} />Apply the same change to the call on line 60. This keeps the prop strictly typed as
stringand removes superfluous conversions.Also applies to: 60-60
web/core/components/project/header.tsx (1)
40-48: Localise the “Archived” labelAll other labels in this header are wrapped in
t(...). The hard-coded string here skips localisation and will surface in English for non-English users.-{isArchived && <Breadcrumbs.Item component={<BreadcrumbLink label="Archived" />} />} +{isArchived && ( + <Breadcrumbs.Item + component={<BreadcrumbLink label={t("common.archived")} />} + /> +)}web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/archives/header.tsx (1)
80-87: Key prop missing for dynamically created breadcrumbWhen
activeTabBreadcrumbDetailis truthy, a newBreadcrumbs.Itemis pushed into an array.
React will emit a warning in dev mode because neitherBreadcrumbs.Itemnor its wrapper is given akeyprop.-<Breadcrumbs.Item +<Breadcrumbs.Item + key={`archives-${activeTab}`}web/ce/components/breadcrumbs/common.tsx (2)
24-26: Eliminate redundanttoString()conversions
workspaceSlugandprojectIdare already declared asstring; converting them back to string is superfluous and can hide type problems upstream.- workspaceSlug={workspaceSlug?.toString()} - projectId={projectId?.toString()} + workspaceSlug={workspaceSlug} + projectId={projectId}
17-32: Consider memoising to avoid re-renders
CommonProjectBreadcrumbsis pure and relies only on scalar props.
Wrapping it inmemowill prevent unnecessary reconciliations in large headers where breadcrumbs seldom change.-export const CommonProjectBreadcrumbs: FC<TCommonProjectBreadcrumbProps> = (props) => { +export const CommonProjectBreadcrumbs: FC<TCommonProjectBreadcrumbProps> = memo((props) => { ... -}; +});You’ll need to import
memofromreact.web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(list)/header.tsx (1)
43-47: Breadcrumb component already converts values to string
CommonProjectBreadcrumbsinternally calls.toString()on every identifier (seecommon.tsx).
Calling?.toString()here is therefore redundant and can be dropped to reduce noise.- workspaceSlug={workspaceSlug?.toString() ?? ""} - projectId={projectId?.toString() ?? ""} + workspaceSlug={workspaceSlug} + projectId={projectId}web/ce/components/projects/settings/intake/header.tsx (1)
39-45: Unintendedborderutility may leak into layoutAdding
borderto the flex container changes the visual design of every
project-intake header. Confirm with design before merging, otherwise drop the class.-<div className="flex items-center gap-4 flex-grow border"> +<div className="flex items-center gap-4 flex-grow">web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(list)/header.tsx (1)
16-16: Inconsistent import path for shared breadcrumb moduleOther files import
@/plane-web/components/breadcrumbs, here we use
@/plane-web/components/breadcrumbs/common. Pick one barrel/explicit
path and stick to it to avoid duplicate bundles and confused auto-imports.web/ce/components/issues/header.tsx (2)
7-14: Prefer consistent alias import forCommonProjectBreadcrumbs.Most files in this PR import the breadcrumbs helper via the barrel file (
"@/plane-web/components/breadcrumbs").
Using the relative path here introduces avoidable path-divergence and makes large-scale refactors harder.-import { CommonProjectBreadcrumbs } from "../breadcrumbs/common"; +import { CommonProjectBreadcrumbs } from "@/plane-web/components/breadcrumbs";
62-70: Pass therouter.backhandler by reference to avoid re-creating a closure each render.-<Breadcrumbs onBack={() => router.back()} … +<Breadcrumbs onBack={router.back} …It’s a micro-optimisation, but it also aligns with the pattern used in other headers added in this PR.
web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(list)/header.tsx (1)
13-13: Import path inconsistencyEverywhere else the barrel import (
"@/plane-web/components/breadcrumbs") is used. Sticking to a single path avoids duplicate bundles after tree-shaking and keeps auto-imports predictable.web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(detail)/header.tsx (1)
186-197: Consider re-usingCountChipfor uniform badge stylingYou hand-rolled the badge here whereas other headers rely on the shared
CountChipcomponent. Re-using the shared component avoids styling drift and keeps future design tweaks in one place.-<span className="flex …">{workItemsCount}</span> +<CountChip count={workItemsCount} />web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/header.tsx (1)
24-27:showButtonprop is declared but never used
IPagesHeaderPropsexposesshowButton, yetPageDetailsHeaderdoesn’t consume it
(or forward it). Either wire it up or drop it from the interface to keep the API clean.web/ce/components/breadcrumbs/feature.tsx (1)
38-55:shouldRenderfilter may drop externally supplied items
additionalNavigationItemsare blindly concatenated and then filtered by
item.shouldRender, yet external callers might not set this flag.
Consider defaulting totruewhen the property is absent:- .filter((item) => item.shouldRender) + .filter((item) => item.shouldRender !== false)Prevents accidental disappearance of valid navigation entries coming from the caller.
packages/ui/src/breadcrumbs/navigation-search-dropdown.tsx (2)
17-19:disableRootHoverprop is unusedThe prop is defined but never referenced, leading to dead code and misleading API
surface. Either implement the hover-disabling logic or remove the prop entirely
until there is a concrete use-case.
53-60: Missingtype="button"on clickable elementInside a form context this
<button>will default totype="submit", causing
unexpected navigation when the dropdown lives inside a<form>.
Add an explicit type to future-proof:- <button + <button + type="button"packages/ui/src/breadcrumbs/breadcrumbs.stories.tsx (1)
201-212: Repeated identicalkeyvalues in story data
feature-3is reused for several items. Storybook warns on duplicate keys and it
obscures example behaviour. Give each entry a distinct key.- { key: "feature-3", title: "Cycles", … } - { key: "feature-3", title: "Modules", … } - { key: "feature-3", title: "Views", … } - { key: "feature-3", title: "Pages", … } + { key: "feature-cycles", title: "Cycles", … } + { key: "feature-modules", title: "Modules", … } + { key: "feature-views", title: "Views", … } + { key: "feature-pages", title: "Pages", … }web/app/(all)/[workspaceSlug]/(projects)/workspace-views/header.tsx (1)
136-138: Simplify options filtering logicThe current implementation filters twice - once implicitly in the map and once explicitly. This can be simplified.
Consider using
flatMapfor cleaner code:-const switcherOptions = [...defaultOptions, ...workspaceOptions].filter( - (option) => option !== undefined -) as ICustomSearchSelectOption[]; +const switcherOptions = [ + ...defaultOptions, + ...currentWorkspaceViews?.flatMap((view) => { + const _view = getViewDetailsById(view); + return _view + ? [{ + value: _view.id, + query: _view.name, + content: <SwitcherLabel name={_view.name} LabelIcon={Layers} />, + }] + : []; + }) ?? [] +] as ICustomSearchSelectOption[];web/ce/components/breadcrumbs/project.tsx (1)
60-67: Unify route formatting
router.pushis invoked with/issues(no trailing slash) in the dropdown but/issues/(trailing slash) in the fallback click handler.
In Next.js this can trigger an extra redirect or cache miss. Pick one style (prefer the repo-wide convention) and use it consistently.packages/ui/src/breadcrumbs/navigation-dropdown.tsx (1)
34-40: AllowhandleOnClickfor the last breadcrumb tooCurrent guard
if (!isLast) { … handleOnClick?.() }silences the callback when this item is the tail of the trail.
There are valid cases (e.g., refresh current view) where the last item still needs a click handler. Consider removing the!isLastrestriction or exposing a separate prop to control it.packages/ui/src/breadcrumbs/breadcrumbs.tsx (1)
44-47: Remove redundant fragmentThe fragment adds no semantic value and triggers the
noUselessFragmentslint error.- return ( - <> - <BreadcrumbItemLoader /> - </> - ); + return <BreadcrumbItemLoader />;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
admin/core/components/auth-header.tsx(2 hunks)packages/constants/src/project.ts(1 hunks)packages/ui/src/breadcrumbs/breadcrumbs.stories.tsx(1 hunks)packages/ui/src/breadcrumbs/breadcrumbs.tsx(3 hunks)packages/ui/src/breadcrumbs/index.ts(1 hunks)packages/ui/src/breadcrumbs/navigation-dropdown.tsx(3 hunks)packages/ui/src/breadcrumbs/navigation-search-dropdown.tsx(1 hunks)packages/ui/src/dropdowns/custom-search-select.tsx(2 hunks)packages/ui/src/header/header.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/active-cycles/header.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/analytics/header.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/browse/[workItem]/header.tsx(2 hunks)web/app/(all)/[workspaceSlug]/(projects)/drafts/header.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/header.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/profile/[userId]/header.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/archives/header.tsx(2 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/archives/issues/(detail)/header.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/header.tsx(3 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(list)/header.tsx(2 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/draft-issues/header.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(detail)/header.tsx(3 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(list)/header.tsx(2 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/header.tsx(3 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/header.tsx(2 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(detail)/[viewId]/header.tsx(4 hunks)web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(list)/header.tsx(2 hunks)web/app/(all)/[workspaceSlug]/(projects)/stickies/header.tsx(1 hunks)web/app/(all)/[workspaceSlug]/(projects)/workspace-views/[globalViewId]/page.tsx(0 hunks)web/app/(all)/[workspaceSlug]/(projects)/workspace-views/header.tsx(3 hunks)web/ce/components/breadcrumbs/common.tsx(1 hunks)web/ce/components/breadcrumbs/feature.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/navigation/helper.tsx(1 hunks)web/ce/components/projects/navigation/index.ts(1 hunks)web/ce/components/projects/settings/intake/header.tsx(2 hunks)web/core/components/common/breadcrumb-link.tsx(1 hunks)web/core/components/common/switcher-label.tsx(2 hunks)web/core/components/project/header.tsx(1 hunks)web/core/components/workspace-notifications/sidebar/header/root.tsx(1 hunks)web/core/components/workspace/views/default-view-quick-action.tsx(2 hunks)web/core/components/workspace/views/header.tsx(2 hunks)web/core/components/workspace/views/quick-action.tsx(3 hunks)
💤 Files with no reviewable changes (1)
- web/app/(all)/[workspaceSlug]/(projects)/workspace-views/[globalViewId]/page.tsx
🧰 Additional context used
🧬 Code Graph Analysis (12)
web/app/(all)/[workspaceSlug]/(projects)/active-cycles/header.tsx (2)
packages/i18n/src/store/index.ts (1)
t(211-232)web/ce/components/workspace/upgrade-badge.tsx (1)
UpgradeBadge(11-30)
web/core/components/workspace/views/header.tsx (2)
web/core/components/workspace/views/quick-action.tsx (1)
WorkspaceViewQuickActions(24-126)web/core/components/workspace/views/default-view-quick-action.tsx (1)
DefaultWorkspaceViewQuickActions(20-97)
web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(list)/header.tsx (2)
web/core/hooks/use-app-router.tsx (1)
useAppRouter(4-4)web/ce/components/breadcrumbs/common.tsx (1)
CommonProjectBreadcrumbs(17-32)
web/core/components/workspace/views/default-view-quick-action.tsx (1)
packages/types/src/workspace-views.d.ts (1)
TStaticViewTypes(35-39)
web/app/(all)/[workspaceSlug]/(projects)/browse/[workItem]/header.tsx (3)
packages/ui/src/breadcrumbs/breadcrumbs.tsx (1)
Breadcrumbs(190-190)web/ce/components/breadcrumbs/common.tsx (1)
CommonProjectBreadcrumbs(17-32)web/core/store/router.store.ts (1)
projectId(85-87)
web/app/(all)/[workspaceSlug]/(projects)/workspace-views/header.tsx (8)
web/core/hooks/use-app-router.tsx (1)
useAppRouter(4-4)packages/constants/src/workspace.ts (1)
DEFAULT_GLOBAL_VIEWS_LIST(228-248)web/core/components/common/switcher-label.tsx (1)
SwitcherLabel(38-46)web/core/store/global-view.store.ts (1)
currentWorkspaceViews(67-76)packages/types/src/common.d.ts (1)
ICustomSearchSelectOption(30-36)packages/ui/src/breadcrumbs/navigation-search-dropdown.tsx (1)
BreadcrumbNavigationSearchDropdown(21-96)web/core/components/workspace/views/quick-action.tsx (1)
WorkspaceViewQuickActions(24-126)web/core/components/workspace/views/default-view-quick-action.tsx (1)
DefaultWorkspaceViewQuickActions(20-97)
packages/ui/src/breadcrumbs/breadcrumbs.tsx (1)
packages/ui/src/tooltip/tooltip.tsx (1)
Tooltip(36-110)
web/ce/components/projects/settings/intake/header.tsx (1)
web/ce/components/breadcrumbs/common.tsx (1)
CommonProjectBreadcrumbs(17-32)
web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/header.tsx (3)
web/ce/components/breadcrumbs/common.tsx (1)
CommonProjectBreadcrumbs(17-32)packages/ui/src/breadcrumbs/navigation-search-dropdown.tsx (1)
BreadcrumbNavigationSearchDropdown(21-96)web/core/components/common/switcher-label.tsx (1)
SwitcherIcon(13-29)
web/core/components/common/switcher-label.tsx (2)
packages/types/src/common.d.ts (1)
TLogoProps(13-24)apiserver/plane/db/models/workspace.py (1)
logo_url(145-153)
web/ce/components/projects/navigation/helper.tsx (2)
web/core/store/router.store.ts (1)
projectId(85-87)web/core/components/workspace/sidebar/project-navigation.tsx (1)
TNavigationItem(19-28)
web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(list)/header.tsx (1)
web/ce/components/breadcrumbs/common.tsx (1)
CommonProjectBreadcrumbs(17-32)
🪛 Biome (1.9.4)
packages/ui/src/breadcrumbs/breadcrumbs.tsx
[error] 40-56: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (27)
packages/ui/src/dropdowns/custom-search-select.tsx (1)
100-106: Readable class-name merge looks goodSwitching to
cnimproves clarity and keeps disabled/hover styles consistent.web/core/components/common/switcher-label.tsx (1)
42-42: Good extraction of icon logicReplacing the inline ternary with
SwitcherIconcentralises icon decisions and keepsSwitcherLabeltidy.packages/ui/src/header/header.tsx (1)
41-44:flex-growmay overflow on very long breadcrumb sets
flex-growtogether withmax-w-[80%]generally works, but check narrow viewports: long labels could still push the right-hand actions off-screen. If that happens, considermin-w-0on the grow element or wrapping crumbs in an overflow-auto container.web/ce/components/projects/navigation/index.ts (1)
1-1: Index barrel looks fineSimple re-export keeps import paths clean.
packages/ui/src/breadcrumbs/index.ts (1)
3-3: Barrel update is clear and correct
navigation-search-dropdownis now surfaced through the public API – makes sense and won’t introduce tree-shaking issues.web/app/(all)/[workspaceSlug]/(projects)/drafts/header.tsx (1)
44-49: Nice migration toBreadcrumbs.ItemAdopts the new
componentprop cleanly and keeps the JSX concise. No issues spotted.web/ce/components/breadcrumbs/index.ts (1)
1-3: ```shell
#!/bin/bash
echo "Listing all export statements in each module for manual inspection:"
for f in web/ce/components/breadcrumbs/{common,feature,project}.tsx; do
echo -e "\n=== $f ==="
rg -n 'export ' "$f"
done</details> <details> <summary>web/app/(all)/[workspaceSlug]/(projects)/active-cycles/header.tsx (1)</summary> `17-27`: **Breadcrumb refactor LGTM** Implementation matches the new API, icon & translation hook usage are consistent. </details> <details> <summary>web/app/(all)/[workspaceSlug]/(projects)/header.tsx (1)</summary> `31-36`: **Correct adoption of new breadcrumb API** Clean swap to `Breadcrumbs.Item` with no regressions noted. </details> <details> <summary>admin/core/components/auth-header.tsx (1)</summary> `70-78`: **Breadcrumb API migration looks correct** The switch to `Breadcrumbs.Item` with the `component` prop is consistent with the updated UI package and compiles cleanly – nothing else needed here. Also applies to: 82-85 </details> <details> <summary>web/app/(all)/[workspaceSlug]/(projects)/profile/[userId]/header.tsx (1)</summary> `55-63`: **Breadcrumb refactor applied flawlessly** The new `Breadcrumbs.Item` usage together with `BreadcrumbLink` is fully aligned with the refactored breadcrumbs API. No issues spotted. </details> <details> <summary>web/app/(all)/[workspaceSlug]/(projects)/analytics/header.tsx (1)</summary> `42-49`: **Analytics header crumbs migrated correctly** Implementation matches the new API and keeps translations & icons intact – looks good. </details> <details> <summary>web/app/(all)/[workspaceSlug]/(projects)/stickies/header.tsx (1)</summary> `28-35`: **Stickies breadcrumb update looks good** The conversion to `Breadcrumbs.Item` with a `BreadcrumbLink` component is correct and preserves icon styling. </details> <details> <summary>web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/draft-issues/header.tsx (1)</summary> `95-106`: ```shell #!/bin/bash set -e echo "=== Looking for Props definition in BreadcrumbLink ===" rg -nE "^(type|interface) Props" -C5 web/core/components/common/breadcrumb-link.tsx || echo "No Props type found" echo -e "\n=== Searching for href handling in component implementation ===" rg -n "<(Link|a)" -C5 web/core/components/common/breadcrumb-link.tsx || echo "No anchor or Link usage detected" echo -e "\n=== Showing first 200 lines of the component for manual inspection ===" sed -n '1,200p' web/core/components/common/breadcrumb-link.tsxweb/core/components/workspace-notifications/sidebar/header/root.tsx (1)
29-37: Breadcrumb refactor looks goodThe migration to
Breadcrumbs.Itemwith the newcomponentprop is correct and consistent with the revamped breadcrumbs API. No further action required.web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(list)/header.tsx (1)
22-23: ```shell
#!/bin/bashLocate and print the beginning of the header.tsx file where useParams is used
find . -type f -name header.tsx -path "/modules/" -print -exec sed -n '1,200p' {} ;
</details> <details> <summary>web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(list)/header.tsx (1)</summary> `58-65`: ```bash #!/bin/bash set -e # Locate only the header.tsx files under any "(list)" directory paths=$(find web/app -type f | grep "/(list)/header\.tsx" || true) if [ -z "$paths" ]; then echo "No list header.tsx files found under web/app" exit 1 fi for file in $paths; do echo "---- Inspecting: $file ----" # Print lines 50–90 to capture the snippet around lines 58–65 sed -n '50,90p' "$file" echo doneweb/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(list)/header.tsx (1)
26-31: Empty-string fall-back may leak into the generated URL
CommonProjectBreadcrumbsreceivesworkspaceSlug={workspaceSlug?.toString() ?? ""}and the same forprojectId.
If either param is stillundefinedon first render, the component will receive an empty string and generate routes like"/projects//views", which later stick in the history stack.Consider short-circuiting the render (as you did in other headers) or guarding the breadcrumb call:
{workspaceSlug && projectId && ( <CommonProjectBreadcrumbs … /> )}web/app/(all)/[workspaceSlug]/(projects)/browse/[workItem]/header.tsx (1)
36-50: Minor: keep the back-handler style consistentOther headers pass
router.backdirectly; this one wraps it inonBack={router.back}already — good.
No action needed, just highlighting for future consistency reviews.web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/modules/(detail)/header.tsx (1)
173-180: Guard against undefined route params when pushing module switch
router.push(\/${workspaceSlug}/projects/${projectId}/modules/${value}`)will emit/undefined/projects/undefined/modules/…` on the first couple of renders when the params are not yet populated.Either:
- Wrap the call in a param check, or
- Pre-compute
workspaceSlug?.toString()andprojectId?.toString()once the params are available and skip rendering the dropdown until then.This avoids broken navigation entries in the history stack.
web/app/(all)/[workspaceSlug]/(projects)/workspace-views/header.tsx (1)
208-213: Extract view type detection logicThe view rendering logic correctly handles both workspace and default views.
web/ce/components/projects/navigation/helper.tsx (1)
1-78: Well-structured navigation helper implementationThe helper function is well-implemented with:
- Proper typing and consistent structure
- Clear separation of concerns
- Appropriate use of feature flags for conditional rendering
- Correct permission mapping
web/core/components/workspace/views/quick-action.tsx (1)
82-123: Clean refactor to CustomMenu implementationThe refactor successfully:
- Simplifies the component interface by removing unused props
- Implements proper event handling with preventDefault and stopPropagation
- Uses consistent styling with the ellipsis button pattern
- Maintains proper conditional rendering based on permissions
web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(detail)/header.tsx (2)
169-193: Clean breadcrumb implementation with feature keyThe breadcrumb implementation properly uses:
- CommonProjectBreadcrumbs with appropriate feature key
- BreadcrumbNavigationSearchDropdown for cycle selection
- Proper icon and styling integration
194-207: Well-implemented work items count displayThe tooltip implementation for work items count is well done with:
- Proper mobile responsiveness check
- Clear tooltip content with singular/plural handling
- Appropriate styling and positioning
web/core/components/common/breadcrumb-link.tsx (2)
16-46: Excellent use of memoization for performanceThe refactor demonstrates best practices:
- Proper use of React.memo for preventing unnecessary re-renders
- Clear component names with displayName for debugging
- Modular sub-components for better maintainability
- Efficient memoization of props and content
47-74: Clean implementation with proper conditional renderingThe main component implementation is well-structured:
- Proper memoization of computed values
- Clean conditional rendering based on href presence
- Correct integration with the new Breadcrumbs.ItemWrapper
- Good handling of tooltip behavior for mobile devices
web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(list)/header.tsx
Show resolved
Hide resolved
...[workspaceSlug]/(projects)/projects/(detail)/[projectId]/archives/issues/(detail)/header.tsx
Show resolved
Hide resolved
...app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/header.tsx
Show resolved
Hide resolved
.../[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(detail)/[viewId]/header.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
web/ce/components/breadcrumbs/project-feature.tsx (2)
36-40: Memoise and de-duplicateallNavigationItemsEvery render allocates a new array and can re-create dropdown items even if neither props nor store values changed. In MobX observers this is easy to miss because reactive changes trigger re-renders often.
- const allNavigationItems = [...(additionalNavigationItems || []), ...navigationItems]; + const allNavigationItems = useMemo(() => { + // Prevent duplicate keys while preserving caller order. + const map = new Map<string, TNavigationItem>(); + [...(additionalNavigationItems ?? []), ...navigationItems].forEach((i) => { + if (!map.has(i.key)) map.set(i.key, i); + }); + return Array.from(map.values()); + }, [additionalNavigationItems, navigationItems]);This avoids unnecessary allocations and removes potential key collisions that would break React’s list diffing.
64-65: Separator visibility should respectisLast
showSeparator={false}is hard-coded, so non-last items lose the chevron separator. Prefer:- showSeparator={false} + showSeparator={!isLast}This keeps visual continuity for intermediate crumbs.
web/core/components/common/breadcrumb-link.tsx (2)
64-72: MissingpassHref/prefetchoptions & anchor semantics
next/linkno longer injectshrefinto non-anchor children after v13 whenlegacyBehavioris off. Wrapping a non-anchor component withoutpassHrefcan break semantics & prefetching:- <Link href={href}> + <Link href={href} passHref> <ItemWrapper {...itemWrapperProps}>{content}</ItemWrapper> </Link>Consider adding
prefetch={false}where prefetching large pages is undesirable.
17-25: Accessibility: mark purely decorative iconsWhen the breadcrumb shows both icon + label, the icon should be hidden from screen readers to avoid duplicate announcements.
- <div className="flex size-4 items-center justify-center overflow-hidden !text-[1rem]">{icon}</div> + <div + aria-hidden={label ? true : undefined} + className="flex size-4 items-center justify-center overflow-hidden !text-[1rem]" + > + {icon} + </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
web/ce/components/breadcrumbs/common.tsx(1 hunks)web/ce/components/breadcrumbs/index.ts(1 hunks)web/ce/components/breadcrumbs/project-feature.tsx(1 hunks)web/core/components/common/breadcrumb-link.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/ce/components/breadcrumbs/index.ts
- web/ce/components/breadcrumbs/common.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
web/ce/components/breadcrumbs/project-feature.tsx (1)
47-55: Unsafeas FC<ISvgIcons>cast
item.iconis blindly asserted toFC<ISvgIcons>. If any navigation item carries a plain JSX element or a string, TypeScript will not warn but runtime will break whenReact.createElementreceives an invalid type. Consider narrowing the type at source or gracefully falling back:- icon: item.icon as FC<ISvgIcons>, + icon: + typeof item.icon === "function" + ? (item.icon as FC<ISvgIcons>) + : undefined,
* chore: project feature enum added * feat: revamp breadcrumb and add navigation dropdown component * chore: custom search select component refactoring * chore: breadcrumb stories added * chore: switch label and breadcrumb link component refactor * chore: project navigation helper function added * chore: common breadcrumb component added * chore: breadcrumb refactoring * chore: code refactor * chore: code refactor * fix: build error * fix: nprogress and button tooltip * chore: code refactor * chore: workspace view breadcrumb improvements * chore: code refactor * chore: code refactor * chore: code refactor * chore: code refactor --------- Co-authored-by: vamsikrishnamathala <matalav55@gmail.com>
5 participants
Summary
This PR includes a complete revamp of the breadcrumbs component with the following enhancements:
Reference
[WEB-4050]
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Documentation