This is an automated email from the ASF dual-hosted git repository.
msyavuz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 967134f540 fix(theming): Visual bugs p-3 (#34424)
967134f540 is described below
commit 967134f540b279df00b3856d7d688a14b490985f
Author: Mehmet Salih Yavuz <[email protected]>
AuthorDate: Fri Aug 1 00:26:38 2025 +0300
fix(theming): Visual bugs p-3 (#34424)
---
.../cypress/e2e/dashboard/drillby.test.ts | 17 +++---
.../Chart/ChartContextMenu/ChartContextMenu.tsx | 10 +++-
.../Chart/DrillBy/DrillByMenuItems.test.tsx | 43 +++++++++++----
.../components/Chart/DrillBy/DrillByMenuItems.tsx | 10 ++--
.../components/Chart/MenuItemWithTruncation.tsx | 63 ++++++++++++++++++++--
.../explore/components/ControlPanelsContainer.tsx | 19 +++----
.../DateFilterControl/components/DateLabel.tsx | 14 ++---
7 files changed, 133 insertions(+), 43 deletions(-)
diff --git
a/superset-frontend/cypress-base/cypress/e2e/dashboard/drillby.test.ts
b/superset-frontend/cypress-base/cypress/e2e/dashboard/drillby.test.ts
index d0b10f05ee..b8ded5ee1b 100644
--- a/superset-frontend/cypress-base/cypress/e2e/dashboard/drillby.test.ts
+++ b/superset-frontend/cypress-base/cypress/e2e/dashboard/drillby.test.ts
@@ -65,11 +65,16 @@ const drillBy = (targetDrillByColumn: string, isLegacy =
false) => {
)
.should('be.visible')
.find('[role="menuitem"]')
- .then($el => {
- cy.wrap($el)
- .contains(new RegExp(`^${targetDrillByColumn}$`))
- .trigger('keydown', { keyCode: 13, which: 13, force: true });
- });
+ .contains(new RegExp(`^${targetDrillByColumn}$`))
+ .click();
+
+ cy.get(
+ '.ant-dropdown-menu-submenu:not(.ant-dropdown-menu-submenu-hidden)
[data-test="drill-by-submenu"]',
+ ).trigger('mouseout', { clientX: 0, clientY: 0, force: true });
+
+ cy.get(
+ '.ant-dropdown-menu-submenu:not(.ant-dropdown-menu-submenu-hidden)
[data-test="drill-by-submenu"]',
+ ).should('not.exist');
if (isLegacy) {
return cy.wait('@legacyData');
@@ -240,7 +245,7 @@ describe('Drill by modal', () => {
SUPPORTED_TIER1_CHARTS.forEach(waitForChartLoad);
});
- it('opens the modal from the context menu', () => {
+ it.only('opens the modal from the context menu', () => {
openTableContextMenu('boy');
drillBy('state').then(intercepted => {
verifyExpectedFormData(intercepted, {
diff --git
a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
index bda9c13a83..4e3369429c 100644
---
a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
+++
b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
@@ -63,7 +63,7 @@ export enum ContextMenuItem {
export interface ChartContextMenuProps {
id: number;
formData: QueryFormData;
- onSelection: () => void;
+ onSelection: (args?: any) => void;
onClose: () => void;
additionalConfig?: {
crossFilter?: Record<string, any>;
@@ -123,6 +123,12 @@ const ChartContextMenu = (
const [dataset, setDataset] = useState<Dataset>();
const verboseMap = useVerboseMap(dataset);
+ const closeContextMenu = useCallback(() => {
+ setVisible(false);
+ setOpenKeys([]);
+ onClose();
+ }, [onClose]);
+
const handleDrillBy = useCallback((column: Column, dataset: Dataset) => {
setDrillByColumn(column);
setDataset(dataset); // Save dataset when drilling
@@ -264,6 +270,7 @@ const ChartContextMenu = (
<DrillByMenuItems
drillByConfig={filters?.drillBy}
onSelection={onSelection}
+ onCloseMenu={closeContextMenu}
formData={formData}
contextMenuY={clientY}
submenuIndex={submenuIndex}
@@ -311,6 +318,7 @@ const ChartContextMenu = (
onOpenChange={setOpenKeys}
onClick={() => {
setVisible(false);
+ setOpenKeys([]);
onClose();
}}
>
diff --git
a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx
b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx
index fd838f0108..9d2a9360a1 100644
--- a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx
@@ -166,8 +166,12 @@ test('render menu item with submenu without searchbox',
async () => {
renderMenu({});
await waitFor(() => fetchMock.called(DATASET_ENDPOINT));
await expectDrillByEnabled();
+
+ // Check that each column appears in the drill-by submenu
slicedColumns.forEach(column => {
- expect(screen.getByText(column.column_name)).toBeInTheDocument();
+ const submenus = screen.getAllByTestId('drill-by-submenu');
+ const submenu = submenus[0]; // Use the first submenu
+ expect(within(submenu).getByText(column.column_name)).toBeInTheDocument();
});
expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
});
@@ -186,15 +190,19 @@ test('render menu item with submenu and searchbox', async
() => {
// Wait for all columns to be visible
await waitFor(
() => {
+ const submenus = screen.getAllByTestId('drill-by-submenu');
+ const submenu = submenus[0];
defaultColumns.forEach(column => {
- expect(screen.getByText(column.column_name)).toBeInTheDocument();
+ expect(
+ within(submenu).getByText(column.column_name),
+ ).toBeInTheDocument();
});
},
{ timeout: 10000 },
);
const searchbox = await waitFor(
- () => screen.getAllByPlaceholderText('Search columns')[1],
+ () => screen.getAllByPlaceholderText('Search columns')[0],
);
expect(searchbox).toBeInTheDocument();
@@ -204,19 +212,26 @@ test('render menu item with submenu and searchbox', async
() => {
// Wait for filtered results
await waitFor(() => {
+ const submenus = screen.getAllByTestId('drill-by-submenu');
+ const submenu = submenus[0];
expectedFilteredColumnNames.forEach(colName => {
- expect(screen.getByText(colName)).toBeInTheDocument();
+ expect(within(submenu).getByText(colName)).toBeInTheDocument();
});
});
+ const submenus = screen.getAllByTestId('drill-by-submenu');
+ const submenu = submenus[0];
+
defaultColumns
.filter(col => !expectedFilteredColumnNames.includes(col.column_name))
.forEach(col => {
- expect(screen.queryByText(col.column_name)).not.toBeInTheDocument();
+ expect(
+ within(submenu).queryByText(col.column_name),
+ ).not.toBeInTheDocument();
});
expectedFilteredColumnNames.forEach(colName => {
- expect(screen.getByText(colName)).toBeInTheDocument();
+ expect(within(submenu).getByText(colName)).toBeInTheDocument();
});
});
@@ -238,17 +253,23 @@ test('Do not display excluded column in the menu', async
() => {
// Wait for menu items to be loaded
await waitFor(
() => {
+ const submenus = screen.getAllByTestId('drill-by-submenu');
+ const submenu = submenus[0];
defaultColumns
.filter(column => !excludedColNames.includes(column.column_name))
.forEach(column => {
- expect(screen.getByText(column.column_name)).toBeInTheDocument();
+ expect(
+ within(submenu).getByText(column.column_name),
+ ).toBeInTheDocument();
});
},
{ timeout: 10000 },
);
+ const submenus = screen.getAllByTestId('drill-by-submenu');
+ const submenu = submenus[0];
excludedColNames.forEach(colName => {
- expect(screen.queryByText(colName)).not.toBeInTheDocument();
+ expect(within(submenu).queryByText(colName)).not.toBeInTheDocument();
});
});
@@ -269,7 +290,11 @@ test('When menu item is clicked, call onSelection with
clicked column and drill
await expectDrillByEnabled();
// Wait for col1 to be visible before clicking
- const col1Element = await waitFor(() => screen.getByText('col1'));
+ const col1Element = await waitFor(() => {
+ const submenus = screen.getAllByTestId('drill-by-submenu');
+ const submenu = submenus[0];
+ return within(submenu).getByText('col1');
+ });
userEvent.click(col1Element);
expect(onSelectionMock).toHaveBeenCalledWith(
diff --git
a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx
b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx
index c0ffc795e2..ec0d4e1c21 100644
--- a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx
@@ -54,7 +54,7 @@ import {
import { InputRef } from 'antd';
import { MenuItemTooltip } from '../DisabledMenuItemTooltip';
import { getSubmenuYOffset } from '../utils';
-import { MenuItemWithTruncation } from '../MenuItemWithTruncation';
+import { VirtualizedMenuItem } from '../MenuItemWithTruncation';
import { Dataset } from '../types';
const SUBMENU_HEIGHT = 200;
@@ -68,6 +68,7 @@ export interface DrillByMenuItemsProps {
submenuIndex?: number;
onSelection?: (...args: any) => void;
onClick?: (event: MouseEvent) => void;
+ onCloseMenu?: () => void;
openNewModal?: boolean;
excludedColumns?: Column[];
open: boolean;
@@ -100,6 +101,7 @@ export const DrillByMenuItems = ({
submenuIndex = 0,
onSelection = () => {},
onClick = () => {},
+ onCloseMenu = () => {},
excludedColumns,
openNewModal = true,
open,
@@ -124,6 +126,7 @@ export const DrillByMenuItems = ({
if (openNewModal && onDrillBy && dataset) {
onDrillBy(column, dataset);
}
+ onCloseMenu();
},
[drillByConfig, onClick, onSelection, openNewModal, onDrillBy, dataset],
);
@@ -264,15 +267,14 @@ export const DrillByMenuItems = ({
const { columns, ...rest } = data;
const column = columns[index];
return (
- <MenuItemWithTruncation
- menuKey={`drill-by-item-${column.column_name}`}
+ <VirtualizedMenuItem
tooltipText={column.verbose_name || column.column_name}
onClick={e => handleSelection(e, column)}
style={style}
{...rest}
>
{column.verbose_name || column.column_name}
- </MenuItemWithTruncation>
+ </VirtualizedMenuItem>
);
};
diff --git a/superset-frontend/src/components/Chart/MenuItemWithTruncation.tsx
b/superset-frontend/src/components/Chart/MenuItemWithTruncation.tsx
index 00311c38ad..d766ee780b 100644
--- a/superset-frontend/src/components/Chart/MenuItemWithTruncation.tsx
+++ b/superset-frontend/src/components/Chart/MenuItemWithTruncation.tsx
@@ -18,9 +18,14 @@
*/
import { ReactNode, CSSProperties, useCallback } from 'react';
-import { css, truncationCSS, useCSSTextTruncation } from '@superset-ui/core';
+import {
+ css,
+ truncationCSS,
+ useCSSTextTruncation,
+ useTheme,
+} from '@superset-ui/core';
import { Menu, type ItemType } from '@superset-ui/core/components/Menu';
-import { Tooltip } from '@superset-ui/core/components';
+import { Flex, Tooltip } from '@superset-ui/core/components';
import { MenuItemProps } from 'antd';
export type MenuItemWithTruncationProps = {
@@ -113,7 +118,12 @@ export const MenuItemWithTruncation = ({
onClick={onClick}
style={style}
>
- <Tooltip title={itemIsTruncated ? tooltipText : null}>
+ <Tooltip
+ title={itemIsTruncated ? tooltipText : null}
+ css={css`
+ max-width: 200px;
+ `}
+ >
<div
ref={itemRef}
css={css`
@@ -127,3 +137,50 @@ export const MenuItemWithTruncation = ({
</Menu.Item>
);
};
+
+export const VirtualizedMenuItem = ({
+ tooltipText,
+ children,
+ onClick,
+ style,
+}: {
+ tooltipText: ReactNode;
+ children: ReactNode;
+ onClick?: (e: React.MouseEvent) => void;
+ style?: CSSProperties;
+}) => {
+ const theme = useTheme();
+ const [itemRef, itemIsTruncated] = useCSSTextTruncation<HTMLDivElement>();
+
+ return (
+ <Flex
+ role="menuitem"
+ tabIndex={0}
+ onClick={onClick}
+ align="center"
+ style={style}
+ css={css`
+ cursor: pointer;
+ padding-left: ${theme.paddingXS}px;
+ &:hover {
+ background-color: ${theme.colorBgTextHover};
+ }
+ &:active {
+ background-color: ${theme.colorBgTextActive};
+ }
+ `}
+ >
+ <Tooltip title={itemIsTruncated ? tooltipText : null}>
+ <div
+ ref={itemRef}
+ css={css`
+ max-width: 100%;
+ ${truncationCSS};
+ `}
+ >
+ {children}
+ </div>
+ </Tooltip>
+ </Flex>
+ );
+};
diff --git
a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
index 09c6b0f3b4..83524d102a 100644
--- a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
+++ b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
@@ -53,7 +53,6 @@ import {
sections,
} from '@superset-ui/chart-controls';
import { useSelector } from 'react-redux';
-import { rgba } from 'emotion-rgba';
import { kebabCase, isEqual } from 'lodash';
import {
@@ -118,16 +117,11 @@ const iconStyles = css`
const actionButtonsContainerStyles = (theme: SupersetTheme) => css`
display: flex;
- position: sticky;
- bottom: 0;
flex-direction: column;
align-items: center;
padding: ${theme.sizeUnit * 4}px;
- z-index: 999;
- background: linear-gradient(
- ${rgba(theme.colorBgBase, 0)},
- ${theme.colorBgBase} 35%
- );
+ background: ${theme.colorBgContainer};
+ flex-shrink: 0;
& > button {
min-width: 156px;
@@ -138,15 +132,18 @@ const Styles = styled.div`
position: relative;
height: 100%;
width: 100%;
+ display: flex;
+ flex-direction: column;
// Resizable add overflow-y: auto as a style to this div
// To override it, we need to use !important
overflow: visible !important;
+
#controlSections {
- height: 100%;
- overflow: visible;
- padding-bottom: ${({ theme }) => theme.sizeUnit * 10}px;
+ flex: 1;
+ overflow: auto;
}
+
.tab-content {
overflow: auto;
flex: 1 1 100%;
diff --git
a/superset-frontend/src/explore/components/controls/DateFilterControl/components/DateLabel.tsx
b/superset-frontend/src/explore/components/controls/DateFilterControl/components/DateLabel.tsx
index b4643bf32b..67d076e60e 100644
---
a/superset-frontend/src/explore/components/controls/DateFilterControl/components/DateLabel.tsx
+++
b/superset-frontend/src/explore/components/controls/DateFilterControl/components/DateLabel.tsx
@@ -30,10 +30,6 @@ export type DateLabelProps = {
onClick?: (event: MouseEvent) => void;
};
-// This is the color that antd components (such as Select or Input) use on
hover
-// TODO: use theme.colorPrimary here and in antd components
-const ACTIVE_BORDER_COLOR = '#45BED6';
-
const LabelContainer = styled.div<{
isActive?: boolean;
isPlaceholder?: boolean;
@@ -47,10 +43,9 @@ const LabelContainer = styled.div<{
padding: 0 ${theme.sizeUnit * 3}px;
- background-color: ${theme.colors.grayscale.light5};
+ background-color: ${theme.colorBgContainer};
- border: 1px solid
- ${isActive ? ACTIVE_BORDER_COLOR : theme.colors.grayscale.light2};
+ border: 1px solid ${isActive ? theme.colorPrimary : theme.colorBorder};
border-radius: ${theme.borderRadius}px;
cursor: pointer;
@@ -58,11 +53,11 @@ const LabelContainer = styled.div<{
transition: border-color 0.3s cubic-bezier(0.65, 0.05, 0.36, 1);
:hover,
:focus {
- border-color: ${ACTIVE_BORDER_COLOR};
+ border-color: ${theme.colorPrimary};
}
.date-label-content {
- color: ${isPlaceholder ? theme.colors.grayscale.light1 :
theme.colorText};
+ color: ${isPlaceholder ? theme.colorTextPlaceholder : theme.colorText};
overflow: hidden;
text-overflow: ellipsis;
min-width: 0;
@@ -71,6 +66,7 @@ const LabelContainer = styled.div<{
}
span[role='img'] {
+ color: ${isPlaceholder ? theme.colorTextPlaceholder : theme.colorText};
margin-left: auto;
padding-left: ${theme.sizeUnit}px;
