-
Notifications
You must be signed in to change notification settings - Fork 0
refactor(settings): reorganize settings dialog into tabs and improve API key encryption #299
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
Conversation
|
🏷️ This PR has been automatically assigned to milestone 0.6.0-alpha based on the version in |
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.
Pull Request Overview
This PR reorganizes the settings dialog into a tabbed interface and enhances API key encryption security. The changes improve user experience by splitting settings into logical tabs (General, Providers, Canvas Assistant, and provider-specific tabs) while migrating to OS-level secure storage for API keys.
- Implemented tabbed settings dialog with dedicated pages for different configuration areas
- Enhanced API key encryption using OS secure store (DPAPI on Windows, file-based on macOS/Linux) with automatic migration
- Added SmartHopper Assistant configuration with dedicated settings model
Reviewed Changes
Copilot reviewed 36 out of 38 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/SmartHopper.Menu/Dialogs/SettingsDialog.cs | Complete refactor to tabbed interface with separate settings pages |
| src/SmartHopper.Infrastructure/Settings/SmartHopperSettings.cs | Enhanced encryption system with OS secure store and migration logic |
| src/SmartHopper.Menu/Dialogs/SettingsTabs/*.cs | New tab pages for different settings categories |
| Provider classes | Code style improvements and nullable annotations |
| src/SmartHopper.Infrastructure/Settings/SmartHopperAssistantSettings.cs | New settings model for assistant configuration |
Comments suppressed due to low confidence (3)
src/SmartHopper.Infrastructure/Settings/SmartHopperSettings.cs:504
- The conditional compilation directive 'WINDOWS' may not be defined. Consider using 'NET7_0_OR_GREATER && WINDOWS' or the proper target framework conditional compilation symbols.
#if WINDOWS
src/SmartHopper.Infrastructure/Settings/SmartHopperSettings.cs:557
- Same conditional compilation issue - 'WINDOWS' may not be defined. Use proper target framework conditionals like 'NET7_0_OR_GREATER && WINDOWS'.
#if WINDOWS
src/SmartHopper.Core.Grasshopper/AITools/web_generic_page_read.cs:30
- Class name 'web_generic_page_read' does not follow C# naming conventions. Should be PascalCase, e.g., 'WebGenericPageRead'.
public class web_generic_page_read : IAIToolProvider
src/SmartHopper.Infrastructure/Settings/SmartHopperAssistantSettings.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ttings.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…s-toolkit/SmartHopper into feature/0.6.0-settings
Description
Reorganized the settings dialog into a tabbed interface to improve navigation and clarity, splitting SmartHopper Assistant and Trusted Providers into separate tabs. Enhanced security by migrating API key encryption to use the OS secure store (DPAPI on Windows, file-based on macOS) with automatic migration of existing keys.
Breaking Changes
None.
Testing Done
SmartHopperSettingsencryption, decryption, and migration logic.Checklist