Skip to content

Commit d183969

Browse files
gabotorresruizgabotorresruiz
authored
fix(theming): fix TimeTable chart issues (#34868)
1 parent 4695be5 commit d183969

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

64 files changed

+3995
-709
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ export const IconTooltip = ({
2727
placement = 'top',
2828
style = {},
2929
tooltip = null,
30+
mouseEnterDelay = 0.3,
31+
mouseLeaveDelay = 0.15,
3032
}: IconTooltipProps) => {
3133
const iconTooltip = (
3234
<Button
@@ -47,8 +49,8 @@ export const IconTooltip = ({
4749
id="tooltip"
4850
title={tooltip}
4951
placement={placement}
50-
mouseEnterDelay={0.3}
51-
mouseLeaveDelay={0.15}
52+
mouseEnterDelay={mouseEnterDelay}
53+
mouseLeaveDelay={mouseLeaveDelay}
5254
>
5355
{iconTooltip}
5456
</Tooltip>

‎superset-frontend/packages/superset-ui-core/src/components/IconTooltip/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,6 @@ export interface IconTooltipProps {
3737
| 'rightBottom';
3838
style?: object;
3939
tooltip?: string | null;
40+
mouseEnterDelay?: number;
41+
mouseLeaveDelay?: number;
4042
}

‎superset-frontend/plugins/legacy-preset-chart-nvd3/src/ReactNVD3.jsx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ export default styled(NVD3)`
7272
text.nv-axislabel {
7373
font-size: ${({ theme }) => theme.fontSize} !important;
7474
}
75+
g.nv-axis text {
76+
fill: ${({ theme }) => theme.colorText};
77+
}
78+
g.nv-series text {
79+
fill: ${({ theme }) => theme.colorText};
80+
}
7581
g.solid path,
7682
line.solid {
7783
stroke-dasharray: unset;

‎superset-frontend/src/explore/components/ControlHeader.tsx‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ const ControlHeader: FC<ControlHeaderProps> = ({
7777
<span
7878
css={() => css`
7979
position: absolute;
80-
top: 60%;
80+
top: 50%;
8181
right: 0;
8282
padding-left: ${theme.sizeUnit}px;
8383
transform: translate(100%, -50%);
@@ -156,13 +156,18 @@ const ControlHeader: FC<ControlHeaderProps> = ({
156156
</span>
157157
)}
158158
{validationErrors?.length > 0 && (
159-
<span data-test="error-tooltip">
159+
<span
160+
data-test="error-tooltip"
161+
css={css`
162+
cursor: pointer;
163+
`}
164+
>
160165
<Tooltip
161166
id="error-tooltip"
162167
placement="top"
163168
title={validationErrors?.join(' ')}
164169
>
165-
<Icons.CloseCircleOutlined iconColor={theme.colorErrorText} />
170+
<Icons.InfoCircleOutlined iconColor={theme.colorErrorText} />
166171
</Tooltip>{' '}
167172
</span>
168173
)}

‎superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx‎

Lines changed: 72 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,12 @@
1717
* under the License.
1818
*/
1919
import { useSelector } from 'react-redux';
20-
import { render, screen, userEvent } from 'spec/helpers/testing-library';
20+
import {
21+
render,
22+
screen,
23+
userEvent,
24+
waitFor,
25+
} from 'spec/helpers/testing-library';
2126
import {
2227
DatasourceType,
2328
getChartControlPanelRegistry,
@@ -40,50 +45,54 @@ const FormDataMock = () => {
4045
};
4146

4247
describe('ControlPanelsContainer', () => {
43-
beforeAll(() => {
44-
getChartControlPanelRegistry().registerValue('table', {
45-
controlPanelSections: [
46-
{
47-
label: t('GROUP BY'),
48-
description: t(
49-
'Use this section if you want a query that aggregates',
50-
),
51-
expanded: true,
52-
controlSetRows: [
53-
['groupby'],
54-
['metrics'],
55-
['percent_metrics'],
56-
['timeseries_limit_metric', 'row_limit'],
57-
['include_time', 'order_desc'],
58-
],
59-
},
60-
{
61-
label: t('NOT GROUPED BY'),
62-
description: t('Use this section if you want to query atomic rows'),
63-
expanded: true,
64-
controlSetRows: [
65-
['all_columns'],
66-
['order_by_cols'],
67-
['row_limit', null],
68-
],
69-
},
70-
{
71-
label: t('Query'),
72-
expanded: true,
73-
controlSetRows: [['adhoc_filters']],
74-
},
75-
{
76-
label: t('Options'),
77-
expanded: true,
78-
controlSetRows: [
79-
['table_timestamp_format'],
80-
['page_length', null],
81-
['include_search', 'table_filter'],
82-
['align_pn', 'color_pn'],
83-
],
84-
},
85-
],
86-
});
48+
const defaultTableConfig = {
49+
controlPanelSections: [
50+
{
51+
label: t('GROUP BY'),
52+
description: t('Use this section if you want a query that aggregates'),
53+
expanded: true,
54+
controlSetRows: [
55+
['groupby'],
56+
['metrics'],
57+
['percent_metrics'],
58+
['timeseries_limit_metric', 'row_limit'],
59+
['include_time', 'order_desc'],
60+
],
61+
},
62+
{
63+
label: t('NOT GROUPED BY'),
64+
description: t('Use this section if you want to query atomic rows'),
65+
expanded: true,
66+
controlSetRows: [
67+
['all_columns'],
68+
['order_by_cols'],
69+
['row_limit', null],
70+
],
71+
},
72+
{
73+
label: t('Query'),
74+
expanded: true,
75+
controlSetRows: [['adhoc_filters']],
76+
},
77+
{
78+
label: t('Options'),
79+
expanded: true,
80+
controlSetRows: [
81+
['table_timestamp_format'],
82+
['page_length', null],
83+
['include_search', 'table_filter'],
84+
['align_pn', 'color_pn'],
85+
],
86+
},
87+
],
88+
};
89+
90+
beforeEach(() => {
91+
getChartControlPanelRegistry().registerValue('table', defaultTableConfig);
92+
});
93+
94+
afterEach(() => {
95+
getChartControlPanelRegistry().remove('table');
8796
});
8897

8998
afterAll(() => {
@@ -110,17 +119,22 @@ describe('ControlPanelsContainer', () => {
110119
render(<ControlPanelsContainer {...getDefaultProps()} />, {
111120
useRedux: true,
112121
});
113-
expect(
114-
await screen.findAllByTestId('collapsible-control-panel-header'),
115-
).toHaveLength(4);
122+
await waitFor(() => {
123+
expect(
124+
screen.getAllByTestId('collapsible-control-panel-header'),
125+
).toHaveLength(4);
126+
});
116127
expect(screen.getByRole('tab', { name: /customize/i })).toBeInTheDocument();
117128
userEvent.click(screen.getByRole('tab', { name: /customize/i }));
118-
expect(
119-
await screen.findAllByTestId('collapsible-control-panel-header'),
120-
).toHaveLength(5);
129+
await waitFor(() => {
130+
expect(
131+
screen.getAllByTestId('collapsible-control-panel-header'),
132+
).toHaveLength(5);
133+
});
121134
});
122135

123136
test('renders ControlPanelSections no Customize Tab', async () => {
137+
getChartControlPanelRegistry().remove('table');
124138
getChartControlPanelRegistry().registerValue('table', {
125139
controlPanelSections: [
126140
{
@@ -148,12 +162,15 @@ describe('ControlPanelsContainer', () => {
148162
useRedux: true,
149163
});
150164
expect(screen.queryByText(/customize/i)).not.toBeInTheDocument();
151-
expect(
152-
await screen.findAllByTestId('collapsible-control-panel-header'),
153-
).toHaveLength(2);
165+
await waitFor(() => {
166+
expect(
167+
screen.getAllByTestId('collapsible-control-panel-header'),
168+
).toHaveLength(2);
169+
});
154170
});
155171

156172
test('visibility of panels is correctly applied', async () => {
173+
getChartControlPanelRegistry().remove('table');
157174
getChartControlPanelRegistry().registerValue('table', {
158175
controlPanelSections: [
159176
{
@@ -204,6 +221,7 @@ describe('ControlPanelsContainer', () => {
204221
});
205222

206223
test('hidden state of controls is correctly applied', async () => {
224+
getChartControlPanelRegistry().remove('table');
207225
getChartControlPanelRegistry().registerValue('table', {
208226
controlPanelSections: [
209227
{

‎superset-frontend/src/explore/components/ControlPanelsContainer.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -598,7 +598,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
598598
id={`${kebabCase('validation-errors')}-tooltip`}
599599
title={t('This section contains validation errors')}
600600
>
601-
<Icons.CloseCircleOutlined iconColor={theme.colorErrorText} />
601+
<Icons.InfoCircleOutlined iconColor={theme.colorErrorText} />
602602
</Tooltip>
603603
)}
604604
</span>
@@ -723,7 +723,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
723723
placement="right"
724724
title={props.errorMessage}
725725
>
726-
<Icons.CloseCircleOutlined
726+
<Icons.InfoCircleOutlined
727727
data-test="query-error-tooltip-trigger"
728728
iconColor={theme.colorErrorText}
729729
iconSize="s"

‎superset-frontend/src/explore/components/controls/CollectionControl/CollectionControl.test.tsx‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,12 @@ test('Should have remove button', async () => {
119119
const props = createProps();
120120
render(<CollectionControl {...props} />);
121121

122-
expect(
123-
await screen.findByRole('button', { name: 'Show info tooltip' }),
124-
).toBeInTheDocument();
122+
const removeButton = await screen.findByRole('img', { name: 'close' });
123+
expect(removeButton).toBeInTheDocument();
125124
expect(props.onChange).toHaveBeenCalledTimes(0);
126-
userEvent.click(screen.getByRole('button', { name: 'Show info tooltip' }));
125+
const buttonElement = removeButton.closest('button');
126+
expect(buttonElement).not.toBeNull();
127+
userEvent.click(buttonElement!);
127128
expect(props.onChange).toHaveBeenCalledWith([]);
128129
});
129130

‎superset-frontend/src/explore/components/controls/CollectionControl/index.jsx‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919
import { Component } from 'react';
2020
import PropTypes from 'prop-types';
21-
import { InfoTooltip, List } from '@superset-ui/core/components';
21+
import { IconTooltip, List } from '@superset-ui/core/components';
2222
import { nanoid } from 'nanoid';
2323
import { t, withTheme } from '@superset-ui/core';
2424
import {
@@ -118,9 +118,10 @@ class CollectionControl extends Component {
118118
<SortableListItem
119119
className="clearfix"
120120
css={theme => ({
121+
alignItems: 'center',
121122
justifyContent: 'flex-start',
122-
display: '-webkit-flex',
123-
paddingInline: theme.sizeUnit * 3,
123+
display: 'flex',
124+
paddingInline: theme.sizeUnit * 6,
124125
})}
125126
key={this.props.keyAccessor(o)}
126127
index={i}
@@ -139,14 +140,30 @@ class CollectionControl extends Component {
139140
onChange={this.onChange.bind(this, i)}
140141
/>
141142
</div>
142-
<InfoTooltip
143-
icon="times"
144-
role="button"
145-
label="remove-item"
146-
tooltip={t('Remove item')}
147-
bsStyle="primary"
143+
<IconTooltip
144+
className="pointer"
145+
placement="right"
148146
onClick={this.removeItem.bind(this, i)}
149-
/>
147+
tooltip={t('Remove item')}
148+
mouseEnterDelay={0}
149+
mouseLeaveDelay={0}
150+
css={theme => ({
151+
padding: 0,
152+
minWidth: 'auto',
153+
height: 'auto',
154+
lineHeight: 1,
155+
cursor: 'pointer',
156+
'& svg path': {
157+
fill: theme.colorIcon,
158+
transition: `fill ${theme.motionDurationMid} ease-out`,
159+
},
160+
'&:hover svg path': {
161+
fill: theme.colorError,
162+
},
163+
})}
164+
>
165+
<Icons.CloseOutlined iconSize="s" />
166+
</IconTooltip>
150167
</SortableListItem>
151168
);
152169
})}
@@ -160,7 +177,10 @@ class CollectionControl extends Component {
160177
<HeaderContainer>
161178
<ControlHeader {...this.props} />
162179
<AddIconButton onClick={this.onAdd}>
163-
<Icons.PlusOutlined iconSize="s" />
180+
<Icons.PlusOutlined
181+
iconSize="s"
182+
iconColor={this.props.theme.colorTextLightSolid}
183+
/>
164184
</AddIconButton>
165185
</HeaderContainer>
166186
{this.renderList()}

‎superset-frontend/src/explore/components/controls/OptionControls/index.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ export const AddIconButton = styled.button`
224224
background-color: ${({ theme }) => theme.colorPrimaryText};
225225
border: none;
226226
border-radius: 2px;
227+
cursor: pointer;
227228
228229
:disabled {
229230
cursor: not-allowed;

0 commit comments

Comments
 (0)