Skip to content

Support themes #200

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

Merged
merged 6 commits into from
Jul 28, 2025
Merged

Support themes #200

merged 6 commits into from
Jul 28, 2025

Conversation

andreaskienle
Copy link
Contributor

@andreaskienle andreaskienle commented Jul 23, 2025

Implements openmcp-project/backlog#127

When integrated into Hyperspace Portal, we automatically adopt its theme:

2025-07-23_16-02-06.mp4

When running stand-alone, we detect the system (OS) settings to toggle between our default dark and light theme. Users can also specify a theme using the ⁠sap-theme parameter, which takes precedence over system settings:

2025-07-23_16-05-40.mp4

@andreaskienle andreaskienle requested a review from Copilot July 23, 2025 13:02
Copy link
Contributor

@Copilot Copilot AI left a 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 implements theme support for the application by enhancing the theme management system. It replaces the simple dark/light mode switcher with a more comprehensive theme manager that can handle themes specified via URL parameters while maintaining fallback to system preferences.

  • Replaced the basic DarkModeSystemSwitcher with a new ThemeManager component that supports URL-based theme selection
  • Improved the authentication flow to preserve theme parameters during redirects
  • Refactored dark mode detection using React's useSyncExternalStore for better reactivity

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/components/ThemeManager/ThemeManager.tsx New theme manager component with URL parameter support and fallback logic
src/components/ThemeManager/ThemeManager.spec.ts Unit tests for the theme resolution logic
src/hooks/useIsDarkModePreferred.ts New hook using useSyncExternalStore for reactive dark mode detection
src/main.tsx Updated to use the new ThemeManager component
src/spaces/onboarding/auth/AuthContextOnboarding.tsx Enhanced login redirect to preserve query parameters and hash fragments
src/components/Yaml/YamlViewer.tsx Updated to use the new dark mode detection hook
src/lib/useThemeMode.ts Removed old theme mode utility
src/components/Core/DarkModeSystemSwitcher.tsx Removed old dark mode switcher component

@andreaskienle andreaskienle marked this pull request as ready for review July 23, 2025 14:12
@enrico-kaack-comp enrico-kaack-comp self-requested a review July 23, 2025 15:16
Copy link
Contributor

@enrico-kaack-comp enrico-kaack-comp left a comment

Choose a reason for hiding this comment

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

When navigating into an mcp (in standalone mode with url parameter, so i guess also in the hyperspace), the url parameter is removed from the url on the auth mcp auth callback and therefore the theme is switched back again

@andreaskienle
Copy link
Contributor Author

When navigating into an mcp (in standalone mode with url parameter, so i guess also in the hyperspace), the url parameter is removed from the url on the auth mcp auth callback and therefore the theme is switched back again

Good catch! I had only checked the first redirect during sign-in and overlooked the redirect when logging into an MCP.

There's a third redirect (after the token expires). I've also added a proper redirect to the previous page, preserving the theme in this case (previously the user was taken to the login screen).

@andreaskienle andreaskienle merged commit ee7ad6e into main Jul 28, 2025
5 checks passed
@andreaskienle andreaskienle deleted the feat/theming branch July 28, 2025 13:47
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.

2 participants