You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "mistercrunch (via GitHub)" <gi...@apache.org> on 2025/07/16 01:30:31 UTC
[PR] feat: add a theme CRUD page to manage themes [superset]
mistercrunch opened a new pull request, #34182:
URL: https://github.com/apache/superset/pull/34182
NOTE: this was mostly vibe coded (with lots of TLC on my side) with Claude Code, CHECK IT OUT IF YOU HAVEN'T!
## Summary
Introduces a complete Theme CRUD interface for Apache Superset, enabling organizational theme management and enhancing the theme development workflow.
## Key Features
### 🎨 **Theme CRUD Interface**
- Full CRUD operations for themes with export/import support
- Added to Settings → Manage menu under CSS Templates
- Live theme preview with "Apply" button (paint brush icon)
- Bulk operations and standard Superset list view patterns
### 🛠️ **Enhanced Development Workflow**
- **Apply**: Preview themes temporarily without saving
- **Clear Local Settings**: Remove dev overrides to see production state
- **Permission-based UI**: Different features for developers vs users
- **Quick Apply**: Apply themes directly from list view
### 🏗️ **Architecture**
- Extended ThemeController with CRUD integration
- Enhanced ThemeProvider with new React context methods
- Theme priority: Dev overrides → CRUD themes → Default themes
- Follows standard Superset CRUD patterns
## Implementation
**Backend**: Complete REST API with Theme model, commands, DAOs, and proper error handling
**Frontend**: React CRUD interface with TypeScript safety and enhanced theme modal
**Integration**: Seamless integration with existing theme system
## Cleanup
- Removed deprecated `THEME_ALLOW_THEME_EDITOR_BETA` feature flag
- Simplified default theme to empty JSON object
- Fixed TypeScript errors and improved button styling
Provides organizational theme management while maintaining backward compatibility and following established Superset conventions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2226607216
##########
docs/docs/configuration/theming.mdx:
##########
@@ -10,44 +10,92 @@ version: 1
apache-superset>=6.0
:::
-Superset now rides on **Ant Design v5’s token-based theming**.
+Superset now rides on **Ant Design v5's token-based theming**.
Every Antd token works, plus a handful of Superset-specific ones for charts and dashboard chrome.
-## 1 — Create a theme
+## Managing Themes via CRUD Interface
-1. Open the official [Ant Design Theme Editor](https://ant.design/theme-editor)
-2. Design your palette, typography, and component overrides.
-3. Open the `CONFIG` modal and paste the JSON.
+Superset now includes a built-in **Theme Management** interface accessible from the admin menu under **Settings > Themes**.
+
+### Creating a New Theme
+
+1. Navigate to **Settings > Themes** in the Superset interface
+2. Click **+ Theme** to create a new theme
+3. Use the [Ant Design Theme Editor](https://ant.design/theme-editor) to design your theme:
+ - Design your palette, typography, and component overrides
+ - Open the `CONFIG` modal and copy the JSON configuration
+4. Paste the JSON into the theme definition field in Superset
+5. Give your theme a descriptive name and save
You can also extend with Superset-specific tokens (documented in the default theme object) before you import.
-## 2 — Apply it instance-wide
+### Applying Themes to Dashboards
+
+Once created, themes can be applied to individual dashboards:
+- Edit any dashboard and select your custom theme from the theme dropdown
+- Each dashboard can have its own theme, allowing for branded or context-specific styling
+
+## Alternative: Instance-wide Configuration
+
+For system-wide theming, you can configure default themes via Python configuration:
+
+### Setting Default Themes
```python
# superset_config.py
-THEME = {
- # Paste your JSON theme definition here
+
+# Default theme (light mode)
+THEME_DEFAULT = {
+ "token": {
+ "colorPrimary": "#2893B3",
+ "colorSuccess": "#5ac189",
+ # ... your theme JSON configuration
+ }
+}
+
+# Dark theme configuration
+THEME_DARK = {
+ "algorithm": "dark",
+ "token": {
+ "colorPrimary": "#2893B3",
+ # ... your dark theme overrides
+ }
+}
+
+# Theme behavior settings
+THEME_SETTINGS = {
+ "enforced": False, # If True, forces default theme always
+ "allowSwitching": True, # Allow users to switch between themes
+ "allowOSPreference": True, # Auto-detect system theme preference
}
```
-Restart Superset to apply changes
+### Referencing CRUD Themes
-## 3 — Tweak live in the app (beta)
+You can also reference themes created via the CRUD interface by UUID:
Review Comment:
What I was thinking here is say you tell a designer: "ok go build an awesome theme for Superset", then the designer goes in and creates a great theme, now how do you set it as your default? I guess you could copy paste the theme definition into the config file manually ...
The whole idea was to allow for a config to reference a non-system-defined theme directly. But if copy/paste JSON blob is fine we can remove some complexity here...
I could remove all that uuid logic/stuff here if we don't think it's helpful. Maybe it's more confusing than helpful overall.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3106111410
@mistercrunch Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/16463795059). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2215999722
##########
superset-frontend/src/features/themes/ThemeModal.tsx:
##########
@@ -0,0 +1,369 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FunctionComponent, useState, useEffect, ChangeEvent } from 'react';
+
+import { css, styled, t, useTheme } from '@superset-ui/core';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+import { isUserAdmin } from 'src/dashboard/util/permissionUtils';
+import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
+import getBootstrapData from 'src/utils/getBootstrapData';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import { Input, Modal, JsonEditor, Button } from '@superset-ui/core/components';
+import { Typography } from '@superset-ui/core/components/Typography';
+
+import { OnlyKeyWithType } from 'src/utils/types';
+import { ThemeObject } from './types';
+
+interface ThemeModalProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast?: (msg: string) => void;
+ theme?: ThemeObject | null;
+ onThemeAdd?: (theme?: ThemeObject) => void;
+ onHide: () => void;
+ show: boolean;
+}
+
+type ThemeStringKeys = keyof Pick<
+ ThemeObject,
+ OnlyKeyWithType<ThemeObject, String>
+>;
+
+const StyledThemeTitle = styled.div(
+ ({ theme }) => css`
+ margin: ${theme.sizeUnit * 2}px auto ${theme.sizeUnit * 4}px auto;
+ `,
+);
+
+const StyledJsonEditor = styled.div`
+ ${({ theme }) => css`
+ border-radius: ${theme.borderRadius}px;
+ border: 1px solid ${theme.colorBorder};
+
+ .ace_editor {
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const ThemeContainer = styled.div(
+ ({ theme }) => css`
+ margin-bottom: ${theme.sizeUnit * 10}px;
+
+ .control-label {
+ margin-bottom: ${theme.sizeUnit * 2}px;
+ }
+
+ .required {
+ margin-left: ${theme.sizeUnit / 2}px;
+ color: ${theme.colorErrorText};
+ }
+
+ input[type='text'] {
+ padding: ${theme.sizeUnit * 1.5}px ${theme.sizeUnit * 2}px;
+ border: 1px solid ${theme.colorBorder};
+ border-radius: ${theme.borderRadius}px;
+ width: 50%;
+ }
+ `,
+);
+
+const ThemeModal: FunctionComponent<ThemeModalProps> = ({
+ addDangerToast,
+ addSuccessToast,
+ onThemeAdd,
+ onHide,
+ show,
+ theme = null,
+}) => {
+ const supersetTheme = useTheme();
+ const { setTemporaryTheme, clearLocalOverrides, hasDevOverride } =
+ useThemeContext();
+ const [disableSave, setDisableSave] = useState<boolean>(true);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [isHidden, setIsHidden] = useState<boolean>(true);
+ const isEditMode = theme !== null;
+ const user = getBootstrapData()?.user;
+ const canDevelopThemes = user
Review Comment:
Should we use the permission tied to the `theme` POST/PUT endpoint instead so that non-admins can also develop themes when granted such permissions or is there a different logic behind this choice?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3119864724
@mistercrunch Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/16528717600). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3106354978
@mistercrunch Ephemeral environment spinning up at http://54.191.232.213:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3111244686
@mistercrunch Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/16483985615). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2229154708
##########
superset-frontend/packages/superset-ui-core/src/theme/utils/themeUtils.ts:
##########
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import tinycolor from 'tinycolor2';
+import type { SupersetTheme, FontSizeKey, ColorVariants } from '../types';
+
+const fontSizeMap: Record<FontSizeKey, keyof SupersetTheme> = {
+ xs: 'fontSizeXS',
+ s: 'fontSizeSM',
+ m: 'fontSize',
+ l: 'fontSizeLG',
+ xl: 'fontSizeXL',
+ xxl: 'fontSizeXXL',
+};
+
+/**
+ * Get font size from theme tokens based on size key
+ * @param theme - Theme tokens from useTheme()
+ * @param size - Font size key
+ * @returns Font size as string
+ */
+export function getFontSize(theme: SupersetTheme, size?: FontSizeKey): string {
+ const key = fontSizeMap[size || 'm'];
+ return String(theme[key] || theme.fontSize);
+}
+
+/**
+ * Get color variants for a given color type from theme tokens
+ * @param theme - Theme tokens from useTheme()
+ * @param color - Color type (e.g., 'primary', 'error', 'success')
+ * @returns ColorVariants object with bg, border, text colors etc.
+ */
+export function getColorVariants(
+ theme: SupersetTheme,
+ color: string,
+): ColorVariants {
+ const firstLetterCapped = color.charAt(0).toUpperCase() + color.slice(1);
+
+ if (color === 'default' || color === 'grayscale') {
+ const isDark = isThemeDark(theme);
+
+ const flipBrightness = (baseColor: string): string => {
+ if (!isDark) return baseColor;
+ const { r, g, b } = tinycolor(baseColor).toRgb();
+ const invertedColor = tinycolor({ r: 255 - r, g: 255 - g, b: 255 - b });
+ return invertedColor.toHexString();
+ };
+
+ return {
+ active: flipBrightness('#222'),
+ textActive: flipBrightness('#444'),
+ text: flipBrightness('#555'),
+ textHover: flipBrightness('#666'),
+ hover: flipBrightness('#888'),
+ borderHover: flipBrightness('#AAA'),
+ border: flipBrightness('#CCC'),
+ bgHover: flipBrightness('#DDD'),
+ bg: flipBrightness('#F4F4F4'),
+ };
+ }
+
+ return {
+ active: (theme as any)[`color${firstLetterCapped}Active`],
+ textActive: (theme as any)[`color${firstLetterCapped}TextActive`],
+ text: (theme as any)[`color${firstLetterCapped}Text`],
+ textHover: (theme as any)[`color${firstLetterCapped}TextHover`],
+ hover: (theme as any)[`color${firstLetterCapped}Hover`],
+ borderHover: (theme as any)[`color${firstLetterCapped}BorderHover`],
+ border: (theme as any)[`color${firstLetterCapped}Border`],
+ bgHover: (theme as any)[`color${firstLetterCapped}BgHover`],
+ bg: (theme as any)[`color${firstLetterCapped}Bg`],
+ };
+}
+
+/**
+ * Check if the current theme is dark mode based on background color
+ * @param theme - Theme tokens from useTheme()
+ * @returns true if theme is dark, false if light
+ */
+export function isThemeDark(theme: SupersetTheme): boolean {
+ return tinycolor(theme.colorBgContainer).isDark();
Review Comment:
I think both/either should return a dark enough color for tinycolor to categorize well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3117987315
> Currently we set some default tokens in packages/superset-ui-core/src/theme/Theme.tsx But they are not visible in the ui for the theme and i am not sure if they are applied to database either. Should we move them to default config options instead?
The theme constructor is (AFAIK) deterministic (well kinda), meaning when you set your theme to `{algorithm: "dark"}`, as it gets created, will go through its construction, and defaults get applied, so the defaults here https://github.com/apache/superset/blob/master/superset-frontend/packages/superset-ui-core/src/theme/Theme.tsx#L73-L101 will get applied. Now if you were to pass an override like `{algorithm: "dark": token: { brandLogoUrl: "{some_url}" }}`, it'll override the default, and generated tokens will end up the same, except for that one. Now across versions of Superset where defaults might change, the "compiled" theme might change as defaults change, but I'm pretty sure that's what we want in most (all?) cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2223432100
##########
superset-frontend/src/features/themes/ThemeModal.tsx:
##########
@@ -0,0 +1,369 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FunctionComponent, useState, useEffect, ChangeEvent } from 'react';
+
+import { css, styled, t, useTheme } from '@superset-ui/core';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+import { isUserAdmin } from 'src/dashboard/util/permissionUtils';
+import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
+import getBootstrapData from 'src/utils/getBootstrapData';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import { Input, Modal, JsonEditor, Button } from '@superset-ui/core/components';
+import { Typography } from '@superset-ui/core/components/Typography';
+
+import { OnlyKeyWithType } from 'src/utils/types';
+import { ThemeObject } from './types';
+
+interface ThemeModalProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast?: (msg: string) => void;
+ theme?: ThemeObject | null;
+ onThemeAdd?: (theme?: ThemeObject) => void;
+ onHide: () => void;
+ show: boolean;
+}
+
+type ThemeStringKeys = keyof Pick<
+ ThemeObject,
+ OnlyKeyWithType<ThemeObject, String>
+>;
+
+const StyledThemeTitle = styled.div(
+ ({ theme }) => css`
+ margin: ${theme.sizeUnit * 2}px auto ${theme.sizeUnit * 4}px auto;
+ `,
+);
+
+const StyledJsonEditor = styled.div`
+ ${({ theme }) => css`
+ border-radius: ${theme.borderRadius}px;
+ border: 1px solid ${theme.colorBorder};
+
+ .ace_editor {
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const ThemeContainer = styled.div(
+ ({ theme }) => css`
+ margin-bottom: ${theme.sizeUnit * 10}px;
+
+ .control-label {
+ margin-bottom: ${theme.sizeUnit * 2}px;
+ }
+
+ .required {
+ margin-left: ${theme.sizeUnit / 2}px;
+ color: ${theme.colorErrorText};
+ }
+
+ input[type='text'] {
+ padding: ${theme.sizeUnit * 1.5}px ${theme.sizeUnit * 2}px;
+ border: 1px solid ${theme.colorBorder};
+ border-radius: ${theme.borderRadius}px;
+ width: 50%;
+ }
+ `,
+);
+
+const ThemeModal: FunctionComponent<ThemeModalProps> = ({
+ addDangerToast,
+ addSuccessToast,
+ onThemeAdd,
+ onHide,
+ show,
+ theme = null,
+}) => {
+ const supersetTheme = useTheme();
+ const { setTemporaryTheme, clearLocalOverrides, hasDevOverride } =
+ useThemeContext();
+ const [disableSave, setDisableSave] = useState<boolean>(true);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [isHidden, setIsHidden] = useState<boolean>(true);
+ const isEditMode = theme !== null;
+ const user = getBootstrapData()?.user;
+ const canDevelopThemes = user
Review Comment:
ok I fixed that!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3106321477
@mistercrunch Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/16464684175). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2223708923
##########
.pre-commit-config.yaml:
##########
@@ -97,9 +97,9 @@ repos:
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.9.7
hooks:
+ - id: ruff-format
Review Comment:
bycatch: format before check is better(!)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2216012236
##########
superset-frontend/src/pages/ThemeList/index.tsx:
##########
@@ -0,0 +1,379 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { useMemo, useState } from 'react';
+import { t, SupersetClient } from '@superset-ui/core';
+
+import rison from 'rison';
+import { useListViewResource } from 'src/views/CRUD/hooks';
+import { createErrorHandler, createFetchRelated } from 'src/views/CRUD/utils';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu';
+import {
+ DeleteModal,
+ ConfirmStatusChange,
+ Loading,
+} from '@superset-ui/core/components';
+import handleResourceExport from 'src/utils/export';
+import {
+ ModifiedInfo,
+ ListView,
+ ListViewActionsBar,
+ ListViewFilterOperator as FilterOperator,
+ type ListViewProps,
+ type ListViewActionProps,
+ type ListViewFilters,
+} from 'src/components';
+
+import ThemeModal from 'src/features/themes/ThemeModal';
+import { ThemeObject } from 'src/features/themes/types';
+import { QueryObjectColumns } from 'src/views/CRUD/types';
+import { Icons } from '@superset-ui/core/components/Icons';
+
+const PAGE_SIZE = 25;
+
+interface ThemesListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+ user: {
+ userId: string | number;
+ firstName: string;
+ lastName: string;
+ };
+}
+
+function ThemesList({
+ addDangerToast,
+ addSuccessToast,
+ user,
+}: ThemesListProps) {
+ const {
+ state: {
+ loading,
+ resourceCount: themesCount,
+ resourceCollection: themes,
+ bulkSelectEnabled,
+ },
+ hasPerm,
+ fetchData,
+ refreshData,
+ toggleBulkSelect,
+ } = useListViewResource<ThemeObject>('theme', t('Themes'), addDangerToast);
+ const { setTemporaryTheme } = useThemeContext();
+ const [themeModalOpen, setThemeModalOpen] = useState<boolean>(false);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [preparingExport, setPreparingExport] = useState<boolean>(false);
+
+ const canCreate = hasPerm('can_write');
Review Comment:
I think in the Modal we are tying the ability to create to admins only while here is based on the view permission?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2220857578
##########
superset-frontend/src/theme/ThemeController.ts:
##########
@@ -236,6 +246,76 @@ export class ThemeController {
this.updateTheme(defaultTheme);
}
+ /**
+ * Sets a CRUD theme by ID. This will fetch the theme from the API and apply it.
+ * @param themeId - The ID of the CRUD theme to apply
+ */
+ public async setCrudTheme(themeId: string | null): Promise<void> {
+ this.crudThemeId = themeId;
+
+ if (themeId) {
+ this.storage.setItem(STORAGE_KEYS.CRUD_THEME_ID, themeId);
+ try {
+ const themeConfig = await this.fetchCrudTheme(themeId);
+ if (themeConfig) {
+ this.updateTheme(themeConfig);
+ }
+ } catch (error) {
+ console.error('Failed to load CRUD theme:', error);
+ this.fallbackToDefaultMode();
+ }
+ } else {
+ this.storage.removeItem(STORAGE_KEYS.CRUD_THEME_ID);
+ this.resetTheme();
+ }
+ }
+
+ /**
+ * Sets a temporary theme override for development purposes.
+ * This does not persist the theme but allows live preview.
+ * @param theme - The theme configuration to apply temporarily
+ */
+ public setTemporaryTheme(theme: AnyThemeConfig): void {
+ this.validateThemeUpdatePermission();
+
+ this.devThemeOverride = theme;
+ this.storage.setItem(
+ STORAGE_KEYS.DEV_THEME_OVERRIDE,
+ JSON.stringify(theme),
+ );
+
+ const normalizedTheme = this.normalizeTheme(theme);
+ this.updateTheme(normalizedTheme);
+ }
Review Comment:
Right, it applies/persist for the current user using local storage, use can "clear local settings" to fallback on the system default + light/dark preferences
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3104451262
@mistercrunch Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/16453246754). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3104798674
@mistercrunch Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/16455288166). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3106126767
@mistercrunch Ephemeral environment spinning up at http://35.161.20.86:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2213976927
##########
superset/migrations/versions/2025-07-15_16-40_3fd555e76e3d_theme_crud.py:
##########
@@ -0,0 +1,65 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""theme_crud
+
+Revision ID: 3fd555e76e3d
+Revises: 363a9b1e8992
+Create Date: 2025-07-15 16:40:35.166210
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+from sqlalchemy.dialects import mysql
+from sqlalchemy_utils import UUIDType
+
+# revision identifiers, used by Alembic.
+revision = "3fd555e76e3d"
+down_revision = "363a9b1e8992"
+
+
+def upgrade():
+ # ### commands auto generated by Alembic - please adjust! ###
+ op.create_table(
Review Comment:
I think we're supposed to use the helper functions for creating tables and columns going forward
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2229939804
##########
superset-frontend/src/features/themes/ThemeModal.tsx:
##########
@@ -0,0 +1,394 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FunctionComponent, useState, useEffect, ChangeEvent } from 'react';
+
+import { css, styled, t, useTheme } from '@superset-ui/core';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import {
+ Input,
+ Modal,
+ JsonEditor,
+ Button,
+ Form,
+ Tooltip,
+ Alert,
+} from '@superset-ui/core/components';
+import { Typography } from '@superset-ui/core/components/Typography';
+
+import { OnlyKeyWithType } from 'src/utils/types';
+import { ThemeObject } from './types';
+
+interface ThemeModalProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast?: (msg: string) => void;
+ theme?: ThemeObject | null;
+ onThemeAdd?: (theme?: ThemeObject) => void;
+ onHide: () => void;
+ show: boolean;
+ canDevelop?: boolean;
+}
+
+type ThemeStringKeys = keyof Pick<
+ ThemeObject,
+ OnlyKeyWithType<ThemeObject, String>
+>;
+
+const StyledJsonEditor = styled.div`
+ ${({ theme }) => css`
+ border-radius: ${theme.borderRadius}px;
+ border: 1px solid ${theme.colorBorder};
+
+ .ace_editor {
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const ThemeModal: FunctionComponent<ThemeModalProps> = ({
+ addDangerToast,
+ addSuccessToast,
+ onThemeAdd,
+ onHide,
+ show,
+ theme = null,
+ canDevelop = false,
+}) => {
+ const supersetTheme = useTheme();
+ const { setTemporaryTheme } = useThemeContext();
+ const [disableSave, setDisableSave] = useState<boolean>(true);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [isHidden, setIsHidden] = useState<boolean>(true);
+ const isEditMode = theme !== null;
+ const isSystemTheme = currentTheme?.is_system === true;
+ const isReadOnly = isSystemTheme;
+ const canDevelopThemes = canDevelop;
+
+ // theme fetch logic
+ const {
+ state: { loading, resource },
+ fetchResource,
+ createResource,
+ updateResource,
+ } = useSingleViewResource<ThemeObject>('theme', t('theme'), addDangerToast);
+
+ // Functions
+ const hide = () => {
+ onHide();
+ setCurrentTheme(null);
+ };
+
+ const onSave = () => {
+ if (isEditMode) {
+ // Edit
+ if (currentTheme?.id) {
+ const update_id = currentTheme.id;
+ delete currentTheme.id;
+ delete currentTheme.created_by;
+ delete currentTheme.changed_by;
+ delete currentTheme.changed_on_delta_humanized;
+
+ updateResource(update_id, currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ } else if (currentTheme) {
+ // Create
+ createResource(currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ };
+
+ const onApply = () => {
+ if (currentTheme?.json_data && isValidJson(currentTheme.json_data)) {
+ try {
+ const themeConfig = JSON.parse(currentTheme.json_data);
+ setTemporaryTheme(themeConfig);
+ if (addSuccessToast) {
+ addSuccessToast(t('Local theme set for preview'));
+ }
+ } catch (error) {
+ addDangerToast(t('Failed to apply theme: Invalid JSON'));
+ }
+ }
+ };
+
+ const onThemeNameChange = (event: ChangeEvent<HTMLInputElement>) => {
+ const { target } = event;
+
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: currentTheme ? currentTheme.json_data : '',
+ };
+
+ data[target.name as ThemeStringKeys] = target.value;
+ setCurrentTheme(data);
+ };
+
+ const onJsonDataChange = (jsonData: string) => {
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: jsonData,
+ };
+ setCurrentTheme(data);
+ };
+
+ const isValidJson = (str?: string) => {
+ if (!str) return false;
+ try {
+ JSON.parse(str);
+ return true;
+ } catch (e) {
+ return false;
+ }
+ };
+
+ const validate = () => {
+ if (isReadOnly) {
+ setDisableSave(true);
+ return;
+ }
+
+ if (
+ currentTheme?.theme_name.length &&
+ currentTheme?.json_data?.length &&
+ isValidJson(currentTheme.json_data)
+ ) {
+ setDisableSave(false);
+ } else {
+ setDisableSave(true);
+ }
+ };
Review Comment:
<img width="571" height="415" alt="Screenshot 2025-07-24 at 6 31 45 PM" src="https://github.com/user-attachments/assets/3f1c501e-f0f0-42f9-b0cb-02039ab92537" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3115046333
@mistercrunch Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/16508313281). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2220862476
##########
superset-frontend/src/theme/ThemeController.ts:
##########
@@ -43,6 +43,8 @@ const DEFAULT_THEME_SETTINGS = {
const STORAGE_KEYS = {
THEME_MODE: 'superset-theme-mode',
Review Comment:
Yes, this should represent whether the user wants light/dark/os-settings for themselves, I made the related feature flag on by default for discoverability.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2220868159
##########
superset-frontend/src/theme/ThemeController.ts:
##########
@@ -236,6 +246,76 @@ export class ThemeController {
this.updateTheme(defaultTheme);
}
+ /**
+ * Sets a CRUD theme by ID. This will fetch the theme from the API and apply it.
+ * @param themeId - The ID of the CRUD theme to apply
+ */
+ public async setCrudTheme(themeId: string | null): Promise<void> {
+ this.crudThemeId = themeId;
+
+ if (themeId) {
+ this.storage.setItem(STORAGE_KEYS.CRUD_THEME_ID, themeId);
+ try {
+ const themeConfig = await this.fetchCrudTheme(themeId);
+ if (themeConfig) {
+ this.updateTheme(themeConfig);
+ }
+ } catch (error) {
+ console.error('Failed to load CRUD theme:', error);
+ this.fallbackToDefaultMode();
+ }
+ } else {
+ this.storage.removeItem(STORAGE_KEYS.CRUD_THEME_ID);
+ this.resetTheme();
+ }
+ }
Review Comment:
That's right, though we might need it in the future (?) might be attached to user prefs in the future. Can keep for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2223704594
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -215,6 +222,34 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
return () => {};
}, [css]);
+ // Apply dashboard theme when dashboard loads
+ useEffect(() => {
+ const applyDashboardTheme = async () => {
+ if (dashboard?.theme?.id) {
+ try {
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/theme/${dashboard.theme.id}`,
+ });
+ const themeConfig = JSON.parse(response.json.result.json_data);
+ themeContext.setTemporaryTheme(themeConfig);
Review Comment:
done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2226295894
##########
.gitignore:
##########
@@ -131,3 +131,4 @@ superset/static/stats/statistics.html
# LLM-related
CLAUDE.local.md
.aider*
+CLAUDE.local.md
Review Comment:
```suggestion
```
seems like a duplicate
##########
superset-frontend/src/theme/ThemeController.ts:
##########
@@ -406,6 +574,17 @@ export class ThemeController {
* @returns The theme configuration for the specified mode or null if not available
*/
private getThemeForMode(mode: ThemeMode): AnyThemeConfig | null {
+ // Priority 1: Dev theme override (highest priority for development)
+ // Dev overrides affect all contexts
+ if (this.devThemeOverride) {
+ return this.devThemeOverride;
+ }
+
+ // Priority 2: CRUD theme (organizational themes)
+ // CRUD themes should NEVER affect the mode-based theme selection
+ // Dashboard themes are handled separately via createDashboardThemeProvider
Review Comment:
Missing implementation
##########
superset-frontend/src/components/ListView/Filters/Search.tsx:
##########
@@ -43,9 +43,7 @@ interface SearchHeaderProps extends BaseFilter {
toolTipDescription: string | undefined;
}
-const StyledInput = styled(Input)`
- border-radius: ${({ theme }) => theme.borderRadius}px;
-`;
+// Using vanilla antd Input to avoid styled component circular dependency issue
Review Comment:
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2228051415
##########
superset-frontend/packages/superset-ui-core/src/components/ThemeSelect/index.tsx:
##########
@@ -43,36 +50,65 @@ const ThemeSelect: React.FC<ThemeSelectProps> = ({
[ThemeAlgorithm.COMPACT]: <Icons.CompressOutlined />,
};
+ // Use different icon when local theme is active
+ const triggerIcon = hasLocalOverride ? (
+ <Icons.FormatPainterOutlined style={{ color: theme.colors.error.base }} />
Review Comment:
Would it be better to use the `iconColor` prop with `theme.colorError` in this case?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2228139803
##########
superset-frontend/packages/superset-ui-core/src/components/ThemeSelect/index.tsx:
##########
@@ -43,36 +50,65 @@ const ThemeSelect: React.FC<ThemeSelectProps> = ({
[ThemeAlgorithm.COMPACT]: <Icons.CompressOutlined />,
};
+ // Use different icon when local theme is active
+ const triggerIcon = hasLocalOverride ? (
+ <Icons.FormatPainterOutlined style={{ color: theme.colors.error.base }} />
Review Comment:
```suggestion
<Icons.FormatPainterOutlined iconColor={theme.colorError} />
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2220865096
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -215,6 +222,34 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
return () => {};
}, [css]);
+ // Apply dashboard theme when dashboard loads
+ useEffect(() => {
+ const applyDashboardTheme = async () => {
+ if (dashboard?.theme?.id) {
Review Comment:
Good catch. For now I may just prevent deletion for themes attached to dashboards or dissociate on theme delete, gotta decide what to do .... TODO
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3110155233
@mistercrunch Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/16481677290). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3110182127
@mistercrunch Ephemeral environment spinning up at http://35.92.99.132:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2209480456
##########
docs/docs/configuration/theming.mdx:
##########
@@ -38,7 +38,6 @@ Set the feature flag in your `superset_config`
```python
DEFAULT_FEATURE_FLAGS: dict[str, bool] = {
{{ ... }}
- THEME_ALLOW_THEME_EDITOR_BETA = True,
Review Comment:
No need for this feature flag now as we have an "apply" button in the theme editor in the CRUD
##########
docs/docs/configuration/theming.mdx:
##########
@@ -38,7 +38,6 @@ Set the feature flag in your `superset_config`
```python
DEFAULT_FEATURE_FLAGS: dict[str, bool] = {
{{ ... }}
- THEME_ALLOW_THEME_EDITOR_BETA = True,
Review Comment:
No need for this feature flag now as we have an "apply" button in the theme editor in the new CRUD
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2229829393
##########
superset-frontend/packages/superset-ui-core/src/components/ThemeSelect/index.tsx:
##########
@@ -43,36 +50,65 @@ const ThemeSelect: React.FC<ThemeSelectProps> = ({
[ThemeAlgorithm.COMPACT]: <Icons.CompressOutlined />,
};
+ // Use different icon when local theme is active
+ const triggerIcon = hasLocalOverride ? (
+ <Icons.FormatPainterOutlined style={{ color: theme.colorErrorText }} />
+ ) : (
+ themeIconMap[themeMode] || <Icons.FormatPainterOutlined />
+ );
+
+ const menuItems: any[] = [
Review Comment:
There should be a type for menu items
##########
superset-frontend/packages/superset-ui-core/src/components/ThemeSelect/index.tsx:
##########
@@ -43,36 +50,65 @@ const ThemeSelect: React.FC<ThemeSelectProps> = ({
[ThemeAlgorithm.COMPACT]: <Icons.CompressOutlined />,
};
+ // Use different icon when local theme is active
+ const triggerIcon = hasLocalOverride ? (
+ <Icons.FormatPainterOutlined style={{ color: theme.colorErrorText }} />
+ ) : (
+ themeIconMap[themeMode] || <Icons.FormatPainterOutlined />
+ );
+
+ const menuItems: any[] = [
+ {
+ type: 'group',
+ label: t('Theme'),
+ },
+ {
+ key: ThemeMode.DEFAULT,
+ label: t('Light'),
+ icon: <Icons.SunOutlined />,
+ onClick: () => handleSelect(ThemeMode.DEFAULT),
+ },
+ {
+ key: ThemeMode.DARK,
+ label: t('Dark'),
+ icon: <Icons.MoonOutlined />,
+ onClick: () => handleSelect(ThemeMode.DARK),
+ },
+ ...(allowOSPreference
+ ? [
+ {
+ key: ThemeMode.SYSTEM,
+ label: t('Match system'),
+ icon: <Icons.FormatPainterOutlined />,
+ onClick: () => handleSelect(ThemeMode.SYSTEM),
+ },
+ ]
+ : []),
+ ];
+
+ // Add clear settings option only when there's a local theme active
+ if (onClearLocalSettings && hasLocalOverride) {
+ menuItems.push(
+ { type: 'divider' } as any,
Review Comment:
These any assertions shouldn't be necessary
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch merged PR #34182:
URL: https://github.com/apache/superset/pull/34182
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2214257921
##########
superset/migrations/versions/2025-07-15_16-40_3fd555e76e3d_theme_crud.py:
##########
@@ -0,0 +1,65 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""theme_crud
+
+Revision ID: 3fd555e76e3d
+Revises: 363a9b1e8992
+Create Date: 2025-07-15 16:40:35.166210
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+from sqlalchemy.dialects import mysql
+from sqlalchemy_utils import UUIDType
+
+# revision identifiers, used by Alembic.
+revision = "3fd555e76e3d"
+down_revision = "363a9b1e8992"
+
+
+def upgrade():
+ # ### commands auto generated by Alembic - please adjust! ###
+ op.create_table(
Review Comment:
makes sense, let me fix and see if I can add a linting rule (?) Though would love to spend some cycles improving migrations:
- compact migration before say 3.0 (wondering where to draw the line)
- making sure we use utils consistently
- add linting rules to prevent not using utils
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2220863906
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -215,6 +222,34 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
return () => {};
}, [css]);
+ // Apply dashboard theme when dashboard loads
+ useEffect(() => {
+ const applyDashboardTheme = async () => {
+ if (dashboard?.theme?.id) {
+ try {
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/theme/${dashboard.theme.id}`,
+ });
+ const themeConfig = JSON.parse(response.json.result.json_data);
+ themeContext.setTemporaryTheme(themeConfig);
Review Comment:
yes we had a product conversation about this. I think we decided to scope it to only the dashboard areas (navbar shouldn't change). I see to change things here so it only injects the dashboard theme to that section of the page ... TODO
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3104308310
@mistercrunch Ephemeral environment spinning up at http://44.245.7.189:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3084934811
@mistercrunch Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/16352284507). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3104810862
@mistercrunch Ephemeral environment spinning up at http://44.243.111.147:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2215962113
##########
superset-frontend/src/theme/ThemeController.ts:
##########
@@ -236,6 +246,76 @@ export class ThemeController {
this.updateTheme(defaultTheme);
}
+ /**
+ * Sets a CRUD theme by ID. This will fetch the theme from the API and apply it.
+ * @param themeId - The ID of the CRUD theme to apply
+ */
+ public async setCrudTheme(themeId: string | null): Promise<void> {
+ this.crudThemeId = themeId;
+
+ if (themeId) {
+ this.storage.setItem(STORAGE_KEYS.CRUD_THEME_ID, themeId);
+ try {
+ const themeConfig = await this.fetchCrudTheme(themeId);
+ if (themeConfig) {
+ this.updateTheme(themeConfig);
+ }
+ } catch (error) {
+ console.error('Failed to load CRUD theme:', error);
+ this.fallbackToDefaultMode();
+ }
+ } else {
+ this.storage.removeItem(STORAGE_KEYS.CRUD_THEME_ID);
+ this.resetTheme();
+ }
+ }
+
+ /**
+ * Sets a temporary theme override for development purposes.
+ * This does not persist the theme but allows live preview.
+ * @param theme - The theme configuration to apply temporarily
+ */
+ public setTemporaryTheme(theme: AnyThemeConfig): void {
+ this.validateThemeUpdatePermission();
+
+ this.devThemeOverride = theme;
+ this.storage.setItem(
+ STORAGE_KEYS.DEV_THEME_OVERRIDE,
+ JSON.stringify(theme),
+ );
+
+ const normalizedTheme = this.normalizeTheme(theme);
+ this.updateTheme(normalizedTheme);
+ }
Review Comment:
This persists the theme? This is what's called when clicked on apply.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2216012236
##########
superset-frontend/src/pages/ThemeList/index.tsx:
##########
@@ -0,0 +1,379 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { useMemo, useState } from 'react';
+import { t, SupersetClient } from '@superset-ui/core';
+
+import rison from 'rison';
+import { useListViewResource } from 'src/views/CRUD/hooks';
+import { createErrorHandler, createFetchRelated } from 'src/views/CRUD/utils';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu';
+import {
+ DeleteModal,
+ ConfirmStatusChange,
+ Loading,
+} from '@superset-ui/core/components';
+import handleResourceExport from 'src/utils/export';
+import {
+ ModifiedInfo,
+ ListView,
+ ListViewActionsBar,
+ ListViewFilterOperator as FilterOperator,
+ type ListViewProps,
+ type ListViewActionProps,
+ type ListViewFilters,
+} from 'src/components';
+
+import ThemeModal from 'src/features/themes/ThemeModal';
+import { ThemeObject } from 'src/features/themes/types';
+import { QueryObjectColumns } from 'src/views/CRUD/types';
+import { Icons } from '@superset-ui/core/components/Icons';
+
+const PAGE_SIZE = 25;
+
+interface ThemesListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+ user: {
+ userId: string | number;
+ firstName: string;
+ lastName: string;
+ };
+}
+
+function ThemesList({
+ addDangerToast,
+ addSuccessToast,
+ user,
+}: ThemesListProps) {
+ const {
+ state: {
+ loading,
+ resourceCount: themesCount,
+ resourceCollection: themes,
+ bulkSelectEnabled,
+ },
+ hasPerm,
+ fetchData,
+ refreshData,
+ toggleBulkSelect,
+ } = useListViewResource<ThemeObject>('theme', t('Themes'), addDangerToast);
+ const { setTemporaryTheme } = useThemeContext();
+ const [themeModalOpen, setThemeModalOpen] = useState<boolean>(false);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [preparingExport, setPreparingExport] = useState<boolean>(false);
+
+ const canCreate = hasPerm('can_write');
Review Comment:
I think in the Modal we are tying the ability to create to admins only while here is based on the view permission?
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -215,6 +222,34 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
return () => {};
}, [css]);
+ // Apply dashboard theme when dashboard loads
+ useEffect(() => {
+ const applyDashboardTheme = async () => {
+ if (dashboard?.theme?.id) {
Review Comment:
When do we clean dead references of deleted themes in the dashboard metadata?
##########
superset-frontend/src/features/themes/ThemeModal.tsx:
##########
@@ -0,0 +1,369 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FunctionComponent, useState, useEffect, ChangeEvent } from 'react';
+
+import { css, styled, t, useTheme } from '@superset-ui/core';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+import { isUserAdmin } from 'src/dashboard/util/permissionUtils';
+import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
+import getBootstrapData from 'src/utils/getBootstrapData';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import { Input, Modal, JsonEditor, Button } from '@superset-ui/core/components';
+import { Typography } from '@superset-ui/core/components/Typography';
+
+import { OnlyKeyWithType } from 'src/utils/types';
+import { ThemeObject } from './types';
+
+interface ThemeModalProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast?: (msg: string) => void;
+ theme?: ThemeObject | null;
+ onThemeAdd?: (theme?: ThemeObject) => void;
+ onHide: () => void;
+ show: boolean;
+}
+
+type ThemeStringKeys = keyof Pick<
+ ThemeObject,
+ OnlyKeyWithType<ThemeObject, String>
+>;
+
+const StyledThemeTitle = styled.div(
+ ({ theme }) => css`
+ margin: ${theme.sizeUnit * 2}px auto ${theme.sizeUnit * 4}px auto;
+ `,
+);
+
+const StyledJsonEditor = styled.div`
+ ${({ theme }) => css`
+ border-radius: ${theme.borderRadius}px;
+ border: 1px solid ${theme.colorBorder};
+
+ .ace_editor {
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const ThemeContainer = styled.div(
+ ({ theme }) => css`
+ margin-bottom: ${theme.sizeUnit * 10}px;
+
+ .control-label {
+ margin-bottom: ${theme.sizeUnit * 2}px;
+ }
+
+ .required {
+ margin-left: ${theme.sizeUnit / 2}px;
+ color: ${theme.colorErrorText};
+ }
+
+ input[type='text'] {
+ padding: ${theme.sizeUnit * 1.5}px ${theme.sizeUnit * 2}px;
+ border: 1px solid ${theme.colorBorder};
+ border-radius: ${theme.borderRadius}px;
+ width: 50%;
+ }
+ `,
+);
+
+const ThemeModal: FunctionComponent<ThemeModalProps> = ({
+ addDangerToast,
+ addSuccessToast,
+ onThemeAdd,
+ onHide,
+ show,
+ theme = null,
+}) => {
+ const supersetTheme = useTheme();
+ const { setTemporaryTheme, clearLocalOverrides, hasDevOverride } =
+ useThemeContext();
+ const [disableSave, setDisableSave] = useState<boolean>(true);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [isHidden, setIsHidden] = useState<boolean>(true);
+ const isEditMode = theme !== null;
+ const user = getBootstrapData()?.user;
+ const canDevelopThemes = user
Review Comment:
Should we use the permission tied to the `theme` POST/PUT endpoint instead so that non-admins can also develop themes when granted such permissions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2226672694
##########
docs/docs/configuration/theming.mdx:
##########
@@ -10,44 +10,92 @@ version: 1
apache-superset>=6.0
:::
-Superset now rides on **Ant Design v5’s token-based theming**.
+Superset now rides on **Ant Design v5's token-based theming**.
Every Antd token works, plus a handful of Superset-specific ones for charts and dashboard chrome.
-## 1 — Create a theme
+## Managing Themes via CRUD Interface
-1. Open the official [Ant Design Theme Editor](https://ant.design/theme-editor)
-2. Design your palette, typography, and component overrides.
-3. Open the `CONFIG` modal and paste the JSON.
+Superset now includes a built-in **Theme Management** interface accessible from the admin menu under **Settings > Themes**.
+
+### Creating a New Theme
+
+1. Navigate to **Settings > Themes** in the Superset interface
+2. Click **+ Theme** to create a new theme
+3. Use the [Ant Design Theme Editor](https://ant.design/theme-editor) to design your theme:
+ - Design your palette, typography, and component overrides
+ - Open the `CONFIG` modal and copy the JSON configuration
+4. Paste the JSON into the theme definition field in Superset
+5. Give your theme a descriptive name and save
You can also extend with Superset-specific tokens (documented in the default theme object) before you import.
-## 2 — Apply it instance-wide
+### Applying Themes to Dashboards
+
+Once created, themes can be applied to individual dashboards:
+- Edit any dashboard and select your custom theme from the theme dropdown
+- Each dashboard can have its own theme, allowing for branded or context-specific styling
+
+## Alternative: Instance-wide Configuration
+
+For system-wide theming, you can configure default themes via Python configuration:
+
+### Setting Default Themes
```python
# superset_config.py
-THEME = {
- # Paste your JSON theme definition here
+
+# Default theme (light mode)
+THEME_DEFAULT = {
+ "token": {
+ "colorPrimary": "#2893B3",
+ "colorSuccess": "#5ac189",
+ # ... your theme JSON configuration
+ }
+}
+
+# Dark theme configuration
+THEME_DARK = {
+ "algorithm": "dark",
+ "token": {
+ "colorPrimary": "#2893B3",
+ # ... your dark theme overrides
+ }
+}
+
+# Theme behavior settings
+THEME_SETTINGS = {
+ "enforced": False, # If True, forces default theme always
+ "allowSwitching": True, # Allow users to switch between themes
+ "allowOSPreference": True, # Auto-detect system theme preference
}
```
-Restart Superset to apply changes
+### Referencing CRUD Themes
-## 3 — Tweak live in the app (beta)
+You can also reference themes created via the CRUD interface by UUID:
Review Comment:
ok, did more thinking and decided to rip that uuid stuff out, thanks for the feedback here, I was overthinking things.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3111372540
@mistercrunch Ephemeral environment spinning up at http://34.210.74.116:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2229978453
##########
superset-frontend/src/features/themes/ThemeModal.tsx:
##########
@@ -0,0 +1,394 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FunctionComponent, useState, useEffect, ChangeEvent } from 'react';
+
+import { css, styled, t, useTheme } from '@superset-ui/core';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import {
+ Input,
+ Modal,
+ JsonEditor,
+ Button,
+ Form,
+ Tooltip,
+ Alert,
+} from '@superset-ui/core/components';
+import { Typography } from '@superset-ui/core/components/Typography';
+
+import { OnlyKeyWithType } from 'src/utils/types';
+import { ThemeObject } from './types';
+
+interface ThemeModalProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast?: (msg: string) => void;
+ theme?: ThemeObject | null;
+ onThemeAdd?: (theme?: ThemeObject) => void;
+ onHide: () => void;
+ show: boolean;
+ canDevelop?: boolean;
+}
+
+type ThemeStringKeys = keyof Pick<
+ ThemeObject,
+ OnlyKeyWithType<ThemeObject, String>
+>;
+
+const StyledJsonEditor = styled.div`
+ ${({ theme }) => css`
+ border-radius: ${theme.borderRadius}px;
+ border: 1px solid ${theme.colorBorder};
+
+ .ace_editor {
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const ThemeModal: FunctionComponent<ThemeModalProps> = ({
+ addDangerToast,
+ addSuccessToast,
+ onThemeAdd,
+ onHide,
+ show,
+ theme = null,
+ canDevelop = false,
+}) => {
+ const supersetTheme = useTheme();
+ const { setTemporaryTheme } = useThemeContext();
+ const [disableSave, setDisableSave] = useState<boolean>(true);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [isHidden, setIsHidden] = useState<boolean>(true);
+ const isEditMode = theme !== null;
+ const isSystemTheme = currentTheme?.is_system === true;
+ const isReadOnly = isSystemTheme;
+ const canDevelopThemes = canDevelop;
+
+ // theme fetch logic
+ const {
+ state: { loading, resource },
+ fetchResource,
+ createResource,
+ updateResource,
+ } = useSingleViewResource<ThemeObject>('theme', t('theme'), addDangerToast);
+
+ // Functions
+ const hide = () => {
+ onHide();
+ setCurrentTheme(null);
+ };
+
+ const onSave = () => {
+ if (isEditMode) {
+ // Edit
+ if (currentTheme?.id) {
+ const update_id = currentTheme.id;
+ delete currentTheme.id;
+ delete currentTheme.created_by;
+ delete currentTheme.changed_by;
+ delete currentTheme.changed_on_delta_humanized;
+
+ updateResource(update_id, currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ } else if (currentTheme) {
+ // Create
+ createResource(currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ };
+
+ const onApply = () => {
+ if (currentTheme?.json_data && isValidJson(currentTheme.json_data)) {
+ try {
+ const themeConfig = JSON.parse(currentTheme.json_data);
+ setTemporaryTheme(themeConfig);
+ if (addSuccessToast) {
+ addSuccessToast(t('Local theme set for preview'));
+ }
+ } catch (error) {
+ addDangerToast(t('Failed to apply theme: Invalid JSON'));
+ }
+ }
+ };
+
+ const onThemeNameChange = (event: ChangeEvent<HTMLInputElement>) => {
+ const { target } = event;
+
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: currentTheme ? currentTheme.json_data : '',
+ };
+
+ data[target.name as ThemeStringKeys] = target.value;
+ setCurrentTheme(data);
+ };
+
+ const onJsonDataChange = (jsonData: string) => {
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: jsonData,
+ };
+ setCurrentTheme(data);
+ };
+
+ const isValidJson = (str?: string) => {
+ if (!str) return false;
+ try {
+ JSON.parse(str);
+ return true;
+ } catch (e) {
+ return false;
+ }
+ };
+
+ const validate = () => {
+ if (isReadOnly) {
+ setDisableSave(true);
+ return;
+ }
+
+ if (
+ currentTheme?.theme_name.length &&
+ currentTheme?.json_data?.length &&
+ isValidJson(currentTheme.json_data)
+ ) {
+ setDisableSave(false);
+ } else {
+ setDisableSave(true);
+ }
+ };
Review Comment:
Did it in JsonEditor so all other uses of JsonEditor in the app will get this now too!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3119905252
@mistercrunch Ephemeral environment spinning up at http://54.190.54.11:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3076430756
Based on your review schedule, I'll hold off on reviewing this PR until it's marked as ready for review. If you'd like me to take a look now, comment `/korbit-review`.
> Your admin can change your review schedule in the [Korbit Console](https://app.korbit.ai/settings)
[](e300c869-feee-4be0-833f-47752e10a0f8)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2228242868
##########
superset-frontend/src/dashboard/components/CssEditor/index.tsx:
##########
@@ -151,8 +256,17 @@ class CssEditor extends PureComponent<CssEditorProps, CssEditorState> {
width="100%"
editorProps={{ $blockScrolling: true }}
enableLiveAutocompletion
- value={this.state.css || ''}
+ value={this.state.pendingCss || ''}
/>
+ <div style={{ marginTop: 16, textAlign: 'right' }}>
Review Comment:
I think it should be better to use `sizeUnit` and `float: right`
##########
superset-frontend/src/dashboard/components/SaveModal.tsx:
##########
@@ -144,96 +115,87 @@ class SaveModal extends PureComponent<SaveModalProps, SaveModalState> {
css: customCss,
dashboard_title:
saveType === SAVE_TYPE_NEWDASHBOARD ? newDashName : dashboardTitle,
- duplicate_slices: this.state.duplicateSlices,
+ duplicate_slices: duplicateSlices,
last_modified_time: lastModifiedTime,
owners: dashboardInfo.owners,
roles: dashboardInfo.roles,
metadata: {
...dashboardInfo?.metadata,
- positions,
- refresh_frequency: refreshFrequency,
+ positions: layout,
+ refresh_frequency: refreshFrequencyToUse,
},
};
if (saveType === SAVE_TYPE_NEWDASHBOARD && !newDashName) {
- this.props.addDangerToast(
- t('You must pick a name for the new dashboard'),
- );
+ addDangerToast(t('You must pick a name for the new dashboard'));
} else {
- this.onSave(data, dashboardId, saveType).then((resp: JsonResponse) => {
- if (saveType === SAVE_TYPE_NEWDASHBOARD && resp.json?.result?.id) {
- navigateTo(`/superset/dashboard/${resp.json.result.id}/`);
- }
- });
- this.modal?.current?.close?.();
+ onSave(data, dashboardId, saveType);
+ modal?.current?.close?.();
}
- }
-
- render() {
- const { theme } = themeObject;
- return (
- <ModalTrigger
- ref={this.modal}
- triggerNode={this.props.triggerNode}
- modalTitle={t('Save dashboard')}
- modalBody={
- <Form layout="vertical">
- <Form.Item>
- <Radio
- value={SAVE_TYPE_OVERWRITE}
- onChange={this.handleSaveTypeChange}
- checked={this.state.saveType === SAVE_TYPE_OVERWRITE}
- disabled={!this.props.canOverwrite}
- >
- {t('Overwrite Dashboard [%s]', this.props.dashboardTitle)}
- </Radio>
- </Form.Item>
-
- <Divider />
-
- <Form.Item style={{ marginBottom: theme.sizeUnit }}>
- <Radio
- value={SAVE_TYPE_NEWDASHBOARD}
- onChange={this.handleSaveTypeChange}
- checked={this.state.saveType === SAVE_TYPE_NEWDASHBOARD}
- >
- {t('Save as:')}
- </Radio>
- </Form.Item>
-
- <Form.Item style={{ marginBottom: theme.sizeUnit }}>
- <Input
- placeholder={t('[dashboard name]')}
- value={this.state.newDashName}
- onFocus={e => this.handleNameChange(e.target.value)}
- onChange={e => this.handleNameChange(e.target.value)}
- />
- </Form.Item>
-
- <Form.Item>
- <Checkbox
- checked={this.state.duplicateSlices}
- onChange={() => this.toggleDuplicateSlices()}
- >
- {t('also copy (duplicate) charts')}
- </Checkbox>
- </Form.Item>
- </Form>
- }
- modalFooter={
- <Space style={{ display: 'flex', justifyContent: 'flex-end' }}>
- <Button
- type="primary"
- data-test="modal-save-dashboard-button"
- onClick={this.saveDashboard}
+ };
+
+ return (
+ <ModalTrigger
+ ref={modal}
+ triggerNode={triggerNode}
+ modalTitle={t('Save dashboard')}
+ modalBody={
+ <Form layout="vertical">
+ <Form.Item>
+ <Radio
+ value={SAVE_TYPE_OVERWRITE}
+ onChange={handleSaveTypeChange}
+ checked={saveType === SAVE_TYPE_OVERWRITE}
+ disabled={!canOverwrite}
+ >
+ {t('Overwrite Dashboard [%s]', dashboardTitle)}
+ </Radio>
+ </Form.Item>
+
+ <Divider />
+
+ <Form.Item style={{ marginBottom: theme.sizeUnit }}>
+ <Radio
+ value={SAVE_TYPE_NEWDASHBOARD}
+ onChange={handleSaveTypeChange}
+ checked={saveType === SAVE_TYPE_NEWDASHBOARD}
+ >
+ {t('Save as:')}
+ </Radio>
+ </Form.Item>
+
+ <Form.Item style={{ marginBottom: theme.sizeUnit }}>
+ <Input
+ placeholder={t('[dashboard name]')}
+ value={newDashName}
+ onFocus={e => handleNameChange(e.target.value)}
+ onChange={e => handleNameChange(e.target.value)}
+ />
+ </Form.Item>
+
+ <Form.Item>
+ <Checkbox
+ checked={duplicateSlices}
+ onChange={() => toggleDuplicateSlices()}
>
- {t('Save')}
- </Button>
- </Space>
- }
- />
- );
- }
+ {t('also copy (duplicate) charts')}
+ </Checkbox>
+ </Form.Item>
+ </Form>
+ }
+ modalFooter={
+ <Space style={{ display: 'flex', justifyContent: 'flex-end' }}>
Review Comment:
```suggestion
<Flex justify="flex-end">
```
##########
superset-frontend/src/features/themes/ThemeModal.tsx:
##########
@@ -0,0 +1,394 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FunctionComponent, useState, useEffect, ChangeEvent } from 'react';
+
+import { css, styled, t, useTheme } from '@superset-ui/core';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import {
+ Input,
+ Modal,
+ JsonEditor,
+ Button,
+ Form,
+ Tooltip,
+ Alert,
+} from '@superset-ui/core/components';
+import { Typography } from '@superset-ui/core/components/Typography';
+
+import { OnlyKeyWithType } from 'src/utils/types';
+import { ThemeObject } from './types';
+
+interface ThemeModalProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast?: (msg: string) => void;
+ theme?: ThemeObject | null;
+ onThemeAdd?: (theme?: ThemeObject) => void;
+ onHide: () => void;
+ show: boolean;
+ canDevelop?: boolean;
+}
+
+type ThemeStringKeys = keyof Pick<
+ ThemeObject,
+ OnlyKeyWithType<ThemeObject, String>
+>;
+
+const StyledJsonEditor = styled.div`
+ ${({ theme }) => css`
+ border-radius: ${theme.borderRadius}px;
+ border: 1px solid ${theme.colorBorder};
+
+ .ace_editor {
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const ThemeModal: FunctionComponent<ThemeModalProps> = ({
+ addDangerToast,
+ addSuccessToast,
+ onThemeAdd,
+ onHide,
+ show,
+ theme = null,
+ canDevelop = false,
+}) => {
+ const supersetTheme = useTheme();
+ const { setTemporaryTheme } = useThemeContext();
+ const [disableSave, setDisableSave] = useState<boolean>(true);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [isHidden, setIsHidden] = useState<boolean>(true);
+ const isEditMode = theme !== null;
+ const isSystemTheme = currentTheme?.is_system === true;
+ const isReadOnly = isSystemTheme;
+ const canDevelopThemes = canDevelop;
+
+ // theme fetch logic
+ const {
+ state: { loading, resource },
+ fetchResource,
+ createResource,
+ updateResource,
+ } = useSingleViewResource<ThemeObject>('theme', t('theme'), addDangerToast);
+
+ // Functions
+ const hide = () => {
+ onHide();
+ setCurrentTheme(null);
+ };
+
+ const onSave = () => {
+ if (isEditMode) {
+ // Edit
+ if (currentTheme?.id) {
+ const update_id = currentTheme.id;
+ delete currentTheme.id;
+ delete currentTheme.created_by;
+ delete currentTheme.changed_by;
+ delete currentTheme.changed_on_delta_humanized;
+
+ updateResource(update_id, currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ } else if (currentTheme) {
+ // Create
+ createResource(currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ };
+
+ const onApply = () => {
+ if (currentTheme?.json_data && isValidJson(currentTheme.json_data)) {
+ try {
+ const themeConfig = JSON.parse(currentTheme.json_data);
+ setTemporaryTheme(themeConfig);
+ if (addSuccessToast) {
+ addSuccessToast(t('Local theme set for preview'));
+ }
+ } catch (error) {
+ addDangerToast(t('Failed to apply theme: Invalid JSON'));
+ }
+ }
+ };
+
+ const onThemeNameChange = (event: ChangeEvent<HTMLInputElement>) => {
+ const { target } = event;
+
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: currentTheme ? currentTheme.json_data : '',
+ };
+
+ data[target.name as ThemeStringKeys] = target.value;
+ setCurrentTheme(data);
+ };
+
+ const onJsonDataChange = (jsonData: string) => {
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: jsonData,
+ };
+ setCurrentTheme(data);
+ };
+
+ const isValidJson = (str?: string) => {
+ if (!str) return false;
+ try {
+ JSON.parse(str);
+ return true;
+ } catch (e) {
+ return false;
+ }
+ };
+
+ const validate = () => {
+ if (isReadOnly) {
+ setDisableSave(true);
+ return;
+ }
+
+ if (
+ currentTheme?.theme_name.length &&
+ currentTheme?.json_data?.length &&
+ isValidJson(currentTheme.json_data)
+ ) {
+ setDisableSave(false);
+ } else {
+ setDisableSave(true);
+ }
+ };
+
+ // Initialize
+ useEffect(() => {
+ if (
+ isEditMode &&
+ (!currentTheme?.id ||
+ (theme && theme?.id !== currentTheme.id) ||
+ (isHidden && show))
+ ) {
+ if (theme?.id && !loading) {
+ fetchResource(theme.id);
+ }
+ } else if (
+ !isEditMode &&
+ (!currentTheme || currentTheme.id || (isHidden && show))
+ ) {
+ setCurrentTheme({
+ theme_name: '',
+ json_data: JSON.stringify({}, null, 2),
+ });
+ }
+ }, [theme, show]);
+
+ useEffect(() => {
+ if (resource) {
+ setCurrentTheme(resource);
+ }
+ }, [resource]);
+
+ // Validation
+ useEffect(() => {
+ validate();
+ }, [
+ currentTheme ? currentTheme.theme_name : '',
+ currentTheme ? currentTheme.json_data : '',
+ isReadOnly,
+ ]);
+
+ // Show/hide
+ if (isHidden && show) {
+ setIsHidden(false);
+ }
+
+ return (
+ <Modal
+ disablePrimaryButton={isReadOnly || disableSave}
+ onHandledPrimaryAction={isReadOnly ? undefined : onSave}
+ onHide={hide}
+ primaryButtonName={isEditMode ? t('Save') : t('Add')}
+ show={show}
+ width="55%"
+ footer={[
+ <Button key="cancel" onClick={hide} buttonStyle="secondary">
+ {isReadOnly ? t('Close') : t('Cancel')}
+ </Button>,
+ ...(!isReadOnly
+ ? [
+ <Button
+ key="save"
+ onClick={onSave}
+ disabled={disableSave}
+ buttonStyle="primary"
+ >
+ {isEditMode ? t('Save') : t('Add')}
+ </Button>,
+ ]
+ : []),
+ ]}
+ title={
+ <Typography.Title level={4} data-test="theme-modal-title">
+ {isEditMode ? (
+ <Icons.EditOutlined
+ iconSize="l"
+ css={css`
+ margin: auto ${supersetTheme.sizeUnit * 2}px auto 0;
+ `}
+ />
+ ) : (
+ <Icons.PlusOutlined
+ iconSize="l"
+ css={css`
+ margin: auto ${supersetTheme.sizeUnit * 2}px auto 0;
+ `}
+ />
+ )}
+ {isEditMode
+ ? isReadOnly
+ ? t('View theme properties')
+ : t('Edit theme properties')
+ : t('Add theme')}
+ </Typography.Title>
+ }
+ >
+ <Form layout="vertical">
+ {isSystemTheme && (
+ <Typography.Text
+ type="secondary"
+ style={{
+ fontSize: '14px',
+ fontStyle: 'italic',
+ marginBottom: '16px',
+ display: 'block',
+ }}
+ >
+ {t('System Theme - Read Only')}
+ </Typography.Text>
+ )}
+
+ <Form.Item
+ label={t('Name')}
+ required={!isReadOnly}
+ style={{ marginBottom: '24px' }}
+ >
+ <Input
+ name="theme_name"
+ onChange={onThemeNameChange}
+ value={currentTheme?.theme_name}
+ readOnly={isReadOnly}
+ placeholder={t('Enter theme name')}
+ />
+ </Form.Item>
+
+ <Alert
+ type="info"
+ showIcon
+ closable={false}
+ message={
+ <span>
+ {t('Design with')}{' '}
+ <a
+ href="https://ant.design/theme-editor"
+ target="_blank"
+ rel="noopener noreferrer"
+ >
+ {t('Ant Design Theme Editor')}
+ </a>
+ {t(', then paste the JSON below. See our')}{' '}
+ <a
+ href="https://superset.apache.org/docs/configuration/theming/"
+ target="_blank"
+ rel="noopener noreferrer"
+ >
+ {t('documentation')}
+ </a>
Review Comment:
is it better to use `Typography.Link` here?
##########
superset-frontend/src/features/themes/ThemeModal.tsx:
##########
@@ -0,0 +1,394 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FunctionComponent, useState, useEffect, ChangeEvent } from 'react';
+
+import { css, styled, t, useTheme } from '@superset-ui/core';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import {
+ Input,
+ Modal,
+ JsonEditor,
+ Button,
+ Form,
+ Tooltip,
+ Alert,
+} from '@superset-ui/core/components';
+import { Typography } from '@superset-ui/core/components/Typography';
+
+import { OnlyKeyWithType } from 'src/utils/types';
+import { ThemeObject } from './types';
+
+interface ThemeModalProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast?: (msg: string) => void;
+ theme?: ThemeObject | null;
+ onThemeAdd?: (theme?: ThemeObject) => void;
+ onHide: () => void;
+ show: boolean;
+ canDevelop?: boolean;
+}
+
+type ThemeStringKeys = keyof Pick<
+ ThemeObject,
+ OnlyKeyWithType<ThemeObject, String>
+>;
+
+const StyledJsonEditor = styled.div`
+ ${({ theme }) => css`
+ border-radius: ${theme.borderRadius}px;
+ border: 1px solid ${theme.colorBorder};
+
+ .ace_editor {
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const ThemeModal: FunctionComponent<ThemeModalProps> = ({
+ addDangerToast,
+ addSuccessToast,
+ onThemeAdd,
+ onHide,
+ show,
+ theme = null,
+ canDevelop = false,
+}) => {
+ const supersetTheme = useTheme();
+ const { setTemporaryTheme } = useThemeContext();
+ const [disableSave, setDisableSave] = useState<boolean>(true);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [isHidden, setIsHidden] = useState<boolean>(true);
+ const isEditMode = theme !== null;
+ const isSystemTheme = currentTheme?.is_system === true;
+ const isReadOnly = isSystemTheme;
+ const canDevelopThemes = canDevelop;
+
+ // theme fetch logic
+ const {
+ state: { loading, resource },
+ fetchResource,
+ createResource,
+ updateResource,
+ } = useSingleViewResource<ThemeObject>('theme', t('theme'), addDangerToast);
+
+ // Functions
+ const hide = () => {
+ onHide();
+ setCurrentTheme(null);
+ };
+
+ const onSave = () => {
+ if (isEditMode) {
+ // Edit
+ if (currentTheme?.id) {
+ const update_id = currentTheme.id;
+ delete currentTheme.id;
+ delete currentTheme.created_by;
+ delete currentTheme.changed_by;
+ delete currentTheme.changed_on_delta_humanized;
+
+ updateResource(update_id, currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ } else if (currentTheme) {
+ // Create
+ createResource(currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ };
+
+ const onApply = () => {
+ if (currentTheme?.json_data && isValidJson(currentTheme.json_data)) {
+ try {
+ const themeConfig = JSON.parse(currentTheme.json_data);
+ setTemporaryTheme(themeConfig);
+ if (addSuccessToast) {
+ addSuccessToast(t('Local theme set for preview'));
+ }
+ } catch (error) {
+ addDangerToast(t('Failed to apply theme: Invalid JSON'));
+ }
+ }
+ };
+
+ const onThemeNameChange = (event: ChangeEvent<HTMLInputElement>) => {
+ const { target } = event;
+
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: currentTheme ? currentTheme.json_data : '',
+ };
+
+ data[target.name as ThemeStringKeys] = target.value;
+ setCurrentTheme(data);
+ };
+
+ const onJsonDataChange = (jsonData: string) => {
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: jsonData,
+ };
+ setCurrentTheme(data);
+ };
+
+ const isValidJson = (str?: string) => {
+ if (!str) return false;
+ try {
+ JSON.parse(str);
+ return true;
+ } catch (e) {
+ return false;
+ }
+ };
+
+ const validate = () => {
+ if (isReadOnly) {
+ setDisableSave(true);
+ return;
+ }
+
+ if (
+ currentTheme?.theme_name.length &&
+ currentTheme?.json_data?.length &&
+ isValidJson(currentTheme.json_data)
+ ) {
+ setDisableSave(false);
+ } else {
+ setDisableSave(true);
+ }
+ };
+
+ // Initialize
+ useEffect(() => {
+ if (
+ isEditMode &&
+ (!currentTheme?.id ||
+ (theme && theme?.id !== currentTheme.id) ||
+ (isHidden && show))
+ ) {
+ if (theme?.id && !loading) {
+ fetchResource(theme.id);
+ }
+ } else if (
+ !isEditMode &&
+ (!currentTheme || currentTheme.id || (isHidden && show))
+ ) {
+ setCurrentTheme({
+ theme_name: '',
+ json_data: JSON.stringify({}, null, 2),
+ });
+ }
+ }, [theme, show]);
+
+ useEffect(() => {
+ if (resource) {
+ setCurrentTheme(resource);
+ }
+ }, [resource]);
+
+ // Validation
+ useEffect(() => {
+ validate();
+ }, [
+ currentTheme ? currentTheme.theme_name : '',
+ currentTheme ? currentTheme.json_data : '',
+ isReadOnly,
+ ]);
+
+ // Show/hide
+ if (isHidden && show) {
+ setIsHidden(false);
+ }
+
+ return (
+ <Modal
+ disablePrimaryButton={isReadOnly || disableSave}
+ onHandledPrimaryAction={isReadOnly ? undefined : onSave}
+ onHide={hide}
+ primaryButtonName={isEditMode ? t('Save') : t('Add')}
+ show={show}
+ width="55%"
+ footer={[
+ <Button key="cancel" onClick={hide} buttonStyle="secondary">
+ {isReadOnly ? t('Close') : t('Cancel')}
+ </Button>,
+ ...(!isReadOnly
+ ? [
+ <Button
+ key="save"
+ onClick={onSave}
+ disabled={disableSave}
+ buttonStyle="primary"
+ >
+ {isEditMode ? t('Save') : t('Add')}
+ </Button>,
+ ]
+ : []),
+ ]}
+ title={
+ <Typography.Title level={4} data-test="theme-modal-title">
+ {isEditMode ? (
+ <Icons.EditOutlined
+ iconSize="l"
+ css={css`
+ margin: auto ${supersetTheme.sizeUnit * 2}px auto 0;
+ `}
+ />
+ ) : (
+ <Icons.PlusOutlined
+ iconSize="l"
+ css={css`
+ margin: auto ${supersetTheme.sizeUnit * 2}px auto 0;
+ `}
+ />
+ )}
+ {isEditMode
+ ? isReadOnly
+ ? t('View theme properties')
+ : t('Edit theme properties')
+ : t('Add theme')}
+ </Typography.Title>
+ }
+ >
+ <Form layout="vertical">
+ {isSystemTheme && (
+ <Typography.Text
+ type="secondary"
+ style={{
+ fontSize: '14px',
+ fontStyle: 'italic',
+ marginBottom: '16px',
+ display: 'block',
+ }}
+ >
+ {t('System Theme - Read Only')}
+ </Typography.Text>
+ )}
+
+ <Form.Item
+ label={t('Name')}
+ required={!isReadOnly}
+ style={{ marginBottom: '24px' }}
Review Comment:
Do you think we should use `css` with `theme.sizeUnit` here?
##########
superset-frontend/src/features/themes/ThemeModal.tsx:
##########
@@ -0,0 +1,394 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FunctionComponent, useState, useEffect, ChangeEvent } from 'react';
+
+import { css, styled, t, useTheme } from '@superset-ui/core';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import {
+ Input,
+ Modal,
+ JsonEditor,
+ Button,
+ Form,
+ Tooltip,
+ Alert,
+} from '@superset-ui/core/components';
+import { Typography } from '@superset-ui/core/components/Typography';
+
+import { OnlyKeyWithType } from 'src/utils/types';
+import { ThemeObject } from './types';
+
+interface ThemeModalProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast?: (msg: string) => void;
+ theme?: ThemeObject | null;
+ onThemeAdd?: (theme?: ThemeObject) => void;
+ onHide: () => void;
+ show: boolean;
+ canDevelop?: boolean;
+}
+
+type ThemeStringKeys = keyof Pick<
+ ThemeObject,
+ OnlyKeyWithType<ThemeObject, String>
+>;
+
+const StyledJsonEditor = styled.div`
+ ${({ theme }) => css`
+ border-radius: ${theme.borderRadius}px;
+ border: 1px solid ${theme.colorBorder};
+
+ .ace_editor {
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const ThemeModal: FunctionComponent<ThemeModalProps> = ({
+ addDangerToast,
+ addSuccessToast,
+ onThemeAdd,
+ onHide,
+ show,
+ theme = null,
+ canDevelop = false,
+}) => {
+ const supersetTheme = useTheme();
+ const { setTemporaryTheme } = useThemeContext();
+ const [disableSave, setDisableSave] = useState<boolean>(true);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [isHidden, setIsHidden] = useState<boolean>(true);
+ const isEditMode = theme !== null;
+ const isSystemTheme = currentTheme?.is_system === true;
+ const isReadOnly = isSystemTheme;
+ const canDevelopThemes = canDevelop;
+
+ // theme fetch logic
+ const {
+ state: { loading, resource },
+ fetchResource,
+ createResource,
+ updateResource,
+ } = useSingleViewResource<ThemeObject>('theme', t('theme'), addDangerToast);
+
+ // Functions
+ const hide = () => {
+ onHide();
+ setCurrentTheme(null);
+ };
+
+ const onSave = () => {
+ if (isEditMode) {
+ // Edit
+ if (currentTheme?.id) {
+ const update_id = currentTheme.id;
+ delete currentTheme.id;
+ delete currentTheme.created_by;
+ delete currentTheme.changed_by;
+ delete currentTheme.changed_on_delta_humanized;
+
+ updateResource(update_id, currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ } else if (currentTheme) {
+ // Create
+ createResource(currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ };
+
+ const onApply = () => {
+ if (currentTheme?.json_data && isValidJson(currentTheme.json_data)) {
+ try {
+ const themeConfig = JSON.parse(currentTheme.json_data);
+ setTemporaryTheme(themeConfig);
+ if (addSuccessToast) {
+ addSuccessToast(t('Local theme set for preview'));
+ }
+ } catch (error) {
+ addDangerToast(t('Failed to apply theme: Invalid JSON'));
+ }
+ }
+ };
+
+ const onThemeNameChange = (event: ChangeEvent<HTMLInputElement>) => {
+ const { target } = event;
+
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: currentTheme ? currentTheme.json_data : '',
+ };
+
+ data[target.name as ThemeStringKeys] = target.value;
+ setCurrentTheme(data);
+ };
+
+ const onJsonDataChange = (jsonData: string) => {
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: jsonData,
+ };
+ setCurrentTheme(data);
+ };
+
+ const isValidJson = (str?: string) => {
+ if (!str) return false;
+ try {
+ JSON.parse(str);
+ return true;
+ } catch (e) {
+ return false;
+ }
+ };
+
+ const validate = () => {
+ if (isReadOnly) {
+ setDisableSave(true);
+ return;
+ }
+
+ if (
+ currentTheme?.theme_name.length &&
+ currentTheme?.json_data?.length &&
+ isValidJson(currentTheme.json_data)
+ ) {
+ setDisableSave(false);
+ } else {
+ setDisableSave(true);
+ }
+ };
+
+ // Initialize
+ useEffect(() => {
+ if (
+ isEditMode &&
+ (!currentTheme?.id ||
+ (theme && theme?.id !== currentTheme.id) ||
+ (isHidden && show))
+ ) {
+ if (theme?.id && !loading) {
+ fetchResource(theme.id);
+ }
+ } else if (
+ !isEditMode &&
+ (!currentTheme || currentTheme.id || (isHidden && show))
+ ) {
+ setCurrentTheme({
+ theme_name: '',
+ json_data: JSON.stringify({}, null, 2),
+ });
+ }
+ }, [theme, show]);
+
+ useEffect(() => {
+ if (resource) {
+ setCurrentTheme(resource);
+ }
+ }, [resource]);
+
+ // Validation
+ useEffect(() => {
+ validate();
+ }, [
+ currentTheme ? currentTheme.theme_name : '',
+ currentTheme ? currentTheme.json_data : '',
+ isReadOnly,
+ ]);
+
+ // Show/hide
+ if (isHidden && show) {
+ setIsHidden(false);
+ }
+
+ return (
+ <Modal
+ disablePrimaryButton={isReadOnly || disableSave}
+ onHandledPrimaryAction={isReadOnly ? undefined : onSave}
+ onHide={hide}
+ primaryButtonName={isEditMode ? t('Save') : t('Add')}
+ show={show}
+ width="55%"
+ footer={[
+ <Button key="cancel" onClick={hide} buttonStyle="secondary">
+ {isReadOnly ? t('Close') : t('Cancel')}
+ </Button>,
+ ...(!isReadOnly
+ ? [
+ <Button
+ key="save"
+ onClick={onSave}
+ disabled={disableSave}
+ buttonStyle="primary"
+ >
+ {isEditMode ? t('Save') : t('Add')}
+ </Button>,
+ ]
+ : []),
+ ]}
+ title={
+ <Typography.Title level={4} data-test="theme-modal-title">
+ {isEditMode ? (
+ <Icons.EditOutlined
+ iconSize="l"
+ css={css`
+ margin: auto ${supersetTheme.sizeUnit * 2}px auto 0;
+ `}
+ />
+ ) : (
+ <Icons.PlusOutlined
+ iconSize="l"
+ css={css`
+ margin: auto ${supersetTheme.sizeUnit * 2}px auto 0;
+ `}
+ />
+ )}
+ {isEditMode
+ ? isReadOnly
+ ? t('View theme properties')
+ : t('Edit theme properties')
+ : t('Add theme')}
+ </Typography.Title>
+ }
+ >
+ <Form layout="vertical">
+ {isSystemTheme && (
+ <Typography.Text
+ type="secondary"
+ style={{
+ fontSize: '14px',
+ fontStyle: 'italic',
+ marginBottom: '16px',
Review Comment:
Do we need these styles here?
##########
superset-frontend/src/features/themes/ThemeModal.tsx:
##########
@@ -0,0 +1,394 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FunctionComponent, useState, useEffect, ChangeEvent } from 'react';
+
+import { css, styled, t, useTheme } from '@superset-ui/core';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import {
+ Input,
+ Modal,
+ JsonEditor,
+ Button,
+ Form,
+ Tooltip,
+ Alert,
+} from '@superset-ui/core/components';
+import { Typography } from '@superset-ui/core/components/Typography';
+
+import { OnlyKeyWithType } from 'src/utils/types';
+import { ThemeObject } from './types';
+
+interface ThemeModalProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast?: (msg: string) => void;
+ theme?: ThemeObject | null;
+ onThemeAdd?: (theme?: ThemeObject) => void;
+ onHide: () => void;
+ show: boolean;
+ canDevelop?: boolean;
+}
+
+type ThemeStringKeys = keyof Pick<
+ ThemeObject,
+ OnlyKeyWithType<ThemeObject, String>
+>;
+
+const StyledJsonEditor = styled.div`
+ ${({ theme }) => css`
+ border-radius: ${theme.borderRadius}px;
+ border: 1px solid ${theme.colorBorder};
+
+ .ace_editor {
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const ThemeModal: FunctionComponent<ThemeModalProps> = ({
+ addDangerToast,
+ addSuccessToast,
+ onThemeAdd,
+ onHide,
+ show,
+ theme = null,
+ canDevelop = false,
+}) => {
+ const supersetTheme = useTheme();
+ const { setTemporaryTheme } = useThemeContext();
+ const [disableSave, setDisableSave] = useState<boolean>(true);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [isHidden, setIsHidden] = useState<boolean>(true);
+ const isEditMode = theme !== null;
+ const isSystemTheme = currentTheme?.is_system === true;
+ const isReadOnly = isSystemTheme;
+ const canDevelopThemes = canDevelop;
+
+ // theme fetch logic
+ const {
+ state: { loading, resource },
+ fetchResource,
+ createResource,
+ updateResource,
+ } = useSingleViewResource<ThemeObject>('theme', t('theme'), addDangerToast);
+
+ // Functions
+ const hide = () => {
+ onHide();
+ setCurrentTheme(null);
+ };
+
+ const onSave = () => {
+ if (isEditMode) {
+ // Edit
+ if (currentTheme?.id) {
+ const update_id = currentTheme.id;
+ delete currentTheme.id;
+ delete currentTheme.created_by;
+ delete currentTheme.changed_by;
+ delete currentTheme.changed_on_delta_humanized;
+
+ updateResource(update_id, currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ } else if (currentTheme) {
+ // Create
+ createResource(currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ };
+
+ const onApply = () => {
+ if (currentTheme?.json_data && isValidJson(currentTheme.json_data)) {
+ try {
+ const themeConfig = JSON.parse(currentTheme.json_data);
+ setTemporaryTheme(themeConfig);
+ if (addSuccessToast) {
+ addSuccessToast(t('Local theme set for preview'));
+ }
+ } catch (error) {
+ addDangerToast(t('Failed to apply theme: Invalid JSON'));
+ }
+ }
+ };
+
+ const onThemeNameChange = (event: ChangeEvent<HTMLInputElement>) => {
+ const { target } = event;
+
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: currentTheme ? currentTheme.json_data : '',
+ };
+
+ data[target.name as ThemeStringKeys] = target.value;
+ setCurrentTheme(data);
+ };
+
+ const onJsonDataChange = (jsonData: string) => {
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: jsonData,
+ };
+ setCurrentTheme(data);
+ };
+
+ const isValidJson = (str?: string) => {
+ if (!str) return false;
+ try {
+ JSON.parse(str);
+ return true;
+ } catch (e) {
+ return false;
+ }
+ };
+
+ const validate = () => {
+ if (isReadOnly) {
+ setDisableSave(true);
+ return;
+ }
+
+ if (
+ currentTheme?.theme_name.length &&
+ currentTheme?.json_data?.length &&
+ isValidJson(currentTheme.json_data)
+ ) {
+ setDisableSave(false);
+ } else {
+ setDisableSave(true);
+ }
+ };
+
+ // Initialize
+ useEffect(() => {
+ if (
+ isEditMode &&
+ (!currentTheme?.id ||
+ (theme && theme?.id !== currentTheme.id) ||
+ (isHidden && show))
+ ) {
+ if (theme?.id && !loading) {
+ fetchResource(theme.id);
+ }
+ } else if (
+ !isEditMode &&
+ (!currentTheme || currentTheme.id || (isHidden && show))
+ ) {
+ setCurrentTheme({
+ theme_name: '',
+ json_data: JSON.stringify({}, null, 2),
+ });
+ }
+ }, [theme, show]);
+
+ useEffect(() => {
+ if (resource) {
+ setCurrentTheme(resource);
+ }
+ }, [resource]);
+
+ // Validation
+ useEffect(() => {
+ validate();
+ }, [
+ currentTheme ? currentTheme.theme_name : '',
+ currentTheme ? currentTheme.json_data : '',
+ isReadOnly,
+ ]);
+
+ // Show/hide
+ if (isHidden && show) {
+ setIsHidden(false);
+ }
+
+ return (
+ <Modal
+ disablePrimaryButton={isReadOnly || disableSave}
+ onHandledPrimaryAction={isReadOnly ? undefined : onSave}
+ onHide={hide}
+ primaryButtonName={isEditMode ? t('Save') : t('Add')}
+ show={show}
+ width="55%"
+ footer={[
+ <Button key="cancel" onClick={hide} buttonStyle="secondary">
+ {isReadOnly ? t('Close') : t('Cancel')}
+ </Button>,
+ ...(!isReadOnly
+ ? [
+ <Button
+ key="save"
+ onClick={onSave}
+ disabled={disableSave}
+ buttonStyle="primary"
+ >
+ {isEditMode ? t('Save') : t('Add')}
+ </Button>,
+ ]
+ : []),
+ ]}
+ title={
+ <Typography.Title level={4} data-test="theme-modal-title">
+ {isEditMode ? (
+ <Icons.EditOutlined
+ iconSize="l"
+ css={css`
+ margin: auto ${supersetTheme.sizeUnit * 2}px auto 0;
+ `}
+ />
+ ) : (
+ <Icons.PlusOutlined
+ iconSize="l"
+ css={css`
+ margin: auto ${supersetTheme.sizeUnit * 2}px auto 0;
+ `}
+ />
+ )}
+ {isEditMode
+ ? isReadOnly
+ ? t('View theme properties')
+ : t('Edit theme properties')
+ : t('Add theme')}
+ </Typography.Title>
+ }
+ >
+ <Form layout="vertical">
+ {isSystemTheme && (
+ <Typography.Text
+ type="secondary"
+ style={{
+ fontSize: '14px',
+ fontStyle: 'italic',
+ marginBottom: '16px',
+ display: 'block',
+ }}
+ >
+ {t('System Theme - Read Only')}
+ </Typography.Text>
+ )}
+
+ <Form.Item
+ label={t('Name')}
+ required={!isReadOnly}
+ style={{ marginBottom: '24px' }}
+ >
+ <Input
+ name="theme_name"
+ onChange={onThemeNameChange}
+ value={currentTheme?.theme_name}
+ readOnly={isReadOnly}
+ placeholder={t('Enter theme name')}
+ />
+ </Form.Item>
+
+ <Alert
+ type="info"
+ showIcon
+ closable={false}
+ message={
+ <span>
+ {t('Design with')}{' '}
+ <a
+ href="https://ant.design/theme-editor"
+ target="_blank"
+ rel="noopener noreferrer"
+ >
+ {t('Ant Design Theme Editor')}
+ </a>
+ {t(', then paste the JSON below. See our')}{' '}
+ <a
+ href="https://superset.apache.org/docs/configuration/theming/"
+ target="_blank"
+ rel="noopener noreferrer"
+ >
+ {t('documentation')}
+ </a>
+ {t(' for details.')}
+ </span>
+ }
+ style={{ marginBottom: '16px' }}
+ />
+
+ <Form.Item
+ label={t('JSON Configuration')}
+ required={!isReadOnly}
+ style={{ marginBottom: '24px' }}
Review Comment:
Should we add `Space` component between the items instead of using `margin`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2229160704
##########
superset-frontend/src/features/themes/ThemeModal.tsx:
##########
@@ -0,0 +1,394 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FunctionComponent, useState, useEffect, ChangeEvent } from 'react';
+
+import { css, styled, t, useTheme } from '@superset-ui/core';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import {
+ Input,
+ Modal,
+ JsonEditor,
+ Button,
+ Form,
+ Tooltip,
+ Alert,
+} from '@superset-ui/core/components';
+import { Typography } from '@superset-ui/core/components/Typography';
+
+import { OnlyKeyWithType } from 'src/utils/types';
+import { ThemeObject } from './types';
+
+interface ThemeModalProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast?: (msg: string) => void;
+ theme?: ThemeObject | null;
+ onThemeAdd?: (theme?: ThemeObject) => void;
+ onHide: () => void;
+ show: boolean;
+ canDevelop?: boolean;
+}
+
+type ThemeStringKeys = keyof Pick<
+ ThemeObject,
+ OnlyKeyWithType<ThemeObject, String>
+>;
+
+const StyledJsonEditor = styled.div`
+ ${({ theme }) => css`
+ border-radius: ${theme.borderRadius}px;
+ border: 1px solid ${theme.colorBorder};
+
+ .ace_editor {
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const ThemeModal: FunctionComponent<ThemeModalProps> = ({
+ addDangerToast,
+ addSuccessToast,
+ onThemeAdd,
+ onHide,
+ show,
+ theme = null,
+ canDevelop = false,
+}) => {
+ const supersetTheme = useTheme();
+ const { setTemporaryTheme } = useThemeContext();
+ const [disableSave, setDisableSave] = useState<boolean>(true);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [isHidden, setIsHidden] = useState<boolean>(true);
+ const isEditMode = theme !== null;
+ const isSystemTheme = currentTheme?.is_system === true;
+ const isReadOnly = isSystemTheme;
+ const canDevelopThemes = canDevelop;
+
+ // theme fetch logic
+ const {
+ state: { loading, resource },
+ fetchResource,
+ createResource,
+ updateResource,
+ } = useSingleViewResource<ThemeObject>('theme', t('theme'), addDangerToast);
+
+ // Functions
+ const hide = () => {
+ onHide();
+ setCurrentTheme(null);
+ };
+
+ const onSave = () => {
+ if (isEditMode) {
+ // Edit
+ if (currentTheme?.id) {
+ const update_id = currentTheme.id;
+ delete currentTheme.id;
+ delete currentTheme.created_by;
+ delete currentTheme.changed_by;
+ delete currentTheme.changed_on_delta_humanized;
+
+ updateResource(update_id, currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ } else if (currentTheme) {
+ // Create
+ createResource(currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ };
+
+ const onApply = () => {
+ if (currentTheme?.json_data && isValidJson(currentTheme.json_data)) {
+ try {
+ const themeConfig = JSON.parse(currentTheme.json_data);
+ setTemporaryTheme(themeConfig);
+ if (addSuccessToast) {
+ addSuccessToast(t('Local theme set for preview'));
+ }
+ } catch (error) {
+ addDangerToast(t('Failed to apply theme: Invalid JSON'));
+ }
+ }
+ };
+
+ const onThemeNameChange = (event: ChangeEvent<HTMLInputElement>) => {
+ const { target } = event;
+
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: currentTheme ? currentTheme.json_data : '',
+ };
+
+ data[target.name as ThemeStringKeys] = target.value;
+ setCurrentTheme(data);
+ };
+
+ const onJsonDataChange = (jsonData: string) => {
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: jsonData,
+ };
+ setCurrentTheme(data);
+ };
+
+ const isValidJson = (str?: string) => {
+ if (!str) return false;
+ try {
+ JSON.parse(str);
+ return true;
+ } catch (e) {
+ return false;
+ }
+ };
+
+ const validate = () => {
+ if (isReadOnly) {
+ setDisableSave(true);
+ return;
+ }
+
+ if (
+ currentTheme?.theme_name.length &&
+ currentTheme?.json_data?.length &&
+ isValidJson(currentTheme.json_data)
+ ) {
+ setDisableSave(false);
+ } else {
+ setDisableSave(true);
+ }
+ };
+
+ // Initialize
+ useEffect(() => {
+ if (
+ isEditMode &&
+ (!currentTheme?.id ||
+ (theme && theme?.id !== currentTheme.id) ||
+ (isHidden && show))
+ ) {
+ if (theme?.id && !loading) {
+ fetchResource(theme.id);
+ }
+ } else if (
+ !isEditMode &&
+ (!currentTheme || currentTheme.id || (isHidden && show))
+ ) {
+ setCurrentTheme({
+ theme_name: '',
+ json_data: JSON.stringify({}, null, 2),
+ });
+ }
+ }, [theme, show]);
+
+ useEffect(() => {
+ if (resource) {
+ setCurrentTheme(resource);
+ }
+ }, [resource]);
+
+ // Validation
+ useEffect(() => {
+ validate();
+ }, [
+ currentTheme ? currentTheme.theme_name : '',
+ currentTheme ? currentTheme.json_data : '',
+ isReadOnly,
+ ]);
+
+ // Show/hide
+ if (isHidden && show) {
+ setIsHidden(false);
+ }
+
+ return (
+ <Modal
+ disablePrimaryButton={isReadOnly || disableSave}
+ onHandledPrimaryAction={isReadOnly ? undefined : onSave}
+ onHide={hide}
+ primaryButtonName={isEditMode ? t('Save') : t('Add')}
+ show={show}
+ width="55%"
+ footer={[
+ <Button key="cancel" onClick={hide} buttonStyle="secondary">
+ {isReadOnly ? t('Close') : t('Cancel')}
+ </Button>,
+ ...(!isReadOnly
+ ? [
+ <Button
+ key="save"
+ onClick={onSave}
+ disabled={disableSave}
+ buttonStyle="primary"
+ >
+ {isEditMode ? t('Save') : t('Add')}
+ </Button>,
+ ]
+ : []),
+ ]}
+ title={
+ <Typography.Title level={4} data-test="theme-modal-title">
+ {isEditMode ? (
+ <Icons.EditOutlined
+ iconSize="l"
+ css={css`
+ margin: auto ${supersetTheme.sizeUnit * 2}px auto 0;
+ `}
+ />
+ ) : (
+ <Icons.PlusOutlined
+ iconSize="l"
+ css={css`
+ margin: auto ${supersetTheme.sizeUnit * 2}px auto 0;
+ `}
+ />
+ )}
+ {isEditMode
+ ? isReadOnly
+ ? t('View theme properties')
+ : t('Edit theme properties')
+ : t('Add theme')}
+ </Typography.Title>
+ }
+ >
+ <Form layout="vertical">
+ {isSystemTheme && (
+ <Typography.Text
+ type="secondary"
+ style={{
+ fontSize: '14px',
+ fontStyle: 'italic',
+ marginBottom: '16px',
Review Comment:
good catch ... let me fix the whole form
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3115066828
@mistercrunch Ephemeral environment spinning up at http://54.213.12.204:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2229839041
##########
superset-frontend/src/features/themes/ThemeModal.tsx:
##########
@@ -0,0 +1,394 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FunctionComponent, useState, useEffect, ChangeEvent } from 'react';
+
+import { css, styled, t, useTheme } from '@superset-ui/core';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import {
+ Input,
+ Modal,
+ JsonEditor,
+ Button,
+ Form,
+ Tooltip,
+ Alert,
+} from '@superset-ui/core/components';
+import { Typography } from '@superset-ui/core/components/Typography';
+
+import { OnlyKeyWithType } from 'src/utils/types';
+import { ThemeObject } from './types';
+
+interface ThemeModalProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast?: (msg: string) => void;
+ theme?: ThemeObject | null;
+ onThemeAdd?: (theme?: ThemeObject) => void;
+ onHide: () => void;
+ show: boolean;
+ canDevelop?: boolean;
+}
+
+type ThemeStringKeys = keyof Pick<
+ ThemeObject,
+ OnlyKeyWithType<ThemeObject, String>
+>;
+
+const StyledJsonEditor = styled.div`
+ ${({ theme }) => css`
+ border-radius: ${theme.borderRadius}px;
+ border: 1px solid ${theme.colorBorder};
+
+ .ace_editor {
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const ThemeModal: FunctionComponent<ThemeModalProps> = ({
+ addDangerToast,
+ addSuccessToast,
+ onThemeAdd,
+ onHide,
+ show,
+ theme = null,
+ canDevelop = false,
+}) => {
+ const supersetTheme = useTheme();
+ const { setTemporaryTheme } = useThemeContext();
+ const [disableSave, setDisableSave] = useState<boolean>(true);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [isHidden, setIsHidden] = useState<boolean>(true);
+ const isEditMode = theme !== null;
+ const isSystemTheme = currentTheme?.is_system === true;
+ const isReadOnly = isSystemTheme;
+ const canDevelopThemes = canDevelop;
+
+ // theme fetch logic
+ const {
+ state: { loading, resource },
+ fetchResource,
+ createResource,
+ updateResource,
+ } = useSingleViewResource<ThemeObject>('theme', t('theme'), addDangerToast);
+
+ // Functions
+ const hide = () => {
+ onHide();
+ setCurrentTheme(null);
+ };
+
+ const onSave = () => {
+ if (isEditMode) {
+ // Edit
+ if (currentTheme?.id) {
+ const update_id = currentTheme.id;
+ delete currentTheme.id;
+ delete currentTheme.created_by;
+ delete currentTheme.changed_by;
+ delete currentTheme.changed_on_delta_humanized;
+
+ updateResource(update_id, currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ } else if (currentTheme) {
+ // Create
+ createResource(currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ };
+
+ const onApply = () => {
+ if (currentTheme?.json_data && isValidJson(currentTheme.json_data)) {
+ try {
+ const themeConfig = JSON.parse(currentTheme.json_data);
+ setTemporaryTheme(themeConfig);
+ if (addSuccessToast) {
+ addSuccessToast(t('Local theme set for preview'));
+ }
+ } catch (error) {
+ addDangerToast(t('Failed to apply theme: Invalid JSON'));
+ }
+ }
+ };
+
+ const onThemeNameChange = (event: ChangeEvent<HTMLInputElement>) => {
+ const { target } = event;
+
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: currentTheme ? currentTheme.json_data : '',
+ };
+
+ data[target.name as ThemeStringKeys] = target.value;
+ setCurrentTheme(data);
+ };
+
+ const onJsonDataChange = (jsonData: string) => {
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: jsonData,
+ };
+ setCurrentTheme(data);
+ };
+
+ const isValidJson = (str?: string) => {
+ if (!str) return false;
+ try {
+ JSON.parse(str);
+ return true;
+ } catch (e) {
+ return false;
+ }
+ };
+
+ const validate = () => {
+ if (isReadOnly) {
+ setDisableSave(true);
+ return;
+ }
+
+ if (
+ currentTheme?.theme_name.length &&
+ currentTheme?.json_data?.length &&
+ isValidJson(currentTheme.json_data)
+ ) {
+ setDisableSave(false);
+ } else {
+ setDisableSave(true);
+ }
+ };
Review Comment:
Maybe we can show an error when this fails, i think having incorrect json is common enough.
##########
superset-frontend/src/pages/ThemeList/ThemeList.test.tsx:
##########
@@ -0,0 +1,243 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ render,
+ screen,
+ userEvent,
+ waitFor,
+} from 'spec/helpers/testing-library';
+import fetchMock from 'fetch-mock';
+import ThemesList from './index';
+
+const mockThemes = [
+ {
+ id: 1,
+ theme_name: 'Test Theme 1',
+ json_data: '{"colors": {"primary": "#1890ff"}}',
+ changed_on_delta_humanized: '1 day ago',
+ changed_by: {
+ first_name: 'John',
+ last_name: 'Doe',
+ },
+ },
+ {
+ id: 2,
+ theme_name: 'Test Theme 2',
+ json_data: '{"colors": {"primary": "#52c41a"}}',
+ changed_on_delta_humanized: '2 days ago',
+ changed_by: {
+ first_name: 'Jane',
+ last_name: 'Smith',
+ },
+ },
+];
+
+const mockUser = {
+ userId: 1,
+ firstName: 'Admin',
+ lastName: 'User',
+};
+
+// Mock the theme API endpoints
+fetchMock.get('glob:*/api/v1/theme/_info*', {
+ permissions: ['can_write', 'can_export', 'can_read'],
+});
+
+fetchMock.get('glob:*/api/v1/theme/*', {
+ result: mockThemes,
+ count: 2,
+});
+
+fetchMock.delete('glob:*/api/v1/theme/*', {
+ message: 'Theme deleted successfully',
+});
+
+fetchMock.get('glob:*/api/v1/theme/related/changed_by*', {
+ result: [
+ { text: 'John Doe', value: 1 },
+ { text: 'Jane Smith', value: 2 },
+ ],
+});
+
+const defaultProps = {
+ addDangerToast: jest.fn(),
+ addSuccessToast: jest.fn(),
+ user: mockUser,
+};
+
+const renderThemesList = (props = {}) =>
+ render(<ThemesList {...defaultProps} {...props} />, {
+ useRedux: true,
+ useTheme: true,
+ useQueryParams: true,
+ useRouter: true,
+ initialState: {
+ user: mockUser,
+ },
+ });
+
+describe('ThemesList', () => {
+ beforeEach(() => {
+ fetchMock.resetHistory();
+ });
+
+ afterEach(() => {
+ jest.clearAllMocks();
+ });
+
+ test('renders theme list with themes', async () => {
+ renderThemesList();
+
+ await waitFor(() => {
+ expect(screen.getByText('Test Theme 1')).toBeInTheDocument();
+ expect(screen.getByText('Test Theme 2')).toBeInTheDocument();
+ });
+ });
+
+ test('shows theme actions for themes', async () => {
+ renderThemesList();
+
+ await waitFor(() => {
+ expect(screen.getByText('Test Theme 1')).toBeInTheDocument();
+ });
+
+ // Should show apply, edit, export, and delete actions
+ const applyButtons = screen.getAllByTestId('apply-action');
+ const editButtons = screen.getAllByTestId('edit-action');
+ const exportButtons = screen.getAllByTestId('export-action');
+ const deleteButtons = screen.getAllByTestId('delete-action');
+
+ expect(applyButtons.length).toBeGreaterThan(0);
+ expect(editButtons.length).toBeGreaterThan(0);
+ expect(exportButtons.length).toBeGreaterThan(0);
+ expect(deleteButtons.length).toBeGreaterThan(0);
+ });
+
+ test('opens theme modal when clicking add theme button', async () => {
+ renderThemesList();
+
+ await waitFor(() => {
+ expect(screen.getByText('Test Theme 1')).toBeInTheDocument();
+ });
+
+ const addButton = screen.getByText('Theme');
+ userEvent.click(addButton);
+
+ await waitFor(() => {
+ expect(screen.getByText('Add theme')).toBeInTheDocument();
+ });
+ });
+
+ test('shows bulk select button when user has delete or export permissions', async () => {
+ renderThemesList();
+
+ await waitFor(() => {
+ expect(screen.getByText('Bulk select')).toBeInTheDocument();
+ });
+ });
+
+ test('filters themes by name', async () => {
+ renderThemesList();
+
+ await waitFor(() => {
+ expect(screen.getByText('Test Theme 1')).toBeInTheDocument();
+ });
+
+ // Look for the search input in the Name filter section
+ const searchInput = screen.getByRole('textbox');
+ userEvent.type(searchInput, 'Test Theme 1');
+
+ // The filtering is handled by the backend, so we just verify the input works
+ await waitFor(() => {
+ expect(searchInput).toHaveValue('Test Theme 1');
+ });
+ });
+
+ test('shows theme apply success message', async () => {
+ const mockAddSuccessToast = jest.fn();
+ renderThemesList({ addSuccessToast: mockAddSuccessToast });
+
+ await waitFor(() => {
+ expect(screen.getByText('Test Theme 1')).toBeInTheDocument();
+ });
+
+ // Mock the theme apply functionality
+ // This would typically involve clicking the apply button and verifying the toast
+ // The exact implementation depends on how the actions are rendered
+ });
+
+ test('shows delete confirmation modal', async () => {
+ renderThemesList();
+
+ await waitFor(() => {
+ expect(screen.getByText('Test Theme 1')).toBeInTheDocument();
+ });
+
+ // Find and click delete button (implementation depends on how actions are rendered)
+ // Should show confirmation modal
+ // This test would be more specific once we know the exact structure
+ });
+
+ test('handles theme export', async () => {
+ renderThemesList();
+
+ await waitFor(() => {
+ expect(screen.getByText('Test Theme 1')).toBeInTheDocument();
+ });
+
+ // Test export functionality
+ // This would involve clicking export and verifying the API call
+ });
+
+ test('handles bulk operations', async () => {
+ renderThemesList();
+
+ await waitFor(() => {
+ expect(screen.getByText('Test Theme 1')).toBeInTheDocument();
+ });
+
+ // Click bulk select
+ const bulkSelectButton = screen.getByText('Bulk select');
+ userEvent.click(bulkSelectButton);
+
+ // Verify bulk selection interface appears
+ // This depends on the exact implementation
+ });
+
+ test('respects permissions for actions', async () => {
+ // Test with limited permissions
+ fetchMock.get(
+ 'glob:*/api/v1/theme/_info*',
+ {
+ permissions: ['can_read'], // Only read permission
+ },
+ { overwriteRoutes: true },
+ );
+
+ renderThemesList();
+
+ await waitFor(() => {
+ expect(screen.getByText('Test Theme 1')).toBeInTheDocument();
+ });
+
+ // Should not show add button or bulk select
+ expect(screen.queryByText('Theme')).not.toBeInTheDocument();
+ expect(screen.queryByText('Bulk select')).not.toBeInTheDocument();
+ });
+});
Review Comment:
This whole suite is missing the actual testing steps
##########
superset-frontend/src/pages/ThemeList/index.tsx:
##########
@@ -0,0 +1,515 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { useMemo, useState } from 'react';
+import { t, SupersetClient } from '@superset-ui/core';
+import {
+ Tag,
+ DeleteModal,
+ ConfirmStatusChange,
+ Loading,
+ Alert,
+ Tooltip,
+} from '@superset-ui/core/components';
+
+import rison from 'rison';
+import { useListViewResource } from 'src/views/CRUD/hooks';
+import { createErrorHandler, createFetchRelated } from 'src/views/CRUD/utils';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu';
+import handleResourceExport from 'src/utils/export';
+import {
+ ModifiedInfo,
+ ListView,
+ ListViewActionsBar,
+ ListViewFilterOperator as FilterOperator,
+ ImportModal as ImportModelsModal,
+ type ListViewProps,
+ type ListViewActionProps,
+ type ListViewFilters,
+} from 'src/components';
+
+import ThemeModal from 'src/features/themes/ThemeModal';
+import { ThemeObject } from 'src/features/themes/types';
+import { QueryObjectColumns } from 'src/views/CRUD/types';
+import { Icons } from '@superset-ui/core/components/Icons';
+
+const PAGE_SIZE = 25;
+
+const CONFIRM_OVERWRITE_MESSAGE = t(
+ 'You are importing one or more themes that already exist. ' +
+ 'Overwriting might cause you to lose some of your work. Are you ' +
+ 'sure you want to overwrite?',
+);
+
+interface ThemesListProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string) => void;
+ user: {
+ userId: string | number;
+ firstName: string;
+ lastName: string;
+ };
+}
+
+function ThemesList({
+ addDangerToast,
+ addSuccessToast,
+ user,
+}: ThemesListProps) {
+ const {
+ state: {
+ loading,
+ resourceCount: themesCount,
+ resourceCollection: themes,
+ bulkSelectEnabled,
+ },
+ hasPerm,
+ fetchData,
+ refreshData,
+ toggleBulkSelect,
+ } = useListViewResource<ThemeObject>('theme', t('Themes'), addDangerToast);
+ const { setTemporaryTheme, getCurrentCrudThemeId } = useThemeContext();
+ const [themeModalOpen, setThemeModalOpen] = useState<boolean>(false);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [preparingExport, setPreparingExport] = useState<boolean>(false);
+ const [importingTheme, showImportModal] = useState<boolean>(false);
+ const [appliedThemeId, setAppliedThemeId] = useState<number | null>(null);
+
+ const canCreate = hasPerm('can_write');
+ const canEdit = hasPerm('can_write');
+ const canDelete = hasPerm('can_write');
+ const canExport = hasPerm('can_export');
+ const canImport = hasPerm('can_write');
+ const canApply = hasPerm('can_write'); // Only users with write permission can apply themes
+
+ const [themeCurrentlyDeleting, setThemeCurrentlyDeleting] =
+ useState<ThemeObject | null>(null);
+
+ const handleThemeDelete = ({ id, theme_name }: ThemeObject) => {
+ SupersetClient.delete({
+ endpoint: `/api/v1/theme/${id}`,
+ }).then(
+ () => {
+ refreshData();
+ setThemeCurrentlyDeleting(null);
+ addSuccessToast(t('Deleted: %s', theme_name));
+ },
+ createErrorHandler(errMsg =>
+ addDangerToast(
+ t('There was an issue deleting %s: %s', theme_name, errMsg),
+ ),
+ ),
+ );
+ };
+
+ const handleBulkThemeDelete = (themesToDelete: ThemeObject[]) => {
+ // Filter out system themes from deletion
+ const deletableThemes = themesToDelete.filter(theme => !theme.is_system);
+
+ if (deletableThemes.length === 0) {
+ addDangerToast(t('Cannot delete system themes'));
+ return;
+ }
+
+ if (deletableThemes.length !== themesToDelete.length) {
+ addDangerToast(
+ t(
+ 'Skipped %d system themes that cannot be deleted',
+ themesToDelete.length - deletableThemes.length,
+ ),
+ );
+ }
+
+ SupersetClient.delete({
+ endpoint: `/api/v1/theme/?q=${rison.encode(
+ deletableThemes.map(({ id }) => id),
+ )}`,
+ }).then(
+ ({ json = {} }) => {
+ refreshData();
+ addSuccessToast(json.message);
+ },
+ createErrorHandler(errMsg =>
+ addDangerToast(
+ t('There was an issue deleting the selected themes: %s', errMsg),
+ ),
+ ),
+ );
+ };
+
+ function handleThemeEdit(themeObj: ThemeObject) {
+ setCurrentTheme(themeObj);
+ setThemeModalOpen(true);
+ }
+
+ function handleThemeApply(themeObj: ThemeObject) {
+ if (themeObj.json_data) {
+ try {
+ const themeConfig = JSON.parse(themeObj.json_data);
+ setTemporaryTheme(themeConfig);
+ setAppliedThemeId(themeObj.id || null);
+ addSuccessToast(t('Local theme set to "%s"', themeObj.theme_name));
+ } catch (error) {
+ addDangerToast(
+ t('Failed to set local theme: Invalid JSON configuration'),
+ );
+ }
+ }
+ }
+
+ const handleBulkThemeExport = (themesToExport: ThemeObject[]) => {
+ const ids = themesToExport
+ .map(({ id }) => id)
+ .filter((id): id is number => id !== undefined);
+ handleResourceExport('theme', ids, () => {
+ setPreparingExport(false);
+ });
+ setPreparingExport(true);
+ };
+
+ const openThemeImportModal = () => {
+ showImportModal(true);
+ };
+
+ const closeThemeImportModal = () => {
+ showImportModal(false);
+ };
+
+ const handleThemeImport = () => {
+ showImportModal(false);
+ refreshData();
+ addSuccessToast(t('Theme imported'));
+ };
+
+ const initialSort = [{ id: 'theme_name', desc: true }];
+ const columns = useMemo(
+ () => [
+ {
+ Cell: ({ row: { original } }: any) => {
+ const currentCrudThemeId = getCurrentCrudThemeId();
+ const isCurrentTheme =
+ (currentCrudThemeId &&
+ original.id?.toString() === currentCrudThemeId) ||
+ (appliedThemeId && original.id === appliedThemeId);
+
+ return (
+ <>
+ {original.theme_name}
+ {isCurrentTheme && (
+ <Tooltip
+ title={t('This theme is set locally for your session')}
+ >
+ <Tag color="green" style={{ marginLeft: 8 }}>
+ {t('Local')}
+ </Tag>
+ </Tooltip>
+ )}
+ {original.is_system && (
+ <Tooltip title={t('Defined through system configuration.')}>
+ <Tag color="blue" style={{ marginLeft: 8 }}>
+ {t('System')}
+ </Tag>
+ </Tooltip>
+ )}
+ </>
+ );
+ },
+ Header: t('Name'),
+ accessor: 'theme_name',
+ id: 'theme_name',
+ },
+ {
+ Cell: ({
+ row: {
+ original: {
+ changed_on_delta_humanized: changedOn,
+ changed_by: changedBy,
+ },
+ },
+ }: any) => <ModifiedInfo date={changedOn} user={changedBy} />,
+ Header: t('Last modified'),
+ accessor: 'changed_on_delta_humanized',
+ size: 'xl',
+ disableSortBy: true,
+ id: 'changed_on_delta_humanized',
+ },
+ {
+ Cell: ({ row: { original } }: any) => {
+ const handleEdit = () => handleThemeEdit(original);
+ const handleDelete = () => setThemeCurrentlyDeleting(original);
+ const handleApply = () => handleThemeApply(original);
+ const handleExport = () => handleBulkThemeExport([original]);
+
+ const actions = [
+ canApply
+ ? {
+ label: 'apply-action',
+ tooltip: t(
+ 'Set local theme. Will be applied to your session until unset.',
+ ),
+ placement: 'bottom',
+ icon: 'ThunderboltOutlined',
+ onClick: handleApply,
+ }
+ : null,
+ canEdit
+ ? {
+ label: 'edit-action',
+ tooltip: original.is_system
+ ? t('View theme')
+ : t('Edit theme'),
+ placement: 'bottom',
+ icon: original.is_system ? 'EyeOutlined' : 'EditOutlined',
+ onClick: handleEdit,
+ }
+ : null,
+ canExport
+ ? {
+ label: 'export-action',
+ tooltip: t('Export theme'),
+ placement: 'bottom',
+ icon: 'UploadOutlined',
+ onClick: handleExport,
+ }
+ : null,
+ canDelete && !original.is_system
+ ? {
+ label: 'delete-action',
+ tooltip: t('Delete theme'),
+ placement: 'bottom',
+ icon: 'DeleteOutlined',
+ onClick: handleDelete,
+ }
+ : null,
+ ].filter(item => !!item);
+
+ return (
+ <ListViewActionsBar actions={actions as ListViewActionProps[]} />
+ );
+ },
+ Header: t('Actions'),
+ id: 'actions',
+ disableSortBy: true,
+ hidden: !canEdit && !canDelete && !canApply && !canExport,
+ size: 'xl',
+ },
+ {
+ accessor: QueryObjectColumns.ChangedBy,
+ hidden: true,
+ id: QueryObjectColumns.ChangedBy,
+ },
+ ],
+ [canDelete, canCreate, canApply, canExport, appliedThemeId],
+ );
+
+ const menuData: SubMenuProps = {
+ name: t('Themes'),
+ };
+
+ const subMenuButtons: SubMenuProps['buttons'] = [];
+
+ if (canDelete || canExport) {
+ subMenuButtons.push({
+ name: t('Bulk select'),
+ onClick: toggleBulkSelect,
+ buttonStyle: 'secondary',
+ });
+ }
+
+ if (canCreate) {
+ subMenuButtons.push({
+ name: <>{t('Theme')}</>,
+ buttonStyle: 'primary',
+ icon: <Icons.PlusOutlined iconSize="m" />,
+ onClick: () => {
+ setCurrentTheme(null);
+ setThemeModalOpen(true);
+ },
+ });
+ }
+
+ if (canImport) {
+ subMenuButtons.push({
+ name: (
+ <Tooltip
+ id="import-tooltip"
+ title={t('Import themes')}
+ placement="bottomRight"
+ >
+ <Icons.DownloadOutlined iconSize="l" data-test="import-button" />
+ </Tooltip>
+ ),
+ buttonStyle: 'link',
+ onClick: openThemeImportModal,
+ });
+ }
+
+ menuData.buttons = subMenuButtons;
+
+ const filters: ListViewFilters = useMemo(
+ () => [
+ {
+ Header: t('Name'),
+ key: 'search',
+ id: 'theme_name',
+ input: 'search',
+ operator: FilterOperator.Contains,
+ },
+ {
+ Header: t('Modified by'),
+ key: 'changed_by',
+ id: 'changed_by',
+ input: 'select',
+ operator: FilterOperator.RelationOneMany,
+ unfilteredLabel: t('All'),
+ fetchSelects: createFetchRelated(
+ 'theme',
+ 'changed_by',
+ createErrorHandler(errMsg =>
+ t(
+ 'An error occurred while fetching theme datasource values: %s',
+ errMsg,
+ ),
+ ),
+ user,
+ ),
+ paginate: true,
+ },
+ ],
+ [],
+ );
+
+ return (
+ <>
+ <SubMenu {...menuData} />
+ <ThemeModal
+ addDangerToast={addDangerToast}
+ theme={currentTheme}
+ onThemeAdd={() => refreshData()}
+ onHide={() => setThemeModalOpen(false)}
+ show={themeModalOpen}
+ canDevelop={canEdit}
+ />
+ <ImportModelsModal
+ resourceName="theme"
+ resourceLabel={t('theme')}
+ passwordsNeededMessage=""
+ confirmOverwriteMessage={CONFIRM_OVERWRITE_MESSAGE}
+ addDangerToast={addDangerToast}
+ addSuccessToast={addSuccessToast}
+ onModelImport={handleThemeImport}
+ show={importingTheme}
+ onHide={closeThemeImportModal}
+ />
+ {themeCurrentlyDeleting && (
+ <DeleteModal
+ description={
+ <>
+ <div style={{ marginBottom: 16 }}>
Review Comment:
Yeah, this component also has some styling issues
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2215977089
##########
superset-frontend/src/theme/ThemeController.ts:
##########
@@ -43,6 +43,8 @@ const DEFAULT_THEME_SETTINGS = {
const STORAGE_KEYS = {
THEME_MODE: 'superset-theme-mode',
Review Comment:
Is this also irrelevant now? It stays as default in local storage
##########
superset-frontend/src/theme/ThemeController.ts:
##########
@@ -236,6 +246,76 @@ export class ThemeController {
this.updateTheme(defaultTheme);
}
+ /**
+ * Sets a CRUD theme by ID. This will fetch the theme from the API and apply it.
+ * @param themeId - The ID of the CRUD theme to apply
+ */
+ public async setCrudTheme(themeId: string | null): Promise<void> {
+ this.crudThemeId = themeId;
+
+ if (themeId) {
+ this.storage.setItem(STORAGE_KEYS.CRUD_THEME_ID, themeId);
+ try {
+ const themeConfig = await this.fetchCrudTheme(themeId);
+ if (themeConfig) {
+ this.updateTheme(themeConfig);
+ }
+ } catch (error) {
+ console.error('Failed to load CRUD theme:', error);
+ this.fallbackToDefaultMode();
+ }
+ } else {
+ this.storage.removeItem(STORAGE_KEYS.CRUD_THEME_ID);
+ this.resetTheme();
+ }
+ }
+
+ /**
+ * Sets a temporary theme override for development purposes.
+ * This does not persist the theme but allows live preview.
+ * @param theme - The theme configuration to apply temporarily
+ */
+ public setTemporaryTheme(theme: AnyThemeConfig): void {
+ this.validateThemeUpdatePermission();
+
+ this.devThemeOverride = theme;
+ this.storage.setItem(
+ STORAGE_KEYS.DEV_THEME_OVERRIDE,
+ JSON.stringify(theme),
+ );
+
+ const normalizedTheme = this.normalizeTheme(theme);
+ this.updateTheme(normalizedTheme);
+ }
Review Comment:
This persists the theme? This is what's called when clicked on apply and it persists the theme.
##########
superset-frontend/src/theme/ThemeController.ts:
##########
@@ -236,6 +246,76 @@ export class ThemeController {
this.updateTheme(defaultTheme);
}
+ /**
+ * Sets a CRUD theme by ID. This will fetch the theme from the API and apply it.
+ * @param themeId - The ID of the CRUD theme to apply
+ */
+ public async setCrudTheme(themeId: string | null): Promise<void> {
+ this.crudThemeId = themeId;
+
+ if (themeId) {
+ this.storage.setItem(STORAGE_KEYS.CRUD_THEME_ID, themeId);
+ try {
+ const themeConfig = await this.fetchCrudTheme(themeId);
+ if (themeConfig) {
+ this.updateTheme(themeConfig);
+ }
+ } catch (error) {
+ console.error('Failed to load CRUD theme:', error);
+ this.fallbackToDefaultMode();
+ }
+ } else {
+ this.storage.removeItem(STORAGE_KEYS.CRUD_THEME_ID);
+ this.resetTheme();
+ }
+ }
Review Comment:
I don't think this is used anywhere. All of the crud related stuff in this file seems unused, we should probably implement it.
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -215,6 +222,34 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
return () => {};
}, [css]);
+ // Apply dashboard theme when dashboard loads
+ useEffect(() => {
+ const applyDashboardTheme = async () => {
+ if (dashboard?.theme?.id) {
+ try {
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/theme/${dashboard.theme.id}`,
+ });
+ const themeConfig = JSON.parse(response.json.result.json_data);
+ themeContext.setTemporaryTheme(themeConfig);
Review Comment:
This doesn't differentiate between dashboard themes and appwide themes. Setting a theme from dashboard changes the whole app, was this intended?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2229939804
##########
superset-frontend/src/features/themes/ThemeModal.tsx:
##########
@@ -0,0 +1,394 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FunctionComponent, useState, useEffect, ChangeEvent } from 'react';
+
+import { css, styled, t, useTheme } from '@superset-ui/core';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import {
+ Input,
+ Modal,
+ JsonEditor,
+ Button,
+ Form,
+ Tooltip,
+ Alert,
+} from '@superset-ui/core/components';
+import { Typography } from '@superset-ui/core/components/Typography';
+
+import { OnlyKeyWithType } from 'src/utils/types';
+import { ThemeObject } from './types';
+
+interface ThemeModalProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast?: (msg: string) => void;
+ theme?: ThemeObject | null;
+ onThemeAdd?: (theme?: ThemeObject) => void;
+ onHide: () => void;
+ show: boolean;
+ canDevelop?: boolean;
+}
+
+type ThemeStringKeys = keyof Pick<
+ ThemeObject,
+ OnlyKeyWithType<ThemeObject, String>
+>;
+
+const StyledJsonEditor = styled.div`
+ ${({ theme }) => css`
+ border-radius: ${theme.borderRadius}px;
+ border: 1px solid ${theme.colorBorder};
+
+ .ace_editor {
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const ThemeModal: FunctionComponent<ThemeModalProps> = ({
+ addDangerToast,
+ addSuccessToast,
+ onThemeAdd,
+ onHide,
+ show,
+ theme = null,
+ canDevelop = false,
+}) => {
+ const supersetTheme = useTheme();
+ const { setTemporaryTheme } = useThemeContext();
+ const [disableSave, setDisableSave] = useState<boolean>(true);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [isHidden, setIsHidden] = useState<boolean>(true);
+ const isEditMode = theme !== null;
+ const isSystemTheme = currentTheme?.is_system === true;
+ const isReadOnly = isSystemTheme;
+ const canDevelopThemes = canDevelop;
+
+ // theme fetch logic
+ const {
+ state: { loading, resource },
+ fetchResource,
+ createResource,
+ updateResource,
+ } = useSingleViewResource<ThemeObject>('theme', t('theme'), addDangerToast);
+
+ // Functions
+ const hide = () => {
+ onHide();
+ setCurrentTheme(null);
+ };
+
+ const onSave = () => {
+ if (isEditMode) {
+ // Edit
+ if (currentTheme?.id) {
+ const update_id = currentTheme.id;
+ delete currentTheme.id;
+ delete currentTheme.created_by;
+ delete currentTheme.changed_by;
+ delete currentTheme.changed_on_delta_humanized;
+
+ updateResource(update_id, currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ } else if (currentTheme) {
+ // Create
+ createResource(currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ };
+
+ const onApply = () => {
+ if (currentTheme?.json_data && isValidJson(currentTheme.json_data)) {
+ try {
+ const themeConfig = JSON.parse(currentTheme.json_data);
+ setTemporaryTheme(themeConfig);
+ if (addSuccessToast) {
+ addSuccessToast(t('Local theme set for preview'));
+ }
+ } catch (error) {
+ addDangerToast(t('Failed to apply theme: Invalid JSON'));
+ }
+ }
+ };
+
+ const onThemeNameChange = (event: ChangeEvent<HTMLInputElement>) => {
+ const { target } = event;
+
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: currentTheme ? currentTheme.json_data : '',
+ };
+
+ data[target.name as ThemeStringKeys] = target.value;
+ setCurrentTheme(data);
+ };
+
+ const onJsonDataChange = (jsonData: string) => {
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: jsonData,
+ };
+ setCurrentTheme(data);
+ };
+
+ const isValidJson = (str?: string) => {
+ if (!str) return false;
+ try {
+ JSON.parse(str);
+ return true;
+ } catch (e) {
+ return false;
+ }
+ };
+
+ const validate = () => {
+ if (isReadOnly) {
+ setDisableSave(true);
+ return;
+ }
+
+ if (
+ currentTheme?.theme_name.length &&
+ currentTheme?.json_data?.length &&
+ isValidJson(currentTheme.json_data)
+ ) {
+ setDisableSave(false);
+ } else {
+ setDisableSave(true);
+ }
+ };
Review Comment:
ok just added this: <img width="571" height="415" alt="Screenshot 2025-07-24 at 6 31 45 PM" src="https://github.com/user-attachments/assets/3f1c501e-f0f0-42f9-b0cb-02039ab92537" />
##########
superset-frontend/src/features/themes/ThemeModal.tsx:
##########
@@ -0,0 +1,394 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FunctionComponent, useState, useEffect, ChangeEvent } from 'react';
+
+import { css, styled, t, useTheme } from '@superset-ui/core';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import {
+ Input,
+ Modal,
+ JsonEditor,
+ Button,
+ Form,
+ Tooltip,
+ Alert,
+} from '@superset-ui/core/components';
+import { Typography } from '@superset-ui/core/components/Typography';
+
+import { OnlyKeyWithType } from 'src/utils/types';
+import { ThemeObject } from './types';
+
+interface ThemeModalProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast?: (msg: string) => void;
+ theme?: ThemeObject | null;
+ onThemeAdd?: (theme?: ThemeObject) => void;
+ onHide: () => void;
+ show: boolean;
+ canDevelop?: boolean;
+}
+
+type ThemeStringKeys = keyof Pick<
+ ThemeObject,
+ OnlyKeyWithType<ThemeObject, String>
+>;
+
+const StyledJsonEditor = styled.div`
+ ${({ theme }) => css`
+ border-radius: ${theme.borderRadius}px;
+ border: 1px solid ${theme.colorBorder};
+
+ .ace_editor {
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const ThemeModal: FunctionComponent<ThemeModalProps> = ({
+ addDangerToast,
+ addSuccessToast,
+ onThemeAdd,
+ onHide,
+ show,
+ theme = null,
+ canDevelop = false,
+}) => {
+ const supersetTheme = useTheme();
+ const { setTemporaryTheme } = useThemeContext();
+ const [disableSave, setDisableSave] = useState<boolean>(true);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [isHidden, setIsHidden] = useState<boolean>(true);
+ const isEditMode = theme !== null;
+ const isSystemTheme = currentTheme?.is_system === true;
+ const isReadOnly = isSystemTheme;
+ const canDevelopThemes = canDevelop;
+
+ // theme fetch logic
+ const {
+ state: { loading, resource },
+ fetchResource,
+ createResource,
+ updateResource,
+ } = useSingleViewResource<ThemeObject>('theme', t('theme'), addDangerToast);
+
+ // Functions
+ const hide = () => {
+ onHide();
+ setCurrentTheme(null);
+ };
+
+ const onSave = () => {
+ if (isEditMode) {
+ // Edit
+ if (currentTheme?.id) {
+ const update_id = currentTheme.id;
+ delete currentTheme.id;
+ delete currentTheme.created_by;
+ delete currentTheme.changed_by;
+ delete currentTheme.changed_on_delta_humanized;
+
+ updateResource(update_id, currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ } else if (currentTheme) {
+ // Create
+ createResource(currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ };
+
+ const onApply = () => {
+ if (currentTheme?.json_data && isValidJson(currentTheme.json_data)) {
+ try {
+ const themeConfig = JSON.parse(currentTheme.json_data);
+ setTemporaryTheme(themeConfig);
+ if (addSuccessToast) {
+ addSuccessToast(t('Local theme set for preview'));
+ }
+ } catch (error) {
+ addDangerToast(t('Failed to apply theme: Invalid JSON'));
+ }
+ }
+ };
+
+ const onThemeNameChange = (event: ChangeEvent<HTMLInputElement>) => {
+ const { target } = event;
+
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: currentTheme ? currentTheme.json_data : '',
+ };
+
+ data[target.name as ThemeStringKeys] = target.value;
+ setCurrentTheme(data);
+ };
+
+ const onJsonDataChange = (jsonData: string) => {
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: jsonData,
+ };
+ setCurrentTheme(data);
+ };
+
+ const isValidJson = (str?: string) => {
+ if (!str) return false;
+ try {
+ JSON.parse(str);
+ return true;
+ } catch (e) {
+ return false;
+ }
+ };
+
+ const validate = () => {
+ if (isReadOnly) {
+ setDisableSave(true);
+ return;
+ }
+
+ if (
+ currentTheme?.theme_name.length &&
+ currentTheme?.json_data?.length &&
+ isValidJson(currentTheme.json_data)
+ ) {
+ setDisableSave(false);
+ } else {
+ setDisableSave(true);
+ }
+ };
Review Comment:
ok just added this:
<img width="571" height="415" alt="Screenshot 2025-07-24 at 6 31 45 PM" src="https://github.com/user-attachments/assets/3f1c501e-f0f0-42f9-b0cb-02039ab92537" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2230071955
##########
tests/integration_tests/dashboards/api_tests.py:
##########
@@ -870,10 +871,13 @@ def test_add_favorite(self):
"""
Dataset API: Test add dashboard to favorites
"""
+ # Let database auto-generate ID and use unique names
+ import uuid
+
+ unique_id = uuid.uuid4().hex[:8]
Review Comment:
might be useful later on, planning on going uuid-first eventually
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3104272050
@mistercrunch Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/16452591626). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3084935782
Korbit doesn't automatically review large (3000+ lines changed) pull requests such as this one. If you want me to review anyway, use `/korbit-review`.
[](633fe190-9104-423a-b8a1-49bf24184174)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3084951934
@mistercrunch Ephemeral environment spinning up at http://54.244.28.232:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34182:
URL: https://github.com/apache/superset/pull/34182#issuecomment-3104507892
@mistercrunch Ephemeral environment spinning up at http://54.203.44.180:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2226352342
##########
docs/docs/configuration/theming.mdx:
##########
@@ -10,44 +10,92 @@ version: 1
apache-superset>=6.0
:::
-Superset now rides on **Ant Design v5’s token-based theming**.
+Superset now rides on **Ant Design v5's token-based theming**.
Every Antd token works, plus a handful of Superset-specific ones for charts and dashboard chrome.
-## 1 — Create a theme
+## Managing Themes via CRUD Interface
-1. Open the official [Ant Design Theme Editor](https://ant.design/theme-editor)
-2. Design your palette, typography, and component overrides.
-3. Open the `CONFIG` modal and paste the JSON.
+Superset now includes a built-in **Theme Management** interface accessible from the admin menu under **Settings > Themes**.
+
+### Creating a New Theme
+
+1. Navigate to **Settings > Themes** in the Superset interface
+2. Click **+ Theme** to create a new theme
+3. Use the [Ant Design Theme Editor](https://ant.design/theme-editor) to design your theme:
+ - Design your palette, typography, and component overrides
+ - Open the `CONFIG` modal and copy the JSON configuration
+4. Paste the JSON into the theme definition field in Superset
+5. Give your theme a descriptive name and save
You can also extend with Superset-specific tokens (documented in the default theme object) before you import.
-## 2 — Apply it instance-wide
+### Applying Themes to Dashboards
+
+Once created, themes can be applied to individual dashboards:
+- Edit any dashboard and select your custom theme from the theme dropdown
+- Each dashboard can have its own theme, allowing for branded or context-specific styling
+
+## Alternative: Instance-wide Configuration
+
+For system-wide theming, you can configure default themes via Python configuration:
+
+### Setting Default Themes
```python
# superset_config.py
-THEME = {
- # Paste your JSON theme definition here
+
+# Default theme (light mode)
+THEME_DEFAULT = {
+ "token": {
+ "colorPrimary": "#2893B3",
+ "colorSuccess": "#5ac189",
+ # ... your theme JSON configuration
+ }
+}
+
+# Dark theme configuration
+THEME_DARK = {
+ "algorithm": "dark",
+ "token": {
+ "colorPrimary": "#2893B3",
+ # ... your dark theme overrides
+ }
+}
+
+# Theme behavior settings
+THEME_SETTINGS = {
+ "enforced": False, # If True, forces default theme always
+ "allowSwitching": True, # Allow users to switch between themes
+ "allowOSPreference": True, # Auto-detect system theme preference
}
```
-Restart Superset to apply changes
+### Referencing CRUD Themes
-## 3 — Tweak live in the app (beta)
+You can also reference themes created via the CRUD interface by UUID:
Review Comment:
This also doesn't feel like an ideal workflow to do from config. It means users have to run superset, get the uuid, rerun superset. Maybe we can do this in the backend while the app is running with a SYSTEM tag or something like that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2228131763
##########
superset-frontend/packages/superset-ui-core/src/theme/utils/themeUtils.ts:
##########
@@ -0,0 +1,97 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import tinycolor from 'tinycolor2';
+import type { SupersetTheme, FontSizeKey, ColorVariants } from '../types';
+
+const fontSizeMap: Record<FontSizeKey, keyof SupersetTheme> = {
+ xs: 'fontSizeXS',
+ s: 'fontSizeSM',
+ m: 'fontSize',
+ l: 'fontSizeLG',
+ xl: 'fontSizeXL',
+ xxl: 'fontSizeXXL',
+};
+
+/**
+ * Get font size from theme tokens based on size key
+ * @param theme - Theme tokens from useTheme()
+ * @param size - Font size key
+ * @returns Font size as string
+ */
+export function getFontSize(theme: SupersetTheme, size?: FontSizeKey): string {
+ const key = fontSizeMap[size || 'm'];
+ return String(theme[key] || theme.fontSize);
+}
+
+/**
+ * Get color variants for a given color type from theme tokens
+ * @param theme - Theme tokens from useTheme()
+ * @param color - Color type (e.g., 'primary', 'error', 'success')
+ * @returns ColorVariants object with bg, border, text colors etc.
+ */
+export function getColorVariants(
+ theme: SupersetTheme,
+ color: string,
+): ColorVariants {
+ const firstLetterCapped = color.charAt(0).toUpperCase() + color.slice(1);
+
+ if (color === 'default' || color === 'grayscale') {
+ const isDark = isThemeDark(theme);
+
+ const flipBrightness = (baseColor: string): string => {
+ if (!isDark) return baseColor;
+ const { r, g, b } = tinycolor(baseColor).toRgb();
+ const invertedColor = tinycolor({ r: 255 - r, g: 255 - g, b: 255 - b });
+ return invertedColor.toHexString();
+ };
+
+ return {
+ active: flipBrightness('#222'),
+ textActive: flipBrightness('#444'),
+ text: flipBrightness('#555'),
+ textHover: flipBrightness('#666'),
+ hover: flipBrightness('#888'),
+ borderHover: flipBrightness('#AAA'),
+ border: flipBrightness('#CCC'),
+ bgHover: flipBrightness('#DDD'),
+ bg: flipBrightness('#F4F4F4'),
+ };
+ }
+
+ return {
+ active: (theme as any)[`color${firstLetterCapped}Active`],
+ textActive: (theme as any)[`color${firstLetterCapped}TextActive`],
+ text: (theme as any)[`color${firstLetterCapped}Text`],
+ textHover: (theme as any)[`color${firstLetterCapped}TextHover`],
+ hover: (theme as any)[`color${firstLetterCapped}Hover`],
+ borderHover: (theme as any)[`color${firstLetterCapped}BorderHover`],
+ border: (theme as any)[`color${firstLetterCapped}Border`],
+ bgHover: (theme as any)[`color${firstLetterCapped}BgHover`],
+ bg: (theme as any)[`color${firstLetterCapped}Bg`],
+ };
+}
+
+/**
+ * Check if the current theme is dark mode based on background color
+ * @param theme - Theme tokens from useTheme()
+ * @returns true if theme is dark, false if light
+ */
+export function isThemeDark(theme: SupersetTheme): boolean {
+ return tinycolor(theme.colorBgContainer).isDark();
Review Comment:
I think it could be a good idea to also consider `colorBgElevated` in addition to `colorBgContainer`
**Note:** There's already a `useIsDark` hook defined in `superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/useTableTheme.ts` that does the same thing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2229160704
##########
superset-frontend/src/features/themes/ThemeModal.tsx:
##########
@@ -0,0 +1,394 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { FunctionComponent, useState, useEffect, ChangeEvent } from 'react';
+
+import { css, styled, t, useTheme } from '@superset-ui/core';
+import { useSingleViewResource } from 'src/views/CRUD/hooks';
+import { useThemeContext } from 'src/theme/ThemeProvider';
+
+import { Icons } from '@superset-ui/core/components/Icons';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import {
+ Input,
+ Modal,
+ JsonEditor,
+ Button,
+ Form,
+ Tooltip,
+ Alert,
+} from '@superset-ui/core/components';
+import { Typography } from '@superset-ui/core/components/Typography';
+
+import { OnlyKeyWithType } from 'src/utils/types';
+import { ThemeObject } from './types';
+
+interface ThemeModalProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast?: (msg: string) => void;
+ theme?: ThemeObject | null;
+ onThemeAdd?: (theme?: ThemeObject) => void;
+ onHide: () => void;
+ show: boolean;
+ canDevelop?: boolean;
+}
+
+type ThemeStringKeys = keyof Pick<
+ ThemeObject,
+ OnlyKeyWithType<ThemeObject, String>
+>;
+
+const StyledJsonEditor = styled.div`
+ ${({ theme }) => css`
+ border-radius: ${theme.borderRadius}px;
+ border: 1px solid ${theme.colorBorder};
+
+ .ace_editor {
+ border-radius: ${theme.borderRadius}px;
+ }
+ `}
+`;
+
+const ThemeModal: FunctionComponent<ThemeModalProps> = ({
+ addDangerToast,
+ addSuccessToast,
+ onThemeAdd,
+ onHide,
+ show,
+ theme = null,
+ canDevelop = false,
+}) => {
+ const supersetTheme = useTheme();
+ const { setTemporaryTheme } = useThemeContext();
+ const [disableSave, setDisableSave] = useState<boolean>(true);
+ const [currentTheme, setCurrentTheme] = useState<ThemeObject | null>(null);
+ const [isHidden, setIsHidden] = useState<boolean>(true);
+ const isEditMode = theme !== null;
+ const isSystemTheme = currentTheme?.is_system === true;
+ const isReadOnly = isSystemTheme;
+ const canDevelopThemes = canDevelop;
+
+ // theme fetch logic
+ const {
+ state: { loading, resource },
+ fetchResource,
+ createResource,
+ updateResource,
+ } = useSingleViewResource<ThemeObject>('theme', t('theme'), addDangerToast);
+
+ // Functions
+ const hide = () => {
+ onHide();
+ setCurrentTheme(null);
+ };
+
+ const onSave = () => {
+ if (isEditMode) {
+ // Edit
+ if (currentTheme?.id) {
+ const update_id = currentTheme.id;
+ delete currentTheme.id;
+ delete currentTheme.created_by;
+ delete currentTheme.changed_by;
+ delete currentTheme.changed_on_delta_humanized;
+
+ updateResource(update_id, currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ } else if (currentTheme) {
+ // Create
+ createResource(currentTheme).then(response => {
+ if (!response) {
+ return;
+ }
+
+ if (onThemeAdd) {
+ onThemeAdd();
+ }
+
+ hide();
+ });
+ }
+ };
+
+ const onApply = () => {
+ if (currentTheme?.json_data && isValidJson(currentTheme.json_data)) {
+ try {
+ const themeConfig = JSON.parse(currentTheme.json_data);
+ setTemporaryTheme(themeConfig);
+ if (addSuccessToast) {
+ addSuccessToast(t('Local theme set for preview'));
+ }
+ } catch (error) {
+ addDangerToast(t('Failed to apply theme: Invalid JSON'));
+ }
+ }
+ };
+
+ const onThemeNameChange = (event: ChangeEvent<HTMLInputElement>) => {
+ const { target } = event;
+
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: currentTheme ? currentTheme.json_data : '',
+ };
+
+ data[target.name as ThemeStringKeys] = target.value;
+ setCurrentTheme(data);
+ };
+
+ const onJsonDataChange = (jsonData: string) => {
+ const data = {
+ ...currentTheme,
+ theme_name: currentTheme ? currentTheme.theme_name : '',
+ json_data: jsonData,
+ };
+ setCurrentTheme(data);
+ };
+
+ const isValidJson = (str?: string) => {
+ if (!str) return false;
+ try {
+ JSON.parse(str);
+ return true;
+ } catch (e) {
+ return false;
+ }
+ };
+
+ const validate = () => {
+ if (isReadOnly) {
+ setDisableSave(true);
+ return;
+ }
+
+ if (
+ currentTheme?.theme_name.length &&
+ currentTheme?.json_data?.length &&
+ isValidJson(currentTheme.json_data)
+ ) {
+ setDisableSave(false);
+ } else {
+ setDisableSave(true);
+ }
+ };
+
+ // Initialize
+ useEffect(() => {
+ if (
+ isEditMode &&
+ (!currentTheme?.id ||
+ (theme && theme?.id !== currentTheme.id) ||
+ (isHidden && show))
+ ) {
+ if (theme?.id && !loading) {
+ fetchResource(theme.id);
+ }
+ } else if (
+ !isEditMode &&
+ (!currentTheme || currentTheme.id || (isHidden && show))
+ ) {
+ setCurrentTheme({
+ theme_name: '',
+ json_data: JSON.stringify({}, null, 2),
+ });
+ }
+ }, [theme, show]);
+
+ useEffect(() => {
+ if (resource) {
+ setCurrentTheme(resource);
+ }
+ }, [resource]);
+
+ // Validation
+ useEffect(() => {
+ validate();
+ }, [
+ currentTheme ? currentTheme.theme_name : '',
+ currentTheme ? currentTheme.json_data : '',
+ isReadOnly,
+ ]);
+
+ // Show/hide
+ if (isHidden && show) {
+ setIsHidden(false);
+ }
+
+ return (
+ <Modal
+ disablePrimaryButton={isReadOnly || disableSave}
+ onHandledPrimaryAction={isReadOnly ? undefined : onSave}
+ onHide={hide}
+ primaryButtonName={isEditMode ? t('Save') : t('Add')}
+ show={show}
+ width="55%"
+ footer={[
+ <Button key="cancel" onClick={hide} buttonStyle="secondary">
+ {isReadOnly ? t('Close') : t('Cancel')}
+ </Button>,
+ ...(!isReadOnly
+ ? [
+ <Button
+ key="save"
+ onClick={onSave}
+ disabled={disableSave}
+ buttonStyle="primary"
+ >
+ {isEditMode ? t('Save') : t('Add')}
+ </Button>,
+ ]
+ : []),
+ ]}
+ title={
+ <Typography.Title level={4} data-test="theme-modal-title">
+ {isEditMode ? (
+ <Icons.EditOutlined
+ iconSize="l"
+ css={css`
+ margin: auto ${supersetTheme.sizeUnit * 2}px auto 0;
+ `}
+ />
+ ) : (
+ <Icons.PlusOutlined
+ iconSize="l"
+ css={css`
+ margin: auto ${supersetTheme.sizeUnit * 2}px auto 0;
+ `}
+ />
+ )}
+ {isEditMode
+ ? isReadOnly
+ ? t('View theme properties')
+ : t('Edit theme properties')
+ : t('Add theme')}
+ </Typography.Title>
+ }
+ >
+ <Form layout="vertical">
+ {isSystemTheme && (
+ <Typography.Text
+ type="secondary"
+ style={{
+ fontSize: '14px',
+ fontStyle: 'italic',
+ marginBottom: '16px',
Review Comment:
good catch ... let me fix the whole form. AI went to town here, need to align with our styling practices.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "msyavuz (via GitHub)" <gi...@apache.org>.
msyavuz commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2229855054
##########
tests/integration_tests/dashboards/api_tests.py:
##########
@@ -870,10 +871,13 @@ def test_add_favorite(self):
"""
Dataset API: Test add dashboard to favorites
"""
+ # Let database auto-generate ID and use unique names
+ import uuid
+
+ unique_id = uuid.uuid4().hex[:8]
Review Comment:
Do we need this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: add a theme CRUD page to manage themes [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #34182:
URL: https://github.com/apache/superset/pull/34182#discussion_r2226607216
##########
docs/docs/configuration/theming.mdx:
##########
@@ -10,44 +10,92 @@ version: 1
apache-superset>=6.0
:::
-Superset now rides on **Ant Design v5’s token-based theming**.
+Superset now rides on **Ant Design v5's token-based theming**.
Every Antd token works, plus a handful of Superset-specific ones for charts and dashboard chrome.
-## 1 — Create a theme
+## Managing Themes via CRUD Interface
-1. Open the official [Ant Design Theme Editor](https://ant.design/theme-editor)
-2. Design your palette, typography, and component overrides.
-3. Open the `CONFIG` modal and paste the JSON.
+Superset now includes a built-in **Theme Management** interface accessible from the admin menu under **Settings > Themes**.
+
+### Creating a New Theme
+
+1. Navigate to **Settings > Themes** in the Superset interface
+2. Click **+ Theme** to create a new theme
+3. Use the [Ant Design Theme Editor](https://ant.design/theme-editor) to design your theme:
+ - Design your palette, typography, and component overrides
+ - Open the `CONFIG` modal and copy the JSON configuration
+4. Paste the JSON into the theme definition field in Superset
+5. Give your theme a descriptive name and save
You can also extend with Superset-specific tokens (documented in the default theme object) before you import.
-## 2 — Apply it instance-wide
+### Applying Themes to Dashboards
+
+Once created, themes can be applied to individual dashboards:
+- Edit any dashboard and select your custom theme from the theme dropdown
+- Each dashboard can have its own theme, allowing for branded or context-specific styling
+
+## Alternative: Instance-wide Configuration
+
+For system-wide theming, you can configure default themes via Python configuration:
+
+### Setting Default Themes
```python
# superset_config.py
-THEME = {
- # Paste your JSON theme definition here
+
+# Default theme (light mode)
+THEME_DEFAULT = {
+ "token": {
+ "colorPrimary": "#2893B3",
+ "colorSuccess": "#5ac189",
+ # ... your theme JSON configuration
+ }
+}
+
+# Dark theme configuration
+THEME_DARK = {
+ "algorithm": "dark",
+ "token": {
+ "colorPrimary": "#2893B3",
+ # ... your dark theme overrides
+ }
+}
+
+# Theme behavior settings
+THEME_SETTINGS = {
+ "enforced": False, # If True, forces default theme always
+ "allowSwitching": True, # Allow users to switch between themes
+ "allowOSPreference": True, # Auto-detect system theme preference
}
```
-Restart Superset to apply changes
+### Referencing CRUD Themes
-## 3 — Tweak live in the app (beta)
+You can also reference themes created via the CRUD interface by UUID:
Review Comment:
What I was thinking here is say you tell a designer: "ok go build an awesome theme for Superset", then the designer goes in and creates a great theme, now how do you set it as your default? I guess you could copy paste the theme definition into the config file manually ...
I could remove all that uuid logic/stuff here if we don't think it's helpful. Maybe it's more confusing than helpful overall.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org