Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(Collapse): Upgrade Collapse to Antd5 #32959

Merged
merged 2 commits into from
Apr 7, 2025
Merged

refactor(Collapse): Upgrade Collapse to Antd5 #32959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b6013f8
Collapse component upgrade to antd5
DamianPendrak Mar 22, 2025
fa79d09
Merge branch 'template_less' into collapse-component-upgrade
mistercrunch Apr 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions superset-frontend/cypress-base/cypress/e2e/explore/advanced_analytics.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('Advanced analytics', () => {
cy.visitChartByName('Num Births Trend');
cy.verifySliceSuccess({ waitAlias: '@v1Data' });

cy.get('.ant-collapse-header')
cy.get('.antd5-collapse-header')
.contains('Advanced analytics')
.click({ force: true });

Expand All @@ -52,7 +52,7 @@ describe('Advanced analytics', () => {
waitAlias: '@v1Data',
});
cy.wait('@getExplore');
cy.get('.ant-collapse-header')
cy.get('.antd5-collapse-header')
.contains('Advanced analytics')
.click({ force: true });
cy.get('[data-test=time_compare]')
Expand Down
22 changes: 11 additions & 11 deletions superset-frontend/cypress-base/cypress/support/directories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,16 @@ export const securityAccess = {
};
export const homePage = {
homeSection: {
sectionArea: '.ant-collapse-content-box',
sectionArea: '.antd5-collapse-content-box',
sectionElement: '.antd5-card-meta-title',
},
sections: {
expandedSection: '.ant-collapse-item-active',
expandedSection: '.antd5-collapse-item-active',
expandedSectionHeader: '[aria-expanded="true"]',
collapseExpandButton: '.ant-collapse-arrow',
collapsedSection: '[class="ant-collapse-item"]',
collapseExpandButton: '.antd5-collapse-arrow',
collapsedSection: '[class="antd5-collapse-item"]',
collapsedSectionHeader: '[aria-expanded="false"]',
section: '[class^="ant-collapse-item"]',
section: '[class^="antd5-collapse-item"]',
sectionsMenuContainer: "[role='navigation']",
sectionsMenuItem: "[role='menuitem']",
card: dataTestLocator('styled-card'),
Expand Down Expand Up @@ -112,7 +112,7 @@ export const databasesPage = {
advancedTab: '#rc-tabs-0-tab-2',
activeTab: '.antd5-tabs-tab-active',
securitySubMenu:
':nth-child(3) > .ant-collapse-header > .anticon > svg > path',
':nth-child(3) > .antd5-collapse-header > .anticon > svg > path',
aceTextInput: '.ace_text-input',
aceContent: '.ace_content',
connectButton: '.css-16i3wh7',
Expand Down Expand Up @@ -359,14 +359,14 @@ export const nativeFilters = {
removeFilter: '[aria-label="remove"]',
silentLoading: '.loading inline-centered css-101mkpk',
filterConfigurationSections: {
sectionHeader: '.ant-collapse-header',
sectionHeader: '.antd5-collapse-header',
displayedSection: 'div[style="height: 100%; overflow-y: auto;"]',
collapseExpandButton: '.ant-collapse-arrow',
collapseExpandButton: '.antd5-collapse-arrow',
checkedCheckbox: '.ant-checkbox-wrapper-checked',
infoTooltip: '[aria-label="Show info tooltip"]',
parentFilterInput: dataTestLocator('parent-filter-input'),
filterPlaceholder: '.ant-select-selection-placeholder',
collapsedSectionContainer: '[class="ant-collapse-content-box"]',
collapsedSectionContainer: '[class="antd5-collapse-content-box"]',
},
filtersList: {
list: '.antd5-tabs-nav-list',
Expand Down Expand Up @@ -460,7 +460,7 @@ export const dashboardListView = {
};
export const exploreView = {
openDatasourceMenu: dataTestLocator('open-datasource-tab'),
sectionsHeader: '.ant-collapse-header',
sectionsHeader: '.antd5-collapse-header',
datasourceMenuThreeDots: dataTestLocator('datasource-menu-trigger'),
threeDotsMenuDropdown: {
editDataset: dataTestLocator('edit-dataset'),
Expand Down Expand Up @@ -495,7 +495,7 @@ export const exploreView = {
},
controlPanel: {
panel: dataTestLocator('control-tabs'),
categoryArea: '.ant-collapse-content-box',
categoryArea: '.antd5-collapse-content-box',
dragField: dataTestLocator('datasource'),
metricsField: dataTestLocator('metrics'),
optionField: dataTestLocator('option-label'),
Expand Down
18 changes: 9 additions & 9 deletions superset-frontend/src/SqlLab/components/ColumnElement/ColumnElement.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,23 @@ describe('ColumnElement', () => {
it('renders a proper primary key', () => {
const { container } = render(<ColumnElement column={table.columns[0]} />);
expect(container.querySelector('i.fa-key')).toBeInTheDocument();
expect(container.querySelector('.col-name')?.firstChild).toHaveTextContent(
'id',
);
expect(
container.querySelector('[data-test="col-name"]')?.firstChild,
).toHaveTextContent('id');
});
it('renders a multi-key column', () => {
const { container } = render(<ColumnElement column={table.columns[1]} />);
expect(container.querySelector('i.fa-link')).toBeInTheDocument();
expect(container.querySelector('i.fa-bookmark')).toBeInTheDocument();
expect(container.querySelector('.col-name')?.firstChild).toHaveTextContent(
'first_name',
);
expect(
container.querySelector('[data-test="col-name"]')?.firstChild,
).toHaveTextContent('first_name');
});
it('renders a column with no keys', () => {
const { container } = render(<ColumnElement column={table.columns[2]} />);
expect(container.querySelector('i')).not.toBeInTheDocument();
expect(container.querySelector('.col-name')?.firstChild).toHaveTextContent(
'last_name',
);
expect(
container.querySelector('[data-test="col-name"]')?.firstChild,
).toHaveTextContent('last_name');
});
});
9 changes: 5 additions & 4 deletions superset-frontend/src/SqlLab/components/ColumnElement/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { ReactNode } from 'react';
import { ClassNames } from '@emotion/react';
import { styled, useTheme, t } from '@superset-ui/core';
import { Tooltip } from 'src/components/Tooltip';
import { Flex } from 'src/components/Flex';

const StyledTooltip = (props: any) => {
const theme = useTheme();
Expand Down Expand Up @@ -104,15 +105,15 @@ const ColumnElement = ({ column }: ColumnElementProps) => {
));
}
return (
<div className="clearfix table-column">
<div className="pull-left m-l-10 col-name">
<Flex align="center" justify="space-between">
<div data-test="col-name">
{columnName}
{icons}
</div>
<NowrapDiv className="pull-right text-muted">
<NowrapDiv className="text-muted">
<small> {column.type}</small>
</NowrapDiv>
</div>
</Flex>
);
};

Expand Down
71 changes: 11 additions & 60 deletions superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,8 @@ import {
resetState,
} from 'src/SqlLab/actions/sqlLab';
import Button from 'src/components/Button';
import { t, styled, css, SupersetTheme } from '@superset-ui/core';
import Collapse from 'src/components/Collapse';
import { Icons } from 'src/components/Icons';
import { t, styled, css } from '@superset-ui/core';
import { TableSelectorMultiple } from 'src/components/TableSelector';
import { IconTooltip } from 'src/components/IconTooltip';
import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor';
import type { DatabaseObject } from 'src/components/DatabaseSelector';
import { EmptyState } from 'src/components/EmptyState';
Expand All @@ -60,29 +57,6 @@ const StyledScrollbarContainer = styled.div`
overflow: auto;
`;

const collapseStyles = (theme: SupersetTheme) => css`
.ant-collapse-item {
margin-bottom: ${theme.sizeUnit * 3}px;
}
.ant-collapse-header {
padding: 0px !important;
display: flex;
align-items: center;
}
.ant-collapse-content-box {
padding: 0px ${theme.sizeUnit * 4}px 0px 0px !important;
}
.ant-collapse-arrow {
padding: 0 !important;
bottom: ${theme.sizeUnit}px !important;
right: ${theme.sizeUnit * 4}px !important;
color: ${theme.colorPrimaryText} !important;
&:hover {
color: ${theme.colorPrimaryTextHover} !important;
}
}
`;

const LeftBarStyles = styled.div`
${({ theme }) => css`
height: 100%;
Expand Down Expand Up @@ -194,25 +168,6 @@ const SqlEditorLeftBar = ({
});
};

const renderExpandIconWithTooltip = ({ isActive }: { isActive: boolean }) => (
<IconTooltip
css={css`
transform: rotate(90deg);
`}
aria-label="Collapse"
tooltip={
isActive ? t('Collapse table preview') : t('Expand table preview')
}
>
<Icons.RightOutlined
iconSize="s"
css={css`
transform: ${isActive ? 'rotateY(180deg)' : ''};
`}
/>
</IconTooltip>
);

const shouldShowReset = window.location.search === '?reset=1';
const tableMetaDataHeight = height - 130; // 130 is the height of the selects above

Expand Down Expand Up @@ -276,20 +231,16 @@ const SqlEditorLeftBar = ({
height: ${tableMetaDataHeight}px;
`}
>
<Collapse
activeKey={tables
.filter(({ expanded }) => expanded)
.map(({ id }) => id)}
css={collapseStyles}
expandIconPosition="right"
ghost
onChange={onToggleTable}
expandIcon={renderExpandIconWithTooltip}
>
{tables.map(table => (
<TableElement table={table} key={table.id} />
))}
</Collapse>
{tables.map(table => (
<TableElement
table={table}
key={table.id}
activeKey={tables
.filter(({ expanded }) => expanded)
.map(({ id }) => id)}
onChange={onToggleTable}
/>
))}
Comment on lines +234 to +243
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why this refactor of Collapse into TableElement necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly because of the hover functionality for the tooltip and the fact that the Collapse items prop accepts label and children as separate components

</div>
</StyledScrollbarContainer>
{shouldShowReset && (
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/SqlLab/components/TableElement/TableElement.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ const mockedProps = {
...table,
initialized: true,
},
activeKey: [table.id],
};

test('renders', () => {
Expand Down
Loading
Loading