refactor(theming): Fixes to previously migrated components by msyavuz · Pull Request #32845 · apache/superset

msyavuz · 2025-03-25T14:27:56Z

SUMMARY

There are some components that were migrated before #31590 that needs checking and small fixes after Ant Design tokens are decoupled from them. This PR aims to provide those fixes.

These components are:

Avatar
Badge
Button
- Default color is primary which seems better than master
- tertiary color is different
- no warning success colors which were available in the old button but unused.
Card
DatePicker
Divider
Dropdown
Input
InputNumber
List
Menu
- MainNav hover on already selected item is different.
- Decision for MainNav: vanilla or current fixed version
Modal
Tag
Progress
- Slight font size on percent text but vanilla looks fine
Popover
- No visual differences but overlayInnerStyle is deprecated
Slider
- Slightly larger tooltip but it looks fine
Steps
- Line colors are a bit darker on vanilla text is a bit lighter
Switch
- Bit darker color
Tooltip

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Changes

MainNav Before:

MainNav After:

PageHeaderWithActions Before:

PageHeaderWithActions After:

TESTING INSTRUCTIONS

Visual testing.

ADDITIONAL INFORMATION

Has associated issue:
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

…components

korbit-ai · 2025-03-25T14:28:01Z

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

…components

korbit-ai

Review by Korbit AI

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

Category	Issue	Status
	Inefficient CSS Transition ▹ view	✅ Fix detected

Files scanned

File Path	Reviewed
superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigItem.tsx	✅
superset-frontend/src/components/PageHeaderWithActions/index.tsx	✅
superset-frontend/src/components/Menu/index.tsx	✅
superset-frontend/src/components/Modal/Modal.tsx	✅
superset-frontend/src/components/DropdownContainer/index.tsx	✅

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

Need a new review? Comment /korbit-review on this PR and I'll review your latest changes.

Korbit Guide: Usage and Customization

Interacting with Korbit

You can manually ask Korbit to review your PR using the /korbit-review command in a comment at the root of your PR.

You can ask Korbit to generate a new PR description using the /korbit-generate-pr-description command in any comment on your PR.

Too many Korbit comments? I can resolve all my comment threads if you use the /korbit-resolve command in any comment on your PR.

On any given comment that Korbit raises on your pull request, you can have a discussion with Korbit by replying to the comment.

Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.

Customizing Korbit

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

Customize Korbit for your organization through the Korbit Console.

Feedback and Support

Tell us what you think of Korbit

Schedule a call with our team

Email us @ support@korbit.ai

alexandrusoare · 2025-04-01T11:12:36Z

superset-frontend/src/components/Menu/index.tsx

+        bottom: ${theme.sizeUnit / 4}px;
+        left: 0;
+        transform: scale(0);
+        transition: 0.2s all ease-out;


What are these extra styles for?

I provided some screenshots. These styles are here to simplify the hover effects of MainNav.

alexandrusoare · 2025-04-01T11:24:47Z

superset-frontend/src/components/DropdownContainer/index.tsx

+                  maxHeight: `${MAX_HEIGHT}px`,
+                  overflow: showOverflow ? 'auto' : 'visible',
+                },
              }}


overlayInnerStyle is deprecated right?

alexandrusoare · 2025-04-01T11:25:17Z

superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigItem.tsx

-      overlayClassName="column-config-popover"
+      styles={{ body: { width, height } }}
+      rootClassName="column-config-popover"
    >


Here as well?

Yes, thought might as well change these

EnxDev · 2025-04-01T16:22:36Z

superset-frontend/src/components/Menu/index.tsx

        opacity: 0;
        transform: translateX(-50%);
-        transition: all ${({ theme }) => theme.motionDurationMid}s;
+        transition: translate ${({ theme }) => theme.motionDurationMid}s;


let's use something like that:

const SomeStyledThing=styled.div` ${({ theme }) => css` padding: ${theme.sizeUnit * 2}px; border-radius: ${theme.borderRadius}px; border: 1px solid ${theme.colorPrimary}; `} `;

