AddressView rendering inside breadcrumbs #1366
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the breadcrumb system to support rendering AddressView components for address-type breadcrumbs instead of plain text. The change enables better address display with features like alias support and click-to-copy functionality within breadcrumbs.
- Updates breadcrumb data structure to support typed breadcrumb objects with address rendering
- Modifies the contract route to pass address breadcrumbs as typed objects
- Conditionally renders the "Clear alias" menu item only when an alias exists
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| gui/src/routes/home/_l/contracts/_l/$chainId.$address.tsx | Changes breadcrumb from string to typed object for address rendering |
| gui/src/components/Breadcrumbs.tsx | Adds support for address-type breadcrumbs with AddressView rendering |
| gui/src/components/AddressView.tsx | Conditionally shows "Clear alias" menu item only when alias exists |
gui/src/components/Breadcrumbs.tsx
Outdated
| @@ -19,13 +20,24 @@ export function Breadcrumbs() { | |||
| if (typeof context.breadcrumb === "string") { | |||
| if (context.breadcrumb === acc[acc.length - 1]?.label) return acc; | |||
| acc.push({ label: context.breadcrumb, path: pathname }); | |||
| } else if (context.breadcrumb.type === "address") { | |||
| if (context.breadcrumb.value === acc[acc.length - 1]?.label) return acc; | |||
There was a problem hiding this comment.
This comparison will always fail because context.breadcrumb.value is a string but acc[acc.length - 1]?.label is now a React.ReactNode (AddressView component). The duplicate check for address breadcrumbs won't work correctly.
| if (context.breadcrumb.value === acc[acc.length - 1]?.label) return acc; | |
| if (context.breadcrumb.value === acc[acc.length - 1]?.rawLabel) return acc; |
Copilot uses AI. Check for mistakes.
gui/src/components/Breadcrumbs.tsx
Outdated
| } else { | ||
| if (context.breadcrumb.label === acc[acc.length - 1]?.label) return acc; | ||
| acc.push(context.breadcrumb); | ||
| } | ||
| return acc; | ||
| }, | ||
| [] as { label: string; path: string }[], | ||
| [] as { label: React.ReactNode; path: string; type?: string }[], |
There was a problem hiding this comment.
[nitpick] The breadcrumb array type should be properly defined with a type interface rather than inline typing. Consider creating a BreadcrumbItem interface to improve type safety and maintainability.
| [] as { label: React.ReactNode; path: string; type?: string }[], | |
| [] as BreadcrumbItem[], |
Copilot uses AI. Check for mistakes.
gui/src/components/Breadcrumbs.tsx
Outdated
| @@ -19,13 +20,24 @@ export function Breadcrumbs() { | |||
| if (typeof context.breadcrumb === "string") { | |||
| if (context.breadcrumb === acc[acc.length - 1]?.label) return acc; | |||
| acc.push({ label: context.breadcrumb, path: pathname }); | |||
| } else if (context.breadcrumb.type === "address") { | |||
There was a problem hiding this comment.
The code assumes context.breadcrumb has a type property when it's not a string, but there's no type safety ensuring this. Consider adding proper TypeScript interfaces for different breadcrumb types.
| } else if (context.breadcrumb.type === "address") { | |
| } else if (isAddressBreadcrumb(context.breadcrumb)) { |
Copilot uses AI. Check for mistakes.
No description provided.