mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-02-03 21:03:08 +00:00
fix: address Gemini code review suggestions
Fixed two critical code quality issues identified in code review: 1. Auth Method Selector - Tailwind CSS Dynamic Classes: - Replaced dynamic class name concatenation with static class mapping - Tailwind JIT compiler requires complete class names at build time - Added getBadgeClasses() helper function with predefined color mappings - Prevents broken styling from unparseable dynamic classes 2. Claude CLI Detector - Async Terminal Detection: - Moved execSync loop to async/await pattern to prevent UI blocking - Refactored Linux terminal detection to run before Promise constructor - Prevents main process freezing during terminal emulator detection - Improves responsiveness on Linux systems Both fixes improve code reliability and user experience.
This commit is contained in:
@@ -611,7 +611,39 @@ class ClaudeCliDetector {
|
|||||||
}
|
}
|
||||||
send("Opening system terminal for authentication...\n");
|
send("Opening system terminal for authentication...\n");
|
||||||
|
|
||||||
return await new Promise((resolve, reject) => {
|
// Helper function to check if a command exists asynchronously
|
||||||
|
const commandExists = (cmd) => {
|
||||||
|
return new Promise((resolve) => {
|
||||||
|
require("child_process").exec(
|
||||||
|
`which ${cmd}`,
|
||||||
|
{ timeout: 1000 },
|
||||||
|
(error) => {
|
||||||
|
resolve(!error);
|
||||||
|
}
|
||||||
|
);
|
||||||
|
});
|
||||||
|
};
|
||||||
|
|
||||||
|
// For Linux, find available terminal first (async)
|
||||||
|
let linuxTerminal = null;
|
||||||
|
if (platform !== "win32" && platform !== "darwin") {
|
||||||
|
const terminals = [
|
||||||
|
["gnome-terminal", ["--", claudePath, "setup-token"]],
|
||||||
|
["konsole", ["-e", claudePath, "setup-token"]],
|
||||||
|
["xterm", ["-e", claudePath, "setup-token"]],
|
||||||
|
["x-terminal-emulator", ["-e", `${claudePath} setup-token`]],
|
||||||
|
];
|
||||||
|
|
||||||
|
for (const [term, termArgs] of terminals) {
|
||||||
|
const exists = await commandExists(term);
|
||||||
|
if (exists) {
|
||||||
|
linuxTerminal = { command: term, args: termArgs };
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return new Promise((resolve, reject) => {
|
||||||
// Open command in external terminal since Claude CLI requires TTY
|
// Open command in external terminal since Claude CLI requires TTY
|
||||||
let command, args;
|
let command, args;
|
||||||
|
|
||||||
@@ -629,27 +661,8 @@ class ClaudeCliDetector {
|
|||||||
'tell application "Terminal" to activate',
|
'tell application "Terminal" to activate',
|
||||||
];
|
];
|
||||||
} else {
|
} else {
|
||||||
// Linux: Try common terminal emulators
|
// Linux: Use the terminal we found earlier
|
||||||
const terminals = [
|
if (!linuxTerminal) {
|
||||||
["gnome-terminal", ["--", claudePath, "setup-token"]],
|
|
||||||
["konsole", ["-e", claudePath, "setup-token"]],
|
|
||||||
["xterm", ["-e", claudePath, "setup-token"]],
|
|
||||||
["x-terminal-emulator", ["-e", `${claudePath} setup-token`]],
|
|
||||||
];
|
|
||||||
|
|
||||||
// Try to find an available terminal
|
|
||||||
for (const [term, termArgs] of terminals) {
|
|
||||||
try {
|
|
||||||
execSync(`which ${term}`, { stdio: "ignore" });
|
|
||||||
command = term;
|
|
||||||
args = termArgs;
|
|
||||||
break;
|
|
||||||
} catch {
|
|
||||||
// Terminal not found, try next
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!command) {
|
|
||||||
reject({
|
reject({
|
||||||
success: false,
|
success: false,
|
||||||
error:
|
error:
|
||||||
@@ -658,6 +671,8 @@ class ClaudeCliDetector {
|
|||||||
});
|
});
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
command = linuxTerminal.command;
|
||||||
|
args = linuxTerminal.args;
|
||||||
}
|
}
|
||||||
|
|
||||||
console.log(
|
console.log(
|
||||||
|
|||||||
@@ -14,33 +14,68 @@ interface AuthMethodSelectorProps {
|
|||||||
onSelect: (methodId: string) => void;
|
onSelect: (methodId: string) => void;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Map badge colors to complete Tailwind class names
|
||||||
|
const getBadgeClasses = (badgeColor: string) => {
|
||||||
|
const colorMap: Record<string, { border: string; bg: string; text: string }> = {
|
||||||
|
"brand-500": {
|
||||||
|
border: "hover:border-brand-500/50",
|
||||||
|
bg: "hover:bg-brand-500/5",
|
||||||
|
text: "text-brand-500",
|
||||||
|
},
|
||||||
|
"green-500": {
|
||||||
|
border: "hover:border-green-500/50",
|
||||||
|
bg: "hover:bg-green-500/5",
|
||||||
|
text: "text-green-500",
|
||||||
|
},
|
||||||
|
"blue-500": {
|
||||||
|
border: "hover:border-blue-500/50",
|
||||||
|
bg: "hover:bg-blue-500/5",
|
||||||
|
text: "text-blue-500",
|
||||||
|
},
|
||||||
|
"purple-500": {
|
||||||
|
border: "hover:border-purple-500/50",
|
||||||
|
bg: "hover:bg-purple-500/5",
|
||||||
|
text: "text-purple-500",
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
return colorMap[badgeColor] || {
|
||||||
|
border: "hover:border-brand-500/50",
|
||||||
|
bg: "hover:bg-brand-500/5",
|
||||||
|
text: "text-brand-500",
|
||||||
|
};
|
||||||
|
};
|
||||||
|
|
||||||
export function AuthMethodSelector({
|
export function AuthMethodSelector({
|
||||||
options,
|
options,
|
||||||
onSelect,
|
onSelect,
|
||||||
}: AuthMethodSelectorProps) {
|
}: AuthMethodSelectorProps) {
|
||||||
return (
|
return (
|
||||||
<div className="grid grid-cols-1 md:grid-cols-2 gap-4">
|
<div className="grid grid-cols-1 md:grid-cols-2 gap-4">
|
||||||
{options.map((option) => (
|
{options.map((option) => {
|
||||||
<button
|
const badgeClasses = getBadgeClasses(option.badgeColor);
|
||||||
key={option.id}
|
return (
|
||||||
onClick={() => onSelect(option.id)}
|
<button
|
||||||
className={`p-4 rounded-lg border border-border hover:border-${option.badgeColor}/50 bg-card hover:bg-${option.badgeColor}/5 transition-all text-left`}
|
key={option.id}
|
||||||
data-testid={`select-${option.id}-auth`}
|
onClick={() => onSelect(option.id)}
|
||||||
>
|
className={`p-4 rounded-lg border border-border ${badgeClasses.border} bg-card ${badgeClasses.bg} transition-all text-left`}
|
||||||
<div className="flex items-start gap-3">
|
data-testid={`select-${option.id}-auth`}
|
||||||
{option.icon}
|
>
|
||||||
<div>
|
<div className="flex items-start gap-3">
|
||||||
<p className="font-medium text-foreground">{option.title}</p>
|
{option.icon}
|
||||||
<p className="text-sm text-muted-foreground mt-1">
|
<div>
|
||||||
{option.description}
|
<p className="font-medium text-foreground">{option.title}</p>
|
||||||
</p>
|
<p className="text-sm text-muted-foreground mt-1">
|
||||||
<p className={`text-xs text-${option.badgeColor} mt-2`}>
|
{option.description}
|
||||||
{option.badge}
|
</p>
|
||||||
</p>
|
<p className={`text-xs ${badgeClasses.text} mt-2`}>
|
||||||
|
{option.badge}
|
||||||
|
</p>
|
||||||
|
</div>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</button>
|
||||||
</button>
|
);
|
||||||
))}
|
})}
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user