-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat: refactor typing, add ColumnFormat.HoverCard #275
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,5 @@ | ||
import { UpdateIcon } from '@radix-ui/react-icons' | ||
import { TableSkeleton } from '@/components/skeleton' | ||
|
||
export default function Loading() { | ||
return ( | ||
<div className="flex flex-row items-center gap-3"> | ||
<UpdateIcon className="size-4 animate-spin" /> | ||
Loading table detail ... | ||
</div> | ||
) | ||
return <TableSkeleton className="mb-4" /> | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -31,7 +31,7 @@ interface DatabaseCount { | |||||||||||||||
export async function DatabaseBreadcrumb({ database }: Props) { | ||||||||||||||||
// Default | ||||||||||||||||
let databases: { name: string; count: number }[] = [ | ||||||||||||||||
{ name: database, count: 0 }, | ||||||||||||||||
{ name: database, count: -1 }, | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: Negative count value might be confusing Using |
||||||||||||||||
] | ||||||||||||||||
|
||||||||||||||||
try { | ||||||||||||||||
|
@@ -64,8 +64,8 @@ export async function DatabaseBreadcrumbSkeleton({ database }: Props) { | |||||||||||||||
<Internal | ||||||||||||||||
current={database} | ||||||||||||||||
databases={[ | ||||||||||||||||
{ name: database, count: 0 }, | ||||||||||||||||
{ name: 'Loading ...', count: 0 }, | ||||||||||||||||
{ name: database, count: -1 }, | ||||||||||||||||
{ name: 'Loading ...', count: -1 }, | ||||||||||||||||
]} | ||||||||||||||||
/> | ||||||||||||||||
) | ||||||||||||||||
|
@@ -93,7 +93,7 @@ function Internal({ | |||||||||||||||
|
||||||||||||||||
<DropdownMenu> | ||||||||||||||||
<DropdownMenuTrigger className="flex items-center gap-1"> | ||||||||||||||||
{current} ({currentCount} {currentCount == 1 ? 'table' : 'tables'}) | ||||||||||||||||
{current} (<Count count={currentCount} />) | ||||||||||||||||
<ChevronDownIcon /> | ||||||||||||||||
</DropdownMenuTrigger> | ||||||||||||||||
<DropdownMenuContent align="start"> | ||||||||||||||||
|
@@ -114,3 +114,10 @@ function Internal({ | |||||||||||||||
</Breadcrumb> | ||||||||||||||||
) | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
function Count({ count }: { count?: number }) { | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (performance): Consider memoizing the Count component To avoid unnecessary re-renders, consider memoizing the
Suggested change
|
||||||||||||||||
if (count == undefined || count == -1) return 'loading ...' | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (code-quality): Use block braces for ifs, whiles, etc. (
Suggested change
ExplanationIt is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing |
||||||||||||||||
if (count == 0) return '0 table' | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (code-quality): Use block braces for ifs, whiles, etc. (
Suggested change
ExplanationIt is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing |
||||||||||||||||
if (count == 1) return '1 table' | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (code-quality): Use block braces for ifs, whiles, etc. (
Suggested change
ExplanationIt is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing |
||||||||||||||||
return `${count} tables` | ||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,50 @@ | ||||||
import { | ||||||
HoverCard, | ||||||
HoverCardContent, | ||||||
HoverCardTrigger, | ||||||
} from '@/components/ui/hover-card' | ||||||
import React from 'react' | ||||||
|
||||||
export type HoverCardContent = string | React.ReactNode | ||||||
|
||||||
export type HoverCardOptions = { | ||||||
content: HoverCardContent | ||||||
} | ||||||
|
||||||
interface HoverCardProps { | ||||||
row: any | ||||||
value: any | ||||||
options?: HoverCardOptions | ||||||
} | ||||||
|
||||||
export function HoverCardFormat({ row, value, options }: HoverCardProps) { | ||||||
let { content } = options || {} | ||||||
|
||||||
if (!content) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Default content assignment Consider using a more explicit default value for
Suggested change
|
||||||
content = value | ||||||
} | ||||||
|
||||||
// Content replacement, e.g. "Hover content: [column_name]" | ||||||
if ( | ||||||
typeof content === 'string' && | ||||||
content.includes('[') && | ||||||
content.includes(']') | ||||||
) { | ||||||
const matches = content.match(/\[(.*?)\]/g) | ||||||
if (matches) { | ||||||
matches.forEach((match) => { | ||||||
const key = match.replace('[', '').replace(']', '').trim() | ||||||
const replacementValue = row.getValue(key) | ||||||
if (typeof content === 'string') { | ||||||
content = content.replace(match, replacementValue) | ||||||
} | ||||||
}) | ||||||
} | ||||||
} | ||||||
return ( | ||||||
<HoverCard> | ||||||
<HoverCardTrigger>{value}</HoverCardTrigger> | ||||||
<HoverCardContent>{content}</HoverCardContent> | ||||||
</HoverCard> | ||||||
) | ||||||
} |
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.
suggestion: Consider adding a fallback for TableSkeleton
In case
TableSkeleton
fails to load or render, consider adding a fallback UI to improve user experience.