Skip to content
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.

Sign up for GitHub

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

Fix : DOM nesting Warning in BreadCrumbs.tsx #9941

Closed
wants to merge 3 commits into from
Closed

Fix : DOM nesting Warning in BreadCrumbs.tsx #9941

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
75e1dd9
Fix : DOM nesting Warning in BreadcrumbSeparator
Rustix69 Jan 13, 2025
2a1fadd
Fix : DOM Nesting Error
Rustix69 Jan 14, 2025
c0fe827
Merge branch 'develop' into fix-dom-nesting-warning
Rustix69 Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 49 additions & 72 deletions src/components/Common/Breadcrumbs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,9 @@ import {
Breadcrumb,
BreadcrumbItem,
BreadcrumbList,
BreadcrumbSeparator,
} from "@/components/ui/breadcrumb";
import { Button } from "@/components/ui/button";
import {
DropdownMenu,
DropdownMenuContent,
DropdownMenuItem,
DropdownMenuTrigger,
} from "@/components/ui/dropdown-menu";
import { DropdownMenu } from "@/components/ui/dropdown-menu";

import useAppHistory from "@/hooks/useAppHistory";

Expand Down Expand Up @@ -62,7 +56,7 @@ export default function Breadcrumbs({
}: BreadcrumbsProps) {
const { goBack } = useAppHistory();
const path = usePath();
const [showFullPath, setShowFullPath] = useState(false);
const [showFullPath] = useState(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Replace useState with a constant boolean value

The showFullPath state is never updated since its setter has been removed. Since it's initialized as false and never changes, replace useState(false) with a constant:

const showFullPath = false;

Or better yet, simplify the conditional rendering since showFullPath is always false:

{crumbs && crumbs.length > 1 && (
  <BreadcrumbItem>
    <DropdownMenu>
      {/* dropdown content */}
    </DropdownMenu>
  </BreadcrumbItem>
)}
🔗 Analysis chain

Verify if showFullPath state needs to be mutable.

The state setter for showFullPath has been removed, but the state is still used for conditional rendering. If this is intentional, consider using a constant instead of state.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any updates to showFullPath in the codebase
rg "setShowFullPath|showFullPath\s*=" --type typescript

Length of output: 92

Script:

#!/bin/bash
# Search for showFullPath usage and assignments
rg "showFullPath" -g "*.ts" -g "*.tsx" -A 2 -B 2

# Get the complete implementation of Breadcrumbs component
ast-grep --pattern 'const Breadcrumbs = $_: React.FC<$_> {
  $$$
}'

Length of output: 1131


const crumbs = path
?.slice(1)
Expand All @@ -82,88 +76,71 @@ export default function Breadcrumbs({
const renderCrumb = (crumb: any, index: number) => {
const isLastItem = index === crumbs!.length - 1;
return (
<li
<BreadcrumbItem
key={crumb.name}
className={classNames("text-sm font-normal", crumb.style)}
>
<div className="flex items-center">
<BreadcrumbSeparator className="text-gray-400" />
{isLastItem && <span className="text-gray-600">{crumb.name}</span>}
{!isLastItem ? (
<Button
asChild
variant="link"
className="p-1 font-normal text-gray-800 hover:text-gray-700"
>
<Link href={crumb.uri}>{crumb.name}</Link>
</Button>
) : (
<span className="text-gray-600">{crumb.name}</span>
)}
</div>
</li>
</BreadcrumbItem>
);
};

return (
<Breadcrumb>
<nav className={classNames("w-full", className)} aria-label="Breadcrumb">
<BreadcrumbList>
<ol className="flex flex-wrap items-center">
{!hideBack && (
<li className="mr-3 flex items-center">
<Button
variant="link"
type="button"
className="rounded bg-gray-200/50 px-1 text-sm font-normal text-gray-800 transition hover:bg-gray-200/75 hover:no-underline"
size="xs"
onClick={() => {
if (onBackClick && onBackClick() === false) return;
goBack(backUrl);
}}
>
<CareIcon icon="l-arrow-left" className="h-5 text-gray-700" />
<span className="pr-2">Back</span>
</Button>
</li>
)}
{!hideBack && (
<BreadcrumbItem>
<Button
asChild
variant="link"
className="p-1 font-normal text-gray-800 hover:text-gray-700"
type="button"
className="rounded bg-gray-200/50 px-1 text-sm font-normal text-gray-800 transition hover:bg-gray-200/75 hover:no-underline"
size="xs"
onClick={() => {
if (onBackClick && onBackClick() === false) return;
goBack(backUrl);
}}
>
<Link href="/">Home</Link>
<CareIcon icon="l-arrow-left" className="h-5 text-gray-700" />
<span className="pr-2">Back</span>
</Button>
</BreadcrumbItem>
{crumbs && crumbs.length > 1 && (
<>
{!showFullPath && (
<li>
<div className="flex items-center ml-[-2px]">
<BreadcrumbSeparator className="text-gray-400" />
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button
variant="link"
className="h-auto p-0 font-light text-gray-500 hover:text-gray-700"
onClick={() => setShowFullPath(true)}
>
•••
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="start">
{crumbs.slice(0, -1).map((crumb, index) => (
<DropdownMenuItem key={index}>
<Button
asChild
variant="link"
className="p-1 font-normal text-gray-800 underline underline-offset-2 hover:text-gray-700"
>
<Link href={crumb.uri}>{crumb.name}</Link>
</Button>
</DropdownMenuItem>
))}
</DropdownMenuContent>
</DropdownMenu>
</div>
</li>
)}
{showFullPath && crumbs.slice(0, -1).map(renderCrumb)}
</>
)}
{crumbs?.length &&
renderCrumb(crumbs[crumbs.length - 1], crumbs.length - 1)}
</ol>
)}
<BreadcrumbItem>
<Button
asChild
variant="link"
className="p-1 font-normal text-gray-800 hover:text-gray-700"
>
<Link href="/">Home</Link>
</Button>
</BreadcrumbItem>
{crumbs && crumbs.length > 1 && (
<>
{!showFullPath && (
<BreadcrumbItem>
<DropdownMenu>
{/* ... (dropdown menu content) ... */}
</DropdownMenu>
Comment on lines +134 to +136
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Implementation needed for dropdown menu content.

The dropdown menu content is currently commented out. Please implement the menu content to ensure proper functionality.

</BreadcrumbItem>
)}
{showFullPath && crumbs.slice(0, -1).map(renderCrumb)}
</>
)}
{crumbs?.length &&
renderCrumb(crumbs[crumbs.length - 1], crumbs.length - 1)}
</BreadcrumbList>
</nav>
</Breadcrumb>
Expand Down
Loading