PR #3: Environment Tab Integration#71
Conversation
be38954 to
e6c46ff
Compare
Apply shared KeyValueEditor component to both Settings > Environment and Terminal Details > Environment tabs for consistent UI and behavior. Changes: - Settings Environment tab: Replace inline form with KeyValueEditor - Terminal Details Environment tab: Replace inline editor with KeyValueEditor - Both tabs now use identical shared component with full-width inputs - Consistent styling with Form .formStyle(.grouped) - Proper state synchronization between KeyValueItem and EnvironmentVariable Addresses feedback #4, #7, #8, #9, #11 from implementation plan Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Split KeyValueEditor into focused components (KeyValueList + KeyValueAddForm) for better UX with separate sections for viewing and adding items. Changes: - Split KeyValueEditor.swift into 3 components: * KeyValueList - display existing items with delete * KeyValueAddForm - add new items with validation * KeyValueEditor - legacy wrapper (deprecated) - SettingsEnvironmentView improvements: * Use split components in separate sections * Add proper error handling with user-visible alerts * Fix encryption key status race condition * Remove nested Form wrapper - CardEditorEnvironmentTab improvements: * Use split components in separate sections * Add secret visibility toggle for inherited globals * Add proper error handling with alerts * Store secret values in memory for display - Add localized section headers: * "Global Environment Variables" * "Add Environment Variable" (Settings) * "Add Environment Variable" (Terminal Details) Fixes: - Secrets not being added (silent error swallowing with try?) - Encryption key status not updating after operations - Secret values not viewable in Terminal Details inherited list Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
e6c46ff to
d0e69bf
Compare
Code ReviewI found 1 issue that requires attention: ❌ Hardcoded string should be localizedFiles:
Issue: CLAUDE.md Reference: Suggested fix:
extension Strings.Alert {
static let unknownError = String(localized: "alert.error.unknown")
}
Text(errorMessage ?? Strings.Alert.unknownError)Context: Summary: All other changes look good. No bugs found. CLAUDE.md compliance verified except for the localization issue above. |
Code ReviewFound 1 issue requiring attention: Hardcoded UI strings should be localizedLocations:
The string Why this matters:
This project supports 40 languages and uses a comprehensive localization system. User-facing strings should use the Suggested fix: // 1. Add to Localizable.strings:
"alert.unknown.error" = "An unknown error occurred";
// 2. Add to Strings.swift:
extension Strings.Alert {
static let unknownError = NSLocalizedString("alert.unknown.error", comment: "Generic error message")
}
// 3. Replace both occurrences:
Text(errorMessage ?? Strings.Alert.unknownError)Context: |
Summary
Changes
KeyValueEditor.swift:
SettingsEnvironmentView.swift:
try?)CardEditorEnvironmentTab.swift:
Localization:
🤖 Generated with Claude Code