-
Notifications
You must be signed in to change notification settings - Fork 632
fix: Format pagination numbers in log footers #3969
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
Changes from 16 commits
adc1ada
a9b6ccf
47e0d42
088f9a3
3ed115d
a1150db
529fcdb
89f2bfb
f392003
4203d27
bd0bf96
ecf5a04
c8a35f0
0769a91
4fe2d24
a01a4c2
b8ebdd8
e2d9de7
9e3b50e
433b53c
7cb8de3
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,6 +1,7 @@ | ||
| "use client"; | ||
| import { VirtualTable } from "@/components/virtual-table/index"; | ||
| import type { Column } from "@/components/virtual-table/types"; | ||
| import { formatNumberFull } from "@/lib/fmt"; | ||
| import { shortenId } from "@/lib/shorten-id"; | ||
| import type { KeyDetails } from "@/lib/trpc/routers/api/keys/query-api-keys/schema"; | ||
| import { BookBookmark, Dots, Focus, Key } from "@unkey/icons"; | ||
|
|
@@ -28,7 +29,7 @@ import { getRowClassName } from "./utils/get-row-class"; | |
| const KeysTableActionPopover = dynamic( | ||
| () => | ||
| import("./components/actions/keys-table-action.popover.constants").then( | ||
| (mod) => mod.KeysTableActions, | ||
| (mod) => mod.KeysTableActions | ||
| ), | ||
| { | ||
| ssr: false, | ||
|
|
@@ -37,13 +38,16 @@ const KeysTableActionPopover = dynamic( | |
| type="button" | ||
| className={cn( | ||
| "group-data-[state=open]:bg-gray-6 group-hover:bg-gray-6 group size-5 p-0 rounded m-0 items-center flex justify-center", | ||
| "border border-gray-6 group-hover:border-gray-8 ring-2 ring-transparent focus-visible:ring-gray-7 focus-visible:border-gray-7", | ||
| "border border-gray-6 group-hover:border-gray-8 ring-2 ring-transparent focus-visible:ring-gray-7 focus-visible:border-gray-7" | ||
| )} | ||
| > | ||
| <Dots className="group-hover:text-gray-12 text-gray-11" size="sm-regular" /> | ||
| <Dots | ||
| className="group-hover:text-gray-12 text-gray-11" | ||
| size="sm-regular" | ||
| /> | ||
| </button> | ||
| ), | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| export const KeysList = ({ | ||
|
|
@@ -53,9 +57,10 @@ export const KeysList = ({ | |
| keyspaceId: string; | ||
| apiId: string; | ||
| }) => { | ||
| const { keys, isLoading, isLoadingMore, loadMore, totalCount, hasMore } = useKeysListQuery({ | ||
| keyAuthId: keyspaceId, | ||
| }); | ||
| const { keys, isLoading, isLoadingMore, loadMore, totalCount, hasMore } = | ||
| useKeysListQuery({ | ||
| keyAuthId: keyspaceId, | ||
| }); | ||
| const [selectedKey, setSelectedKey] = useState<KeyDetails | null>(null); | ||
| const [navigatingKeyId, setNavigatingKeyId] = useState<string | null>(null); | ||
| // Add state for selected keys | ||
|
|
@@ -98,13 +103,17 @@ export const KeysList = ({ | |
| className={cn( | ||
| "size-5 rounded flex items-center justify-center cursor-pointer", | ||
| identity ? "bg-successA-3" : "bg-grayA-3", | ||
| isSelected && "bg-brand-5", | ||
| isSelected && "bg-brand-5" | ||
| )} | ||
| onMouseEnter={() => setHoveredKeyId(key.id)} | ||
| onMouseLeave={() => setHoveredKeyId(null)} | ||
| > | ||
| {isNavigating ? ( | ||
| <div className={cn(identity ? "text-successA-11" : "text-grayA-11")}> | ||
| <div | ||
| className={cn( | ||
| identity ? "text-successA-11" : "text-grayA-11" | ||
| )} | ||
| > | ||
| <Loading size={18} /> | ||
| </div> | ||
| ) : ( | ||
|
|
@@ -155,17 +164,24 @@ export const KeysList = ({ | |
| rel="noopener noreferrer" | ||
| aria-disabled={isNavigating} | ||
| > | ||
| <span className="font-mono bg-gray-4 p-1 rounded">{identity}</span> | ||
| <span className="font-mono bg-gray-4 p-1 rounded"> | ||
| {identity} | ||
| </span> | ||
| </Link> | ||
| ) : ( | ||
| <span className="font-mono bg-gray-4 p-1 rounded">{identity}</span> | ||
| <span className="font-mono bg-gray-4 p-1 rounded"> | ||
| {identity} | ||
| </span> | ||
| )} | ||
|
Comment on lines
+177
to
185
Contributor
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. 🧹 Nitpick (assertive) Make the tooltip trigger focusable for keyboard users iconContainer is a div; with asChild, the trigger won’t be focusable. Add tabIndex and an aria-label for a11y. - {React.cloneElement(iconContainer, {
- className: cn(
- iconContainer.props.className,
- "cursor-pointer"
- ),
- })}
+ {React.cloneElement(iconContainer, {
+ className: cn(iconContainer.props.className, "cursor-pointer"),
+ tabIndex: 0,
+ role: "button",
+ "aria-label": "Key identity info",
+ })}Also applies to: 179-184 🤖 Prompt for AI Agents |
||
| </> | ||
| } | ||
| asChild | ||
| > | ||
| {React.cloneElement(iconContainer, { | ||
| className: cn(iconContainer.props.className, "cursor-pointer"), | ||
| className: cn( | ||
| iconContainer.props.className, | ||
| "cursor-pointer" | ||
| ), | ||
| })} | ||
| </InfoTooltip> | ||
| ) : ( | ||
|
|
@@ -205,7 +221,11 @@ export const KeysList = ({ | |
| header: "Value", | ||
| width: "15%", | ||
| render: (key) => ( | ||
| <HiddenValueCell value={key.start} title="Value" selected={selectedKey?.id === key.id} /> | ||
| <HiddenValueCell | ||
| value={key.start} | ||
| title="Value" | ||
| selected={selectedKey?.id === key.id} | ||
| /> | ||
| ), | ||
| }, | ||
| { | ||
|
|
@@ -266,7 +286,7 @@ export const KeysList = ({ | |
| selectedKeys, | ||
| toggleSelection, | ||
| hoveredKeyId, | ||
| ], | ||
| ] | ||
| ); | ||
|
|
||
| const getSelectedKeysState = useCallback(() => { | ||
|
|
@@ -291,17 +311,15 @@ export const KeysList = ({ | |
|
|
||
| // Early exit if we already found a mix | ||
| if (!allEnabled && !allDisabled) { | ||
| break; | ||
| return "mixed"; | ||
| } | ||
| } | ||
|
|
||
| if (allEnabled) { | ||
| return "all-enabled"; | ||
| } | ||
| if (allDisabled) { | ||
| return "all-disabled"; | ||
| } | ||
| return "mixed"; | ||
|
|
||
| return "all-disabled"; | ||
| }, [selectedKeys, keys]); | ||
|
|
||
| return ( | ||
|
|
@@ -330,9 +348,12 @@ export const KeysList = ({ | |
| ), | ||
| countInfoText: ( | ||
| <div className="flex gap-2"> | ||
| <span>Showing</span> <span className="text-accent-12">{keys.length}</span> | ||
| <span>Showing</span>{" "} | ||
| <span className="text-accent-12"> | ||
| {formatNumberFull(keys.length)} | ||
| </span> | ||
| <span>of</span> | ||
| {totalCount} | ||
| {formatNumberFull(totalCount)} | ||
| <span>keys</span> | ||
| </div> | ||
|
Comment on lines
+361
to
368
Contributor
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. 🛠️ Refactor suggestion Reuse a memoized number formatter Apply the shared formatter to keep consistency with the rest of the PR. - <span className="text-accent-12">
- {new Intl.NumberFormat().format(keys.length)}
- </span>
+ <span className="text-accent-12">{nf.format(keys.length)}</span>
...
- {new Intl.NumberFormat().format(totalCount)}
+ {nf.format(totalCount)}Support (outside selected lines): import React, { useCallback, useMemo, useState } from "react";
const nf = useMemo(() => new Intl.NumberFormat(), []);🤖 Prompt for AI Agents |
||
| ), | ||
|
|
@@ -343,8 +364,8 @@ export const KeysList = ({ | |
| <Empty.Icon className="w-auto" /> | ||
| <Empty.Title>No API Keys Found</Empty.Title> | ||
| <Empty.Description className="text-left"> | ||
| There are no API keys associated with this service yet. Create your first API key to | ||
| get started. | ||
| There are no API keys associated with this service yet. Create | ||
| your first API key to get started. | ||
| </Empty.Description> | ||
| <Empty.Actions className="mt-4 justify-start"> | ||
| <a | ||
|
|
@@ -374,7 +395,7 @@ export const KeysList = ({ | |
| className={cn( | ||
| "text-xs align-middle whitespace-nowrap pr-4", | ||
| idx === 0 ? "pl-[18px]" : "", | ||
| column.key === "key" ? "py-[6px]" : "py-1", | ||
| column.key === "key" ? "py-[6px]" : "py-1" | ||
| )} | ||
| style={{ height: `${rowHeight}px` }} | ||
| > | ||
|
|
@@ -384,9 +405,17 @@ export const KeysList = ({ | |
| {column.key === "last_used" && <LastUsedColumnSkeleton />} | ||
| {column.key === "status" && <StatusColumnSkeleton />} | ||
| {column.key === "action" && <ActionColumnSkeleton />} | ||
| {!["select", "key", "value", "usage", "last_used", "status", "action"].includes( | ||
| column.key, | ||
| ) && <div className="h-4 w-full bg-grayA-3 rounded animate-pulse" />} | ||
| {![ | ||
| "select", | ||
| "key", | ||
| "value", | ||
| "usage", | ||
| "last_used", | ||
| "status", | ||
| "action", | ||
| ].includes(column.key) && ( | ||
| <div className="h-4 w-full bg-grayA-3 rounded animate-pulse" /> | ||
| )} | ||
| </td> | ||
| )) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |||||||||||||||||||||||
| import { useSort } from "@/components/logs/hooks/use-sort"; | ||||||||||||||||||||||||
| import { VirtualTable } from "@/components/virtual-table/index"; | ||||||||||||||||||||||||
| import type { Column } from "@/components/virtual-table/types"; | ||||||||||||||||||||||||
| import { formatNumber } from "@/lib/fmt"; | ||||||||||||||||||||||||
| import { formatNumber, formatNumberFull } from "@/lib/fmt"; | ||||||||||||||||||||||||
| import { cn } from "@/lib/utils"; | ||||||||||||||||||||||||
| import type { RatelimitOverviewLog } from "@unkey/clickhouse/src/ratelimits"; | ||||||||||||||||||||||||
| import { Ban, BookBookmark } from "@unkey/icons"; | ||||||||||||||||||||||||
|
|
@@ -218,8 +218,9 @@ export const RatelimitOverviewLogsTable = ({ | |||||||||||||||||||||||
| hide: isLoading, | ||||||||||||||||||||||||
| countInfoText: ( | ||||||||||||||||||||||||
| <div className="flex gap-2"> | ||||||||||||||||||||||||
| <span>Showing</span> <span className="text-accent-12">{historicalLogs.length}</span> | ||||||||||||||||||||||||
| <span>of {totalCount}</span> | ||||||||||||||||||||||||
| <span>Showing</span>{" "} | ||||||||||||||||||||||||
| <span className="text-accent-12">{formatNumberFull(historicalLogs.length)}</span> | ||||||||||||||||||||||||
| <span>of {formatNumberFull(totalCount)}</span> | ||||||||||||||||||||||||
| <span>rate limit identifiers</span> | ||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||
|
Comment on lines
+221
to
225
Contributor
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. 🛠️ Refactor suggestion Use shared formatter for footer counts You already import - <span>Showing</span>{" "}
- <span className="text-accent-12">
- {new Intl.NumberFormat().format(historicalLogs.length)}
- </span>
- <span>of {new Intl.NumberFormat().format(totalCount)}</span>
+ <span>Showing</span> <span className="text-accent-12">{formatNumber(historicalLogs.length)}</span>
+ <span>of {formatNumber(totalCount)}</span>📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |||||||||||||||||||||||||||
| import { safeParseJson } from "@/app/(app)/logs/utils"; | ||||||||||||||||||||||||||||
| import { VirtualTable } from "@/components/virtual-table/index"; | ||||||||||||||||||||||||||||
| import type { Column } from "@/components/virtual-table/types"; | ||||||||||||||||||||||||||||
| import { formatNumberFull } from "@/lib/fmt"; | ||||||||||||||||||||||||||||
|
Contributor
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. Don't force en-US; honor user locale and cache formatter Import looks good, but current formatNumberFull uses Intl.NumberFormat("en-US"), which conflicts with the PR goal of “locale-appropriate” formatting and allocates a formatter per call. Switch the helper to default-locale (or param) and cache per locale. Support (outside selected lines, apps/dashboard/lib/fmt.ts): -export function formatNumberFull(n: number): string {
- return Intl.NumberFormat("en-US").format(n);
-}
+const nfCache = new Map<string | undefined, Intl.NumberFormat>();
+function getNF(locale?: string) {
+ if (!nfCache.has(locale)) nfCache.set(locale, new Intl.NumberFormat(locale));
+ return nfCache.get(locale)!;
+}
+export function formatNumberFull(n: number, locale?: string): string {
+ return getNF(locale).format(n);
+}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||
| import { cn } from "@/lib/utils"; | ||||||||||||||||||||||||||||
| import type { RatelimitLog } from "@unkey/clickhouse/src/ratelimits"; | ||||||||||||||||||||||||||||
| import { BookBookmark } from "@unkey/icons"; | ||||||||||||||||||||||||||||
|
|
@@ -218,9 +219,10 @@ export const RatelimitLogsTable = () => { | |||||||||||||||||||||||||||
| hide: isLoading, | ||||||||||||||||||||||||||||
| countInfoText: ( | ||||||||||||||||||||||||||||
| <div className="flex gap-2"> | ||||||||||||||||||||||||||||
| <span>Showing</span> <span className="text-accent-12">{historicalLogs.length}</span> | ||||||||||||||||||||||||||||
| <span>Showing</span>{" "} | ||||||||||||||||||||||||||||
| <span className="text-accent-12">{formatNumberFull(historicalLogs.length)}</span> | ||||||||||||||||||||||||||||
| <span>of</span> | ||||||||||||||||||||||||||||
| {totalCount} | ||||||||||||||||||||||||||||
| {formatNumberFull(totalCount)} | ||||||||||||||||||||||||||||
| <span>ratelimit requests</span> | ||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||
|
Comment on lines
+224
to
229
Contributor
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. 🛠️ Refactor suggestion Reuse a memoized number formatter Apply the shared formatter here too. - <span className="text-accent-12">
- {new Intl.NumberFormat().format(historicalLogs.length)}
- </span>
+ <span className="text-accent-12">{nf.format(historicalLogs.length)}</span>
...
- {new Intl.NumberFormat().format(totalCount)}
+ {nf.format(totalCount)}Support (outside selected lines): import { useMemo } from "react";
const nf = useMemo(() => new Intl.NumberFormat(), []);🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.