fix(theming): Theming visual fixes #34253

msyavuz · 2025-07-21T15:59:42Z

SUMMARY

Fixes for various theming visuals

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Filters overlapping with action buttons:
Before:

After:

UploadDataModal fixes to titles, collapse sizes and remove metadata
Before:

After:

Secondary buttons contrast
Before:

After:

Delete modal spacing
Before:

After:

Async Ace Editor
Before:

After:

Explore columns and metrics
Before:

After:

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue: Fixes Dark Mode: Text formatting and Visibility Issue in UI #34260
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

korbit-ai · 2025-07-21T15:59:48Z

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

mistercrunch · 2025-07-21T16:19:55Z

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/index.tsx

@@ -75,13 +74,9 @@ const verticalStyle = (theme: SupersetTheme, width: number) => css`

  background: linear-gradient(
    ${rgba(theme.colors.grayscale.light5, 0)},
-    ${theme.colors.grayscale.light5} 60%
+    ${theme.colors.grayscale.light5} 20%


let's use one of the colorBg.* for this. Guessing colorBgLayout or colorBgContainer (?)

we still have the task to deprecate all theme.colors.* in the repo ...

Right, this file has some more of those. I might as well touch them while at it. Done!

korbit-ai

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Status
	Unexplained Magic Number in Gradient ▹ view	🧠 Not in scope

Files scanned

File Path	Reviewed
superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.stories.tsx	✅
superset-frontend/src/features/databases/UploadDataModel/styles.ts	✅
superset-frontend/packages/superset-ui-core/src/components/Modal/types.ts	✅
superset-frontend/packages/superset-ui-core/src/components/DeleteModal/index.tsx	✅
superset-frontend/packages/superset-ui-core/src/components/Button/index.tsx	✅
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/index.tsx	✅
superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.tsx	✅
superset-frontend/src/components/ImportModal/index.tsx	✅
superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/index.tsx	✅
superset-frontend/src/features/databases/UploadDataModel/index.tsx	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/index.tsx

  background: linear-gradient(
-    ${rgba(theme.colors.grayscale.light5, 0)},
-    ${theme.colors.grayscale.light5} 60%
+    ${rgba(theme.colorBgLayout, 0)},
+    ${theme.colorBgElevated} 20%
  );


github-actions · 2025-07-22T19:56:25Z

@msyavuz Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments

github-actions · 2025-07-22T20:05:49Z

@msyavuz Ephemeral environment spinning up at http://34.222.10.23:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.

mistercrunch · 2025-07-22T20:18:33Z

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

@@ -134,6 +133,11 @@ export function Button(props: ButtonProps) {
        '& > span > :first-of-type': {
          marginRight: firstChildMargin,
        },
+        ':not(:hover)': effectiveButtonStyle === 'secondary' && {
+          // Increase contrast for secondary buttons
+          backgroundColor: `${tinycolor(theme.colorPrimaryBg).darken(2).toHexString()} !important`,


ideally here we use colorPrimary.* instead of darken/lighten

can we use these?

I looked into this but using colorPrimaryHover or Border for the background of secondary button seemed off semantically. ~~Do you think it's better than using lighten/darken?~~ Nevermind saw the comment below

feel free to move forward with this, could extend the framework with extra semantics for color.*Bg{something} if there's a clear need for more semantics ....

mistercrunch

Minor comment on leveraging colorPrimary.* tokens instead of lighten/darken, but approving regadless. Please use the right tokens if they exist or the best one as proxy if they don't

mistercrunch · 2025-07-22T20:52:18Z

One option would be to add a TODO or NOTE of some kind when we abuse the semantics for a particular shade by using a proxy.

msyavuz · 2025-07-22T21:07:08Z

What i am finding out now is that we don't really have a good option when it comes to contrast for secondary button:

I went with the lightest and darkest here, which is slightly better than what we had before. I think we might need to extend the framework for buttons specifically since we are customizing it so much to our needs. Altering the algorithm for our default themes also should work.

mistercrunch · 2025-07-22T21:17:16Z

I think we might need to extend the framework for buttons specifically since we are customizing it so much to our needs.

Curious what semantic you would introduce? something like colorPrimaryBgSecondary? Noting that it might be a bit of a mess since the darkAlgorithm is what generates a lot of the shading with builtin logic. Might be better to establish design constraints, telling @kasiazjc she has to work with the shades/tokens in the framework (?)

msyavuz · 2025-07-22T21:30:49Z

If we need a secondary button we can introduce seed tokens for it and extend the algorithm to produce necessary map tokens (in the default themes). colorSecondary colorSecondaryBg etc. It will mean diversion from Antd but we already do that.

Also since user's will use Ant Design's theme editor to customize their theme, none of the options seem good enough:

Introduce colorSecondary as a seed token: Doesn't exist on the theme editor
Use semantically incorrect tokens: Unexpected colors that doesn't reflect in theme editor.
Lighten/Darken: Same thing as the first one but smaller impact.

So yes, design constraints seem like the easier/most straightforward option out of these

fix(theming): Theming visual fixes

ecb81fe

pull-request-size bot added the size/XS label Jul 21, 2025

mistercrunch reviewed Jul 21, 2025

View reviewed changes

fix(ActionButtons): use new theme tokens

51bbb32

pull-request-size bot added size/S and removed size/XS labels Jul 21, 2025

fix(UploadDataModel): UploadDataModal fixes and modal improvement

f30c215

pull-request-size bot added size/M and removed size/S labels Jul 21, 2025

github-actions bot added the packages label Jul 21, 2025

msyavuz added 2 commits July 21, 2025 20:02

fix(UploadDataModel): use new color tokens

013e4b3

fix(Button): Make secondary button outlined

3a02920

rusackas requested a review from geido July 21, 2025 17:44

rusackas added review:draft review:checkpoint Last PR reviewed during the daily review standup labels Jul 21, 2025

fix(DeleteModal): spacing

77b7bce

pull-request-size bot added size/L and removed size/M labels Jul 21, 2025

fix(AsyncAceEditor): classname update

2ecdd7a

msyavuz marked this pull request as ready for review July 21, 2025 20:55

dosubot bot added the global:theming Related to theming Superset label Jul 21, 2025

korbit-ai bot reviewed Jul 21, 2025

View reviewed changes

msyavuz added 5 commits July 22, 2025 12:53

fix(UploadDataModel): test refactor

042283b

fix(UploadDataModel): test case fixes

44c4dd2

fix(Modal): correct paddings

02c0424

fix(Button): increase contrast instead of outline

90af979

fix(Button): not hover styles

2edb1cc

msyavuz mentioned this pull request Jul 22, 2025

Dark Mode: Text formatting and Visibility Issue in UI #34260

Closed

fix(Explore): change DatasourcePanelItem from span to text

c32c3b4

msyavuz requested a review from mistercrunch July 22, 2025 15:14

michael-s-molina removed the review:checkpoint Last PR reviewed during the daily review standup label Jul 22, 2025

msyavuz added the testenv-up label Jul 22, 2025

mistercrunch reviewed Jul 22, 2025

View reviewed changes

mistercrunch approved these changes Jul 22, 2025

View reviewed changes

fix(Button): use antd token

91aef4c

msyavuz merged commit 1df5e59 into master Jul 22, 2025
56 checks passed

msyavuz deleted the msyavuz/fix/theming-visual-bugs branch July 22, 2025 22:55

msyavuz mentioned this pull request Jul 23, 2025

fix(theming): Visual regressions p2 #34279

Merged

9 tasks

fix(theming): Theming visual fixes #34253

fix(theming): Theming visual fixes #34253

Uh oh!

Conversation

msyavuz commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

korbit-ai bot commented Jul 21, 2025

Uh oh!

mistercrunch Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

mistercrunch Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

msyavuz Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

korbit-ai bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Uh oh!

This comment was marked as resolved.

Uh oh!

github-actions bot commented Jul 22, 2025

Uh oh!

github-actions bot commented Jul 22, 2025

Uh oh!

mistercrunch Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

mistercrunch Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

msyavuz Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mistercrunch Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

mistercrunch left a comment

Choose a reason for hiding this comment

Uh oh!

mistercrunch commented Jul 22, 2025

Uh oh!

msyavuz commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mistercrunch commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

msyavuz commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

msyavuz commented Jul 21, 2025 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading

msyavuz Jul 22, 2025 •

edited

Loading

msyavuz commented Jul 22, 2025 •

edited

Loading

mistercrunch commented Jul 22, 2025 •

edited

Loading

msyavuz commented Jul 22, 2025 •

edited

Loading