Yeah we can move the theme top level if we use it in multiple instances

geido · 2025-04-02T10:39:16Z

superset-frontend/src/components/Menu/index.tsx

        opacity: 0;
        transform: translateX(-50%);
-        transition: all ${({ theme }) => theme.motionDurationMid}s;
+        transition: translate ${({ theme }) => theme.motionDurationMid}s;


Yeah we can move the theme top level if we use it in multiple instances

geido · 2025-04-02T10:42:11Z

superset-frontend/src/components/Menu/index.tsx

+        position: absolute;
+        width: 100%;
+        height: 2px;
+        background-color: ${theme.colors.primary.base};


I believe this is what we don't want right? With this approach there is no control over this style via the theme configuration. Isn't this supposed to be a vanilla feature? https://ant.design/components/menu#menu-demo-horizontal

let's get in the habit of using native antd tokens, in this case maybe colorPrimaryBg since it's a background-color (?)

Fixed! I think this colorPrimaryBorderHover is a better fit because this is used as a border color of MainNav on hover

geido · 2025-04-02T10:42:32Z

superset-frontend/src/components/Menu/index.tsx

        transform: translateX(-50%);
-        transition: all ${({ theme }) => theme.motionDurationMid}s;
+        transition: translate ${({ theme }) => theme.motionDurationMid}s;
        background-color: ${({ theme }) => theme.colorPrimary};


Do we need this background color here? Can't we use vanilla?

You are right! I can't see the change without this

mistercrunch · 2025-04-02T20:22:31Z

I think I broke (and fixed) something on the base branch, you may need to remerge/rebase

…components

mistercrunch

LGTM! merging so we can iterate on template_less

msyavuz added 5 commits March 21, 2025 18:38

fix: use tertiary button

91db9df

fix: Main nav active, hover styles line height

3e61446

Merge branch 'template_less' into msyavuz/refactor/decouple-migrated-…

6377bd6

…components

fix: menu hover and modal todo

a4006cf

fix: change deprecated prop

f504ec4

pull-request-size bot added the size/M label Mar 25, 2025

msyavuz added the review:draft label Mar 25, 2025

msyavuz added 4 commits March 26, 2025 10:35

Merge branch 'template_less' into msyavuz/refactor/decouple-migrated-…

98ebd27

…components

Merge branch 'template_less' into msyavuz/refactor/decouple-migrated-…

7b98e06

…components

Merge branch 'template_less' into msyavuz/refactor/decouple-migrated-…

90e13e1

…components

Merge branch 'template_less' into msyavuz/refactor/decouple-migrated-…

6224f48

…components

msyavuz marked this pull request as ready for review April 1, 2025 08:28

msyavuz requested review from geido, kgabryje and michael-s-molina as code owners April 1, 2025 08:28

dosubot bot added the global:theming Related to theming Superset label Apr 1, 2025

korbit-ai bot reviewed Apr 1, 2025

View reviewed changes

alexandrusoare reviewed Apr 1, 2025

View reviewed changes

michael-s-molina requested review from EnxDev and mistercrunch April 1, 2025 11:50

perf: only transition translate

f564684

EnxDev reviewed Apr 1, 2025

View reviewed changes

geido reviewed Apr 2, 2025

View reviewed changes

sadpandajoe removed the review:draft label Apr 2, 2025

refactor: style fixes and refactors

7ba2cba

Merge branch 'template_less' into msyavuz/refactor/decouple-migrated-…

e397107

…components

mistercrunch approved these changes Apr 3, 2025

View reviewed changes

mistercrunch merged commit 6f73e58 into apache:template_less Apr 3, 2025
41 checks passed

Conversation

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

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Changes

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

korbit-ai bot commented Mar 25, 2025

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.

Interacting with Korbit

Customizing Korbit

Feedback and Support

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EnxDev Apr 1, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mistercrunch commented Apr 2, 2025

Uh oh!

mistercrunch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

msyavuz commented Mar 25, 2025 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading

EnxDev Apr 1, 2025 •

edited

Loading