-
Notifications
You must be signed in to change notification settings - Fork 605
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 all 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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,7 +2,7 @@ | |||||
| import { VirtualTable } from "@/components/virtual-table/index"; | ||||||
| import type { Column } from "@/components/virtual-table/types"; | ||||||
| import { useIsMobile } from "@/hooks/use-mobile"; | ||||||
| import type { Deployment, Environment } from "@/lib/collections"; | ||||||
|
|
||||||
|
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. Missing type imports cause compilation error: View Details📝 Patch Detailsdiff --git a/apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx b/apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx
index afe222b25..9876d5843 100644
--- a/apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx
+++ b/apps/dashboard/app/(app)/projects/[projectId]/deployments/components/table/deployments-list.tsx
@@ -2,6 +2,7 @@
import { VirtualTable } from "@/components/virtual-table/index";
import type { Column } from "@/components/virtual-table/types";
import { useIsMobile } from "@/hooks/use-mobile";
+import type { Deployment, Environment } from "@/lib/collections";
import { shortenId } from "@/lib/shorten-id";
import { BookBookmark, Cloud, CodeBranch, Cube } from "@unkey/icons";
AnalysisMissing type imports cause TypeScript compilation errors in deployments-list.tsxWhat fails: TypeScript compilation fails with "Cannot find name 'Deployment'" and "Cannot find name 'Environment'" errors in How to reproduce: cd apps/dashboard
pnpm exec tsc --noEmit --isolatedModules --jsx react-jsx app/\(app\)/projects/\[projectId\]/deployments/components/table/deployments-list.tsxResult: TypeScript errors: Expected: Code should compile without type errors since the types are defined and exported from Fix: Added |
||||||
| import { shortenId } from "@/lib/shorten-id"; | ||||||
| import { BookBookmark, Cloud, CodeBranch, Cube } from "@unkey/icons"; | ||||||
| import { Button, Empty, TimestampInfo } from "@unkey/ui"; | ||||||
|
|
@@ -307,7 +307,7 @@ export const DeploymentsList = () => { | |||||
| onRowClick={setSelectedDeployment} | ||||||
| selectedItem={selectedDeployment} | ||||||
| keyExtractor={(deployment) => deployment.id} | ||||||
|
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. Fix keyExtractor to use row.deployment.id VirtualTable rows appear to be shaped as { deployment, environment }. The current extractor uses the row itself as a Deployment, which likely produces undefined/duplicate keys. - keyExtractor={(deployment) => deployment.id}
+ keyExtractor={(row) => row.deployment.id}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| rowClassName={(deployment) => getRowClassName(deployment, selectedDeployment?.deployment.id)} | ||||||
|
|
||||||
| emptyState={ | ||||||
| <div className="w-full flex justify-center items-center h-full"> | ||||||
| <Empty className="w-[400px] flex items-start"> | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,4 @@ | ||
| import { collection } from "@/lib/collections"; | ||
| import { ilike, useLiveQuery } from "@tanstack/react-db"; | ||
|
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. Missing imports cause compilation error: View Details📝 Patch Detailsdiff --git a/apps/dashboard/app/(app)/projects/_components/list/index.tsx b/apps/dashboard/app/(app)/projects/_components/list/index.tsx
index b0d569ece..e2d6fe4ea 100644
--- a/apps/dashboard/app/(app)/projects/_components/list/index.tsx
+++ b/apps/dashboard/app/(app)/projects/_components/list/index.tsx
@@ -1,6 +1,8 @@
import { BookBookmark, Dots } from "@unkey/icons";
import { Button, Empty } from "@unkey/ui";
+import { ilike, useLiveQuery } from "@tanstack/react-db";
+import { collection } from "@/lib/collections";
import { useProjectsFilters } from "../hooks/use-projects-filters";
import { ProjectActions } from "./project-actions";
import { ProjectCard } from "./projects-card";
@@ -101,6 +103,6 @@ export const ProjectsList = () => {
/>
))}
</div>
-
+ </div>
);
};
AnalysisMissing imports cause TypeScript compilation errors in ProjectsList componentWhat fails: ProjectsList component in How to reproduce: cd apps/dashboard && pnpm run typecheckResult: TypeScript compiler reports: Expected: Clean compilation with proper imports from |
||
|
|
||
|
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. Critical syntax errors: Missing required imports for View Details📝 Patch Detailsdiff --git a/apps/dashboard/app/(app)/projects/_components/list/index.tsx b/apps/dashboard/app/(app)/projects/_components/list/index.tsx
index b0d569ece..e2d6fe4ea 100644
--- a/apps/dashboard/app/(app)/projects/_components/list/index.tsx
+++ b/apps/dashboard/app/(app)/projects/_components/list/index.tsx
@@ -1,6 +1,8 @@
import { BookBookmark, Dots } from "@unkey/icons";
import { Button, Empty } from "@unkey/ui";
+import { ilike, useLiveQuery } from "@tanstack/react-db";
+import { collection } from "@/lib/collections";
import { useProjectsFilters } from "../hooks/use-projects-filters";
import { ProjectActions } from "./project-actions";
import { ProjectCard } from "./projects-card";
@@ -101,6 +103,6 @@ export const ProjectsList = () => {
/>
))}
</div>
-
+ </div>
);
};
AnalysisCritical syntax errors in ProjectsList component: Missing imports and unclosed JSX tagWhat fails: File How to reproduce: cd apps/dashboard && pnpm run typecheckResult: TypeScript compiler fails with errors:
Expected: File should compile without TypeScript errors Fix applied:
|
||
| import { BookBookmark, Dots } from "@unkey/icons"; | ||
| import { Button, Empty } from "@unkey/ui"; | ||
| import { useProjectsFilters } from "../hooks/use-projects-filters"; | ||
|
|
@@ -102,6 +101,6 @@ export const ProjectsList = () => { | |
| /> | ||
| ))} | ||
| </div> | ||
| </div> | ||
|
|
||
| ); | ||
| }; | ||
| 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"; | ||||||||||||||||||||||||||||
|
|
@@ -220,9 +221,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> | ||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
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.
🧹 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.
Also applies to: 179-184
🤖 Prompt for AI Agents