msyavuz commented on code in PR #38604:
URL: https://github.com/apache/superset/pull/38604#discussion_r2954334041
##########
superset-frontend/packages/superset-ui-core/src/components/Button/index.tsx:
##########
@@ -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`,
+ },
+});
+
Review Comment:
Should we just go with one approach here? Looks good to me otherwise
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
