Skip to content
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

feat: static settings page #570

Merged
merged 22 commits into from
Jan 13, 2025
Merged

feat: static settings page #570

merged 22 commits into from
Jan 13, 2025

Conversation

lily-de
Copy link
Collaborator

@lily-de lily-de commented Jan 10, 2025

PR Summary: Implement New Static Settings Page

Author's note: this change is feature-flag gated by npm environment and will only appear if you're running the electron app (npm start)

This pull request introduces a new static settings page to the Goose project, providing users with an interface to manage application preferences and configurations.

Screenshot 2025-01-10 at 11 21 00 AM Screenshot 2025-01-10 at 11 20 51 AM
Screen.Recording.2025-01-10.at.11.18.07.AM.mov

How It Works

  • Page Structure:

    • The settings page is implemented as a React component located in ui/desktop/src/components/settings/Settings.tsx.
    • Accessible through the /settings route, added to the application's routing configuration.
    • The page dynamically loads and manages settings through React state.
  • Core Functionality:

    • Sectioned Layout: Settings are grouped into organized sections with headers for better navigation, implemented using SectionHeader.tsx.
    • Editable Items: Each setting is represented by SettingItem.tsx, which allows toggling options or updating values.
    • Key Management: API keys or similar fields are managed with the KeyItem.tsx component, supporting edits.
    • Dialogs: @radix-ui/react-dialog is used to display modals for advanced settings or confirmations.
    • Adding Models or Extensions: Users can add a new model or extension by typing strings into a dialog. Note: This is a placeholder implementation, and the final version may require validation, additional metadata, or backend integration to reflect real-world usage.
    • Reseting to default settings: Can reset to some hard-coded default state.
  • State Persistence:

    • The page uses localStorage to save and retrieve user preferences, ensuring settings persist across sessions (persists with cmd+N or if you close the electron app and re-run npm start).
    • Example:
      • User settings are saved with localStorage.setItem('user_settings', JSON.stringify(settings)).
      • Retrieved during initialization using localStorage.getItem('user_settings').
    • Limitations: This implementation does not sync settings across devices or browsers and relies entirely on local storage. Also will force all session windows to the same settings (e.g. open 2nd window -> change settings -> window 1 will now have those settings too).

Missing or Incomplete Functionality

  • Backend Integration: Currently, settings are stored only in localStorage. A backend solution is required for cross-device synchronization or user account integration.
  • Deletion and Editing of Models and Extensions: Currently you can only add.
  • Validation: Limited validation exists for certain fields like API keys, which may require further improvements for production use.
  • Advanced Features: Import/export settings.

Key Code Changes

  • Routing:
    • Added the /settings route to the app's routing configuration.
  • Components:
    • Introduced new components:
      • Settings: Main page logic and rendering.
      • SettingItem: UI for individual settings.
      • KeyItem: Specialized input for key-based settings.
      • SectionHeader: Organizational headers for grouped settings.
  • Dependencies:
    • Integrated @radix-ui/react-dialog for modal dialogs.

Checklist

  • Implemented static settings page and core functionality.
  • Verified settings persist across sessions using localStorage.
  • Backend persistence is not implemented.
  • Validation for fields requires enhancement.

@lily-de lily-de changed the base branch from main to v1.0 January 10, 2025 01:05
@lily-de lily-de changed the title feature: static settings page feat: static settings page Jan 10, 2025
@lily-de lily-de marked this pull request as ready for review January 10, 2025 01:17

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 6 out of 20 changed files in this pull request and generated no comments.

Files not reviewed (14)
  • ui/desktop/package.json: Language not supported
  • ui/desktop/src/ChatWindow.tsx: Evaluated as low risk
  • ui/desktop/src/components/MoreMenu.tsx: Evaluated as low risk
  • ui/desktop/src/utils/cn.ts: Evaluated as low risk
  • ui/desktop/src/components/settings/types.ts: Evaluated as low risk
  • ui/desktop/src/components/settings/Settings.tsx: Evaluated as low risk
  • ui/desktop/src/components/ui/Tooltip.tsx: Evaluated as low risk
  • ui/desktop/src/components/settings/SettingsSection.tsx: Evaluated as low risk
  • ui/desktop/src/components/settings/SectionHeader.tsx: Evaluated as low risk
  • ui/desktop/src/components/settings/ToggleableItem.tsx: Evaluated as low risk
  • ui/desktop/src/components/settings/SettingItem.tsx: Evaluated as low risk
  • ui/desktop/src/components/settings/modals/RevealKeysDialog.tsx: Evaluated as low risk
  • ui/desktop/src/components/settings/modals/AddModelDialog.tsx: Evaluated as low risk
  • ui/desktop/src/components/settings/modals/BaseDialog.tsx: Evaluated as low risk
Comments suppressed due to low confidence (1)

ui/desktop/src/components/ui/toast.tsx:11

  • The class name animate-in should be animate-in-fade-in-slide-in-from-bottom-5.
animate-in fade-in slide-in-from-bottom-5
@Kvadratni
Copy link
Collaborator

@alexhancock alexhancock self-requested a review January 10, 2025 16:38
Copy link
Collaborator

@alexhancock alexhancock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great we should merge it.

Before we do, is there a way we can "feature flag" it so it only shows up when running from source code, and not in the deployed app sent to MSC? I just wouldn't want to ship it to real users before the settings actually take effect.

@lily-de
Copy link
Collaborator Author

lily-de commented Jan 10, 2025

This is great we should merge it.

Before we do, is there a way we can "feature flag" it so it only shows up when running from source code, and not in the deployed app sent to MSC? I just wouldn't want to ship it to real users before the settings actually take effect.

yeah great idea -- was also thinking maybe the settings page stuff can be merged to some intermediary branch before merging to v1.0 but might end up messier

Copy link

Desktop App for this PR

The following build is available for testing:

The app is signed and notarized for macOS. After downloading, unzip the file and drag the Goose.app to your Applications folder.

This link is provided by nightly.link and will work even if you're not logged into GitHub.

Copy link

Desktop App for this PR

The following build is available for testing:

The app is signed and notarized for macOS. After downloading, unzip the file and drag the Goose.app to your Applications folder.

This link is provided by nightly.link and will work even if you're not logged into GitHub.

Copy link

Desktop App for this PR

The following build is available for testing:

The app is signed and notarized for macOS. After downloading, unzip the file and drag the Goose.app to your Applications folder.

This link is provided by nightly.link and will work even if you're not logged into GitHub.

@lily-de lily-de force-pushed the ldelalande/settings-set-up branch from b90ecda to cf7bdc4 Compare January 13, 2025 18:57
Copy link

Desktop App for this PR

The following build is available for testing:

The app is signed and notarized for macOS. After downloading, unzip the file and drag the Goose.app to your Applications folder.

This link is provided by nightly.link and will work even if you're not logged into GitHub.

- Fix import conflicts from Settings component move
- Update electron API type definitions
- Standardize createChatWindow calls with empty string param
- Remove duplicate imports
@lily-de lily-de force-pushed the ldelalande/settings-set-up branch from da6a5b8 to 6276f87 Compare January 13, 2025 19:50
Copy link

Desktop App for this PR

The following build is available for testing:

The app is signed and notarized for macOS. After downloading, unzip the file and drag the Goose.app to your Applications folder.

This link is provided by nightly.link and will work even if you're not logged into GitHub.

Copy link

Desktop App for this PR

The following build is available for testing:

The app is signed and notarized for macOS. After downloading, unzip the file and drag the Goose.app to your Applications folder.

This link is provided by nightly.link and will work even if you're not logged into GitHub.

@lily-de lily-de merged commit bf65a7d into v1.0 Jan 13, 2025
6 checks passed
@lily-de lily-de deleted the ldelalande/settings-set-up branch January 13, 2025 21:04
michaelneale added a commit that referenced this pull request Jan 14, 2025
* v1.0:
  feat: static settings page (#570)
  styles: adding arcade styles and cash sans (#581)
  feat: quick spinner while loading tokenizers (#573)
  Add timeout middleware for clients (#572)
  chore: remove unused old setup for CLI  (#574)
  feat: env and secrets configuration for mcp server (#565)
  Add Databricks moderation (#540)
  feat: add pagination support for tools/list and resources/list (#566)
  Add resource capabilties to MCP servers that use it (#576)
  Add goose versions to the UI (#526)
  fix: Set stdin to null in shell/bash tools (#568)
  Add base moderation to Providers (#513)
  feat: set process_group(0) on stdio systems to avoid ctrl-c handling (#567)
  feat: read only active resources in the agent loop (#560)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants