-
-
Notifications
You must be signed in to change notification settings - Fork 377
fix(my-usage): UX improvements for quota and statistics cards #794
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
2654f10
b8dc8ca
91a571e
fe1fe4e
1576a25
3007012
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,8 +2,65 @@ | |
|
|
||
| import { Layers, ShieldCheck } from "lucide-react"; | ||
| import { useTranslations } from "next-intl"; | ||
| import { Badge } from "@/components/ui/badge"; | ||
| import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; | ||
| import { cn } from "@/lib/utils"; | ||
|
|
||
| function abbreviateModel(name: string): string { | ||
| const parts = name.split("-").filter(Boolean); | ||
|
|
||
| if (parts.length === 1) { | ||
| return parts[0].length <= 4 ? parts[0].toUpperCase() : parts[0].slice(0, 2).toUpperCase(); | ||
| } | ||
|
|
||
| const letterParts: string[] = []; | ||
| let versionMixed = ""; | ||
| const versionNums: string[] = []; | ||
|
|
||
| for (const part of parts) { | ||
| if (/^\d{8,}$/.test(part)) continue; | ||
| if (/^[a-zA-Z]+$/.test(part)) { | ||
| letterParts.push(part); | ||
| } else if (/^\d+\.\d+$/.test(part)) { | ||
| versionMixed = part; | ||
| } else if (/^\d+[a-zA-Z]/.test(part)) { | ||
| versionMixed = part; | ||
| } else if (/^\d+$/.test(part)) { | ||
| versionNums.push(part); | ||
| } else { | ||
| letterParts.push(part); | ||
| } | ||
| } | ||
|
|
||
| const prefix = letterParts | ||
| .slice(0, 3) | ||
| .map((w) => w[0].toUpperCase()) | ||
| .join(""); | ||
|
|
||
| let version = ""; | ||
| if (versionMixed) { | ||
| version = versionMixed; | ||
| } else if (versionNums.length > 0) { | ||
| version = versionNums.slice(0, 2).join("."); | ||
| } | ||
|
|
||
| if (version && prefix) { | ||
| return `${prefix}-${version}`; | ||
| } | ||
| return prefix || name.toUpperCase().substring(0, 3); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| function abbreviateClient(name: string): string { | ||
| const parts = name.split(/[-\s]+/).filter(Boolean); | ||
| if (parts.length === 1) { | ||
| return name.slice(0, 2).toUpperCase(); | ||
| } | ||
| return parts | ||
| .slice(0, 3) | ||
| .map((w) => w[0].toUpperCase()) | ||
|
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. |
||
| .join(""); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| interface ProviderGroupInfoProps { | ||
| keyProviderGroup: string | null; | ||
| userProviderGroup: string | null; | ||
|
|
@@ -26,10 +83,8 @@ export function ProviderGroupInfo({ | |
| const userDisplay = userProviderGroup ?? tGroup("allProviders"); | ||
| const inherited = !keyProviderGroup && !!userProviderGroup; | ||
|
|
||
| const modelsDisplay = | ||
| userAllowedModels.length > 0 ? userAllowedModels.join(", ") : tRestrictions("noRestrictions"); | ||
| const clientsDisplay = | ||
| userAllowedClients.length > 0 ? userAllowedClients.join(", ") : tRestrictions("noRestrictions"); | ||
| const hasModels = userAllowedModels.length > 0; | ||
| const hasClients = userAllowedClients.length > 0; | ||
|
|
||
| return ( | ||
| <div | ||
|
|
@@ -45,16 +100,20 @@ export function ProviderGroupInfo({ | |
| <span>{tGroup("title")}</span> | ||
| </div> | ||
| <div className="space-y-1"> | ||
| <div className="flex items-baseline gap-1.5"> | ||
| <span className="text-xs text-muted-foreground">{tGroup("keyGroup")}:</span> | ||
| <span className="text-sm font-semibold text-foreground">{keyDisplay}</span> | ||
| <div className="flex flex-wrap items-center gap-1.5"> | ||
| <span className="shrink-0 text-xs text-muted-foreground">{tGroup("keyGroup")}:</span> | ||
| <Badge variant="outline" className="cursor-default text-xs"> | ||
| {keyDisplay} | ||
| </Badge> | ||
| {inherited && ( | ||
| <span className="text-xs text-muted-foreground">({tGroup("inheritedFromUser")})</span> | ||
| )} | ||
| </div> | ||
| <div className="flex items-baseline gap-1.5"> | ||
| <span className="text-xs text-muted-foreground">{tGroup("userGroup")}:</span> | ||
| <span className="text-sm font-semibold text-foreground">{userDisplay}</span> | ||
| <div className="flex flex-wrap items-center gap-1.5"> | ||
| <span className="shrink-0 text-xs text-muted-foreground">{tGroup("userGroup")}:</span> | ||
| <Badge variant="outline" className="cursor-default text-xs"> | ||
| {userDisplay} | ||
| </Badge> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
@@ -66,13 +125,47 @@ export function ProviderGroupInfo({ | |
| <span>{tRestrictions("title")}</span> | ||
| </div> | ||
| <div className="space-y-1"> | ||
| <div className="flex items-baseline gap-1.5"> | ||
| <span className="text-xs text-muted-foreground">{tRestrictions("models")}:</span> | ||
| <span className="text-sm font-semibold text-foreground">{modelsDisplay}</span> | ||
| <div className="flex flex-wrap items-center gap-1.5"> | ||
| <span className="shrink-0 text-xs text-muted-foreground"> | ||
| {tRestrictions("models")}: | ||
| </span> | ||
| {hasModels ? ( | ||
| userAllowedModels.map((name) => ( | ||
| <Tooltip key={name}> | ||
| <TooltipTrigger asChild> | ||
| <Badge variant="outline" className="cursor-default font-mono text-xs"> | ||
| {abbreviateModel(name)} | ||
| </Badge> | ||
| </TooltipTrigger> | ||
| <TooltipContent>{name}</TooltipContent> | ||
| </Tooltip> | ||
| )) | ||
|
Comment on lines
+133
to
+142
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. Wrapping Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! Prompt To Fix With AIThis is a comment left during a code review.
Path: src/app/[locale]/my-usage/_components/provider-group-info.tsx
Line: 133:144
Comment:
Wrapping `TooltipTrigger` with an extra `<span>` is unnecessary. The `asChild` prop makes `TooltipTrigger` pass its props to its child, so you can directly wrap `Badge`.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise. |
||
| ) : ( | ||
| <span className="text-sm font-semibold text-foreground"> | ||
| {tRestrictions("noRestrictions")} | ||
| </span> | ||
| )} | ||
| </div> | ||
| <div className="flex items-baseline gap-1.5"> | ||
| <span className="text-xs text-muted-foreground">{tRestrictions("clients")}:</span> | ||
| <span className="text-sm font-semibold text-foreground">{clientsDisplay}</span> | ||
| <div className="flex flex-wrap items-center gap-1.5"> | ||
| <span className="shrink-0 text-xs text-muted-foreground"> | ||
| {tRestrictions("clients")}: | ||
| </span> | ||
| {hasClients ? ( | ||
| userAllowedClients.map((name) => ( | ||
| <Tooltip key={name}> | ||
| <TooltipTrigger asChild> | ||
| <Badge variant="outline" className="cursor-default font-mono text-xs"> | ||
| {abbreviateClient(name)} | ||
| </Badge> | ||
| </TooltipTrigger> | ||
| <TooltipContent>{name}</TooltipContent> | ||
| </Tooltip> | ||
| )) | ||
|
Comment on lines
+154
to
+163
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. Same issue - the extra Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! Prompt To Fix With AIThis is a comment left during a code review.
Path: src/app/[locale]/my-usage/_components/provider-group-info.tsx
Line: 156:167
Comment:
Same issue - the extra `<span>` wrapper is unnecessary when using `asChild` prop on `TooltipTrigger`.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise. |
||
| ) : ( | ||
| <span className="text-sm font-semibold text-foreground"> | ||
| {tRestrictions("noRestrictions")} | ||
| </span> | ||
| )} | ||
| </div> | ||
| </div> | ||
| </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.
The
mapoperation will throw an error ifletterPartscontains an empty string (e.g., if the model name has double hyphens), asw[0]would beundefined. It's safer to use optional chaining or filter out empty strings.