fix(button): Theming configurations for button elements is not consistent

superset-frontend/packages/superset-core/src/theme/types.ts

@@ -168,6 +168,55 @@ export interface SupersetSpecificTokens {
    * Defaults to colorPrimaryBgHover if not specified.
    */
   colorEditorSelection?: string;
+
+  // Secondary button tokens (Superset-specific)
+  // Ant Design's filled variant has no component tokens, so we provide our own.
+  // These fallback to colorPrimary* derived tokens when not set.
+  /**
+   * Text color for secondary buttons.
+   * Fallback: colorPrimary
+   */
+  buttonSecondaryColor?: string;
+  /**
+   * Background color for secondary buttons.
+   * Fallback: colorPrimaryBg
+   */
+  buttonSecondaryBg?: string;
+  /**
+   * Border color for secondary buttons.
+   * Fallback: transparent
+   */
+  buttonSecondaryBorderColor?: string;
+  /**
+   * Text color for secondary buttons on hover.
+   * Fallback: colorPrimary
+   */
+  buttonSecondaryHoverColor?: string;
+  /**
+   * Background color for secondary buttons on hover.
+   * Fallback: colorPrimaryBgHover
+   */
+  buttonSecondaryHoverBg?: string;
+  /**
+   * Border color for secondary buttons on hover.
+   * Fallback: transparent
+   */
+  buttonSecondaryHoverBorderColor?: string;
+  /**
+   * Text color for secondary buttons when active/pressed.
+   * Fallback: colorPrimary
+   */
+  buttonSecondaryActiveColor?: string;
+  /**
+   * Background color for secondary buttons when active/pressed.
+   * Fallback: colorPrimaryBorder
+   */
+  buttonSecondaryActiveBg?: string;
+  /**
+   * Border color for secondary buttons when active/pressed.
+   * Fallback: transparent
+   */
+  buttonSecondaryActiveBorderColor?: string;
 }
 
 /**

superset-frontend/packages/superset-ui-core/src/components/Button/Button.test.tsx

@@ -23,6 +23,11 @@ import {
   SIZES as buttonSizes,
   STYLES as buttonStyles,
 } from './Button.stories';
+import {
+  getSecondaryButtonStyle,
+  getSecondaryButtonHoverStyles,
+} from './index';
+import type { SupersetTheme } from '@apache-superset/core/theme';
 
 test('works with an onClick handler', () => {
   const mockAction = jest.fn();
@@ -47,3 +52,165 @@ test('All the sorybook gallery variants mount', () => {
 
   expect(getAllByRole('button')).toHaveLength(permutationCount);
 });
+
+test('secondary button renders without errors', () => {
+  const { getByRole } = render(
+    <Button buttonStyle="secondary">Secondary</Button>,
+  );
+  expect(getByRole('button')).toBeInTheDocument();
+});
+
+test('getSecondaryButtonStyle uses fallback tokens when custom tokens not set', () => {
+  const mockTheme = {
+    colorPrimary: '#2893B3',
+    colorPrimaryBg: '#e6f4f7',
+    colorPrimaryBgHover: '#cce9ef',
+    colorPrimaryBorder: '#99d3df',
+  } as SupersetTheme;
+
+  const styles = getSecondaryButtonStyle(mockTheme);
+
+  // Default state uses inline styles (no !important needed)
+  expect(styles.color).toBe('#2893B3');
+  expect(styles.backgroundColor).toBe('#e6f4f7');
+  expect(styles.borderColor).toBe('transparent');
+});
+
+test('getSecondaryButtonHoverStyles uses fallback tokens when custom tokens not set', () => {
+  const mockTheme = {
+    colorPrimary: '#2893B3',
+    colorPrimaryBg: '#e6f4f7',
+    colorPrimaryBgHover: '#cce9ef',
+    colorPrimaryBorder: '#99d3df',
+  } as SupersetTheme;
+
+  const hoverStyles = getSecondaryButtonHoverStyles(mockTheme);
+
+  // Hover/active states use CSS with !important for specificity
+  expect(hoverStyles['&:hover'].backgroundColor).toBe('#cce9ef !important');
+  expect(hoverStyles['&:active'].backgroundColor).toBe('#99d3df !important');
+});
+
+test('getSecondaryButtonStyle uses custom tokens when provided', () => {
+  const mockTheme = {
+    colorPrimary: '#2893B3',
+    colorPrimaryBg: '#e6f4f7',
+    colorPrimaryBgHover: '#cce9ef',
+    colorPrimaryBorder: '#99d3df',
+    // Custom secondary button tokens
+    buttonSecondaryColor: '#custom-color',
+    buttonSecondaryBg: '#custom-bg',
+    buttonSecondaryBorderColor: '#custom-border',
+  } as SupersetTheme;
+
+  const styles = getSecondaryButtonStyle(mockTheme);
+
+  // Default state uses inline styles (no !important needed)
+  expect(styles.color).toBe('#custom-color');
+  expect(styles.backgroundColor).toBe('#custom-bg');
+  expect(styles.borderColor).toBe('#custom-border');
+});
+
+test('getSecondaryButtonHoverStyles uses custom tokens when provided', () => {
+  const mockTheme = {
+    colorPrimary: '#2893B3',
+    colorPrimaryBg: '#e6f4f7',
+    colorPrimaryBgHover: '#cce9ef',
+    colorPrimaryBorder: '#99d3df',
+    // Custom secondary button tokens
+    buttonSecondaryHoverColor: '#custom-hover-color',
+    buttonSecondaryHoverBg: '#custom-hover-bg',
+    buttonSecondaryHoverBorderColor: '#custom-hover-border',
+    buttonSecondaryActiveColor: '#custom-active-color',
+    buttonSecondaryActiveBg: '#custom-active-bg',
+    buttonSecondaryActiveBorderColor: '#custom-active-border',
+  } as SupersetTheme;
+
+  const hoverStyles = getSecondaryButtonHoverStyles(mockTheme);
+
+  // Hover/active states use CSS with !important for specificity
+  expect(hoverStyles['&:hover'].color).toBe('#custom-hover-color !important');
+  expect(hoverStyles['&:hover'].backgroundColor).toBe(
+    '#custom-hover-bg !important',
+  );
+  expect(hoverStyles['&:hover'].borderColor).toBe(
+    '#custom-hover-border !important',
+  );
+  expect(hoverStyles['&:active'].color).toBe('#custom-active-color !important');
+  expect(hoverStyles['&:active'].backgroundColor).toBe(
+    '#custom-active-bg !important',
+  );
+  expect(hoverStyles['&:active'].borderColor).toBe(
+    '#custom-active-border !important',
+  );
+});
+
+test('getSecondaryButtonStyle supports partial token overrides', () => {
+  const mockTheme = {
+    colorPrimary: '#2893B3',
+    colorPrimaryBg: '#e6f4f7',
+    colorPrimaryBgHover: '#cce9ef',
+    colorPrimaryBorder: '#99d3df',
+    // Only override some tokens
+    buttonSecondaryBg: '#custom-bg',
+    buttonSecondaryBorderColor: '#custom-border',
+  } as SupersetTheme;
+
+  const styles = getSecondaryButtonStyle(mockTheme);
+
+  // Should use custom values where provided (no !important for inline styles)
+  expect(styles.backgroundColor).toBe('#custom-bg');
+  expect(styles.borderColor).toBe('#custom-border');
+  // Should fallback to Ant Design tokens where not provided
+  expect(styles.color).toBe('#2893B3');
+});
+
+test('getSecondaryButtonHoverStyles supports partial token overrides', () => {
+  const mockTheme = {
+    colorPrimary: '#2893B3',
+    colorPrimaryBg: '#e6f4f7',
+    colorPrimaryBgHover: '#cce9ef',
+    colorPrimaryBorder: '#99d3df',
+    // Only override hover bg
+    buttonSecondaryHoverBg: '#custom-hover-bg',
+  } as SupersetTheme;
+
+  const hoverStyles = getSecondaryButtonHoverStyles(mockTheme);
+
+  // Should use custom value where provided
+  expect(hoverStyles['&:hover'].backgroundColor).toBe(
+    '#custom-hover-bg !important',
+  );
+  // Should fallback to Ant Design tokens where not provided
+  expect(hoverStyles['&:hover'].color).toBe('#2893B3 !important');
+  expect(hoverStyles['&:active'].backgroundColor).toBe('#99d3df !important');
+});
+
+test('getSecondaryButtonStyle falls back when tokens are empty strings', () => {
+  const mockTheme = {
+    colorPrimary: '#2893B3',
+    colorPrimaryBg: '#e6f4f7',
+    buttonSecondaryColor: '',
+    buttonSecondaryBg: '',
+    buttonSecondaryBorderColor: '',
+  } as SupersetTheme;
+
+  const styles = getSecondaryButtonStyle(mockTheme);
+
+  // Empty strings should trigger fallback to primary tokens
+  expect(styles.color).toBe('#2893B3');
+  expect(styles.backgroundColor).toBe('#e6f4f7');
+  expect(styles.borderColor).toBe('transparent');
+});
+
+test('secondary button merges consumer style with theme styles', () => {
+  const { getByRole } = render(
+    <Button buttonStyle="secondary" style={{ marginTop: 10, padding: 20 }}>
+      Styled Secondary
+    </Button>,
+  );
+  const button = getByRole('button');
+
+  // Consumer styles should be applied
+  expect(button).toHaveStyle({ marginTop: '10px', padding: '20px' });
+});

superset-frontend/packages/superset-ui-core/src/components/Button/index.tsx

@@ -21,6 +21,7 @@ import cx from 'classnames';
 import { Button as AntdButton } from 'antd';
 import { useTheme } from '@apache-superset/core/theme';
 import { Tooltip } from '../Tooltip';
+import type { SupersetTheme } from '@apache-superset/core/theme';
 import type {
   ButtonColorType,
   ButtonProps,
@@ -30,6 +31,59 @@ import type {
   OnClickHandler,
 } from './types';
 
+/**
+ * Secondary Button Theming
+ *
+ * Ant Design's "filled" variant (used for secondary buttons) has no component-level
+ * tokens for customization. To enable full theming of secondary buttons, we use
+ * Superset-specific tokens (buttonSecondary*) with fallbacks to Ant Design's
+ * colorPrimary* derived tokens.
+ *
+ * Implementation approach (follows PR #38679 pattern for label tokens):
+ * - Default state: Applied via inline `style` prop (higher specificity than CSS classes)
+ * - Hover/Active states: Applied via `css` prop with !important (pseudo-selectors
+ *   cannot be applied via inline styles)
+ *
+ * Available tokens (all optional, with sensible fallbacks):
+ * - buttonSecondaryColor: Text color (fallback: colorPrimary)
+ * - buttonSecondaryBg: Background color (fallback: colorPrimaryBg)
+ * - buttonSecondaryBorderColor: Border color (fallback: transparent)
+ * - buttonSecondaryHoverColor: Hover text color (fallback: colorPrimary)
+ * - buttonSecondaryHoverBg: Hover background (fallback: colorPrimaryBgHover)
+ * - buttonSecondaryHoverBorderColor: Hover border (fallback: transparent)
+ * - buttonSecondaryActiveColor: Active/pressed text color (fallback: colorPrimary)
+ * - buttonSecondaryActiveBg: Active/pressed background (fallback: colorPrimaryBorder)
+ * - buttonSecondaryActiveBorderColor: Active/pressed border (fallback: transparent)
+ */
+
+/**
+ * Generates inline styles for secondary buttons (default state).
+ * Inline styles have higher specificity than CSS classes, so no !important needed.
+ */
+export const getSecondaryButtonStyle = (theme: SupersetTheme) => ({
+  color: theme.buttonSecondaryColor || theme.colorPrimary,
+  backgroundColor: theme.buttonSecondaryBg || theme.colorPrimaryBg,
+  borderColor: theme.buttonSecondaryBorderColor || 'transparent',
+});
+
+/**
+ * Generates CSS styles for secondary button hover/active states.
+ * Must use CSS (not inline styles) since pseudo-selectors cannot be applied via style prop.
+ * Uses !important to override Ant Design's default styles.
+ */
+export const getSecondaryButtonHoverStyles = (theme: SupersetTheme) => ({
+  '&:hover': {
+    color: `${theme.buttonSecondaryHoverColor || theme.colorPrimary} !important`,
+    backgroundColor: `${theme.buttonSecondaryHoverBg || theme.colorPrimaryBgHover} !important`,
+    borderColor: `${theme.buttonSecondaryHoverBorderColor || 'transparent'} !important`,
+  },
+  '&:active': {
+    color: `${theme.buttonSecondaryActiveColor || theme.colorPrimary} !important`,
+    backgroundColor: `${theme.buttonSecondaryActiveBg || theme.colorPrimaryBorder} !important`,
+    borderColor: `${theme.buttonSecondaryActiveBorderColor || 'transparent'} !important`,
+  },
+});
+
 const BUTTON_STYLE_MAP: Record<
   ButtonStyle,
   {
@@ -98,6 +152,12 @@ export function Button(props: ButtonProps) {
 
   const effectiveButtonStyle: ButtonStyle = buttonStyle ?? 'primary';
 
+  // Secondary button inline styles (default state) - inline styles override CSS classes
+  const secondaryStyle =
+    effectiveButtonStyle === 'secondary' && !disabled
+      ? getSecondaryButtonStyle(theme)
+      : undefined;
+
   const button = (
     <AntdButton
       href={disabled ? undefined : href}
@@ -132,15 +192,18 @@ export function Button(props: ButtonProps) {
         '& > span > :first-of-type': {
           marginRight: firstChildMargin,
         },
-        ':not(:hover)': effectiveButtonStyle === 'secondary' &&
-          !disabled && {
-            // NOTE: This is the best we can do contrast wise for the secondary button using antd tokens
-            // and abusing the semantics. Should be revisited when possible. https://github.com/apache/superset/pull/34253#issuecomment-3104834692
-            color: `${theme.colorPrimaryTextHover} !important`,
-          },
+        // Secondary button hover/active states via CSS
+        ...(effectiveButtonStyle === 'secondary' &&
+          !disabled &&
+          getSecondaryButtonHoverStyles(theme)),
       }}
       icon={icon}
       {...restProps}
+      style={
+        secondaryStyle
+          ? { ...secondaryStyle, ...restProps.style }
+          : restProps.style
+      }
     >
       {children}
     </AntdButton>

superset-frontend/src/theme/utils/antdTokenNames.ts

@@ -47,6 +47,20 @@ const SUPERSET_CUSTOM_TOKENS: Set<string> = new Set([
 
   // Font loading
   'fontUrls',
+
+  // Editor tokens
+  'colorEditorSelection',
+
+  // Secondary button tokens
+  'buttonSecondaryColor',
+  'buttonSecondaryBg',
+  'buttonSecondaryBorderColor',
+  'buttonSecondaryHoverColor',
+  'buttonSecondaryHoverBg',
+  'buttonSecondaryHoverBorderColor',
+  'buttonSecondaryActiveColor',
+  'buttonSecondaryActiveBg',
+  'buttonSecondaryActiveBorderColor',
 ]);
 
 /**