Skip to content

Commit

Permalink
Merge branch 'master' of github.com:apache/superset into dm/chore/dep…
Browse files Browse the repository at this point in the history
…recate-slice-annotation-json
  • Loading branch information
diegomedina248 committed Jan 24, 2023
2 parents 93dc7fb + 02a3c0d commit 065b288
Show file tree
Hide file tree
Showing 22 changed files with 138 additions and 90 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,17 @@ import DatasourceEditor from 'src/components/Datasource/DatasourceEditor';
import mockDatasource from 'spec/fixtures/mockDatasource';
import * as featureFlags from 'src/featureFlags';

jest.mock('src/components/Icons/Icon', () => ({ fileName, role, ...rest }) => (
<span
role={role ?? 'img'}
aria-label={fileName.replace('_', '-')}
{...rest}
/>
));
jest.mock('src/components/Icons/Icon', () => ({
__esModule: true,
default: ({ fileName, role, ...rest }) => (
<span
role={role ?? 'img'}
aria-label={fileName.replace('_', '-')}
{...rest}
/>
),
StyledIcon: () => <span />,
}));

const props = {
datasource: mockDatasource['7__table'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { ReactWrapper } from 'enzyme';
import { styledMount as mount } from 'spec/helpers/theming';
import FilterableTable, {
MAX_COLUMNS_FOR_TABLE,
renderBigIntStrToNumber,
convertBigIntStrToNumber,
} from 'src/components/FilterableTable';
import { render, screen } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
Expand Down Expand Up @@ -334,17 +334,17 @@ describe('FilterableTable sorting - RTL', () => {
});

test('renders bigInt value in a number format', () => {
expect(renderBigIntStrToNumber('123')).toBe('123');
expect(renderBigIntStrToNumber('some string value')).toBe(
expect(convertBigIntStrToNumber('123')).toBe('123');
expect(convertBigIntStrToNumber('some string value')).toBe(
'some string value',
);
expect(renderBigIntStrToNumber('{ a: 123 }')).toBe('{ a: 123 }');
expect(renderBigIntStrToNumber('"Not a Number"')).toBe('"Not a Number"');
expect(convertBigIntStrToNumber('{ a: 123 }')).toBe('{ a: 123 }');
expect(convertBigIntStrToNumber('"Not a Number"')).toBe('"Not a Number"');
// trim quotes for bigint string format
expect(renderBigIntStrToNumber('"-12345678901234567890"')).toBe(
expect(convertBigIntStrToNumber('"-12345678901234567890"')).toBe(
'-12345678901234567890',
);
expect(renderBigIntStrToNumber('"12345678901234567890"')).toBe(
expect(convertBigIntStrToNumber('"12345678901234567890"')).toBe(
'12345678901234567890',
);
});
6 changes: 5 additions & 1 deletion superset-frontend/src/components/FilterableTable/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,17 @@ function safeJsonObjectParse(
}
}

export function renderBigIntStrToNumber(value: string) {
export function convertBigIntStrToNumber(value: string | number) {
if (typeof value === 'string' && /^"-?\d+"$/.test(value)) {
return value.substring(1, value.length - 1);
}
return value;
}

function renderBigIntStrToNumber(value: string | number) {
return <>{convertBigIntStrToNumber(value)}</>;
}

const GRID_POSITION_ADJUSTMENT = 4;
const SCROLL_BAR_HEIGHT = 15;
// This regex handles all possible number formats in javascript, including ints, floats,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,13 @@ describe('LabeledErrorBoundInput', () => {
defaultProps.visibilityToggle = true;
render(<LabeledErrorBoundInput {...defaultProps} />);

expect(await screen.findByRole('img', { name: /eye/i })).toBeVisible();
expect(await screen.findByTestId('icon-eye')).toBeVisible();
});

it('becomes a password input if props.name === password (backwards compatibility)', async () => {
defaultProps.name = 'password';
render(<LabeledErrorBoundInput {...defaultProps} />);

expect(await screen.findByRole('img', { name: /eye/i })).toBeVisible();
expect(await screen.findByTestId('icon-eye')).toBeVisible();
});
});
16 changes: 13 additions & 3 deletions superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
*/
import React from 'react';
import { Input, Tooltip } from 'antd';
import { EyeInvisibleOutlined, EyeOutlined } from '@ant-design/icons';
import { styled, css, SupersetTheme, t } from '@superset-ui/core';
import InfoTooltip from 'src/components/InfoTooltip';
import Icons from 'src/components/Icons';
import errorIcon from 'src/assets/images/icons/error.svg';
import FormItem from './FormItem';
import FormLabel from './FormLabel';
Expand Down Expand Up @@ -92,6 +92,12 @@ const StyledFormLabel = styled(FormLabel)`
margin-bottom: 0;
`;

const iconReset = css`
&.anticon > * {
line-height: 0;
}
`;

const LabeledErrorBoundInput = ({
label,
validationMethods,
Expand Down Expand Up @@ -128,11 +134,15 @@ const LabeledErrorBoundInput = ({
iconRender={visible =>
visible ? (
<Tooltip title={t('Hide password.')}>
<EyeInvisibleOutlined />
<Icons.EyeInvisibleOutlined iconSize="m" css={iconReset} />
</Tooltip>
) : (
<Tooltip title={t('Show password.')}>
<EyeOutlined />
<Icons.EyeOutlined
iconSize="m"
css={iconReset}
data-test="icon-eye"
/>
</Tooltip>
)
}
Expand Down
6 changes: 5 additions & 1 deletion superset-frontend/src/components/ListView/ListView.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ import Pagination from 'src/components/Pagination/Wrapper';

import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';

jest.mock('src/components/Icons/Icon', () => () => <span />);
jest.mock('src/components/Icons/Icon', () => ({
__esModule: true,
default: () => <span />,
StyledIcon: () => <span />,
}));

function makeMockLocation(query) {
const queryStr = encodeURIComponent(query);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ import HeaderReportDropdown, { HeaderReportProps } from '.';

let isFeatureEnabledMock: jest.MockInstance<boolean, [string]>;

jest.mock('src/components/Icons/Icon', () => () => <span />);
jest.mock('src/components/Icons/Icon', () => ({
__esModule: true,
default: () => <span />,
StyledIcon: () => <span />,
}));

const createProps = () => ({
dashboardId: 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ fetchMock.get(REPORT_ENDPOINT, {});

const NOOP = () => {};

jest.mock('src/components/Icons/Icon', () => () => <span />);
jest.mock('src/components/Icons/Icon', () => ({
__esModule: true,
default: () => <span />,
StyledIcon: () => <span />,
}));

const defaultProps = {
addDangerToast: NOOP,
Expand Down
6 changes: 3 additions & 3 deletions superset-frontend/src/components/Select/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import { ensureIsArray, t } from '@superset-ui/core';
import AntdSelect, { LabeledValue as AntdLabeledValue } from 'antd/lib/select';
import React, { ReactElement, RefObject } from 'react';
import { DownOutlined, SearchOutlined } from '@ant-design/icons';
import Icons from 'src/components/Icons';
import { StyledHelperText, StyledLoadingText, StyledSpin } from './styles';
import { LabeledValue, RawValue, SelectOptionsType, V } from './types';

Expand Down Expand Up @@ -132,9 +132,9 @@ export const getSuffixIcon = (
return <StyledSpin size="small" />;
}
if (showSearch && isDropdownVisible) {
return <SearchOutlined />;
return <Icons.SearchOutlined iconSize="s" />;
}
return <DownOutlined />;
return <Icons.DownOutlined iconSize="s" />;
};

export const dropDownRenderHelper = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,11 @@ test('Should render "appliedCrossFilterIndicators"', () => {
userEvent.click(screen.getByTestId('details-panel-content'));
expect(screen.getByText('Applied Cross Filters (1)')).toBeInTheDocument();
expect(
screen.getByRole('button', { name: 'search Clinical Stage' }),
screen.getByRole('button', { name: 'Clinical Stage' }),
).toBeInTheDocument();

expect(props.onHighlightFilterSource).toBeCalledTimes(0);
userEvent.click(
screen.getByRole('button', { name: 'search Clinical Stage' }),
);
userEvent.click(screen.getByRole('button', { name: 'Clinical Stage' }));
expect(props.onHighlightFilterSource).toBeCalledTimes(1);
expect(props.onHighlightFilterSource).toBeCalledWith([
'ROOT_ID',
Expand All @@ -135,12 +133,10 @@ test('Should render "appliedIndicators"', () => {

userEvent.click(screen.getByTestId('details-panel-content'));
expect(screen.getByText('Applied Filters (1)')).toBeInTheDocument();
expect(
screen.getByRole('button', { name: 'search Country' }),
).toBeInTheDocument();
expect(screen.getByRole('button', { name: 'Country' })).toBeInTheDocument();

expect(props.onHighlightFilterSource).toBeCalledTimes(0);
userEvent.click(screen.getByRole('button', { name: 'search Country' }));
userEvent.click(screen.getByRole('button', { name: 'Country' }));
expect(props.onHighlightFilterSource).toBeCalledTimes(1);
expect(props.onHighlightFilterSource).toBeCalledWith([
'ROOT_ID',
Expand Down Expand Up @@ -168,12 +164,12 @@ test('Should render "incompatibleIndicators"', () => {
userEvent.click(screen.getByTestId('details-panel-content'));
expect(screen.getByText('Incompatible Filters (1)')).toBeInTheDocument();
expect(
screen.getByRole('button', { name: 'search Vaccine Approach Copy' }),
screen.getByRole('button', { name: 'Vaccine Approach Copy' }),
).toBeInTheDocument();

expect(props.onHighlightFilterSource).toBeCalledTimes(0);
userEvent.click(
screen.getByRole('button', { name: 'search Vaccine Approach Copy' }),
screen.getByRole('button', { name: 'Vaccine Approach Copy' }),
);
expect(props.onHighlightFilterSource).toBeCalledTimes(1);
expect(props.onHighlightFilterSource).toBeCalledWith([
Expand Down Expand Up @@ -202,13 +198,11 @@ test('Should render "unsetIndicators"', () => {
userEvent.click(screen.getByTestId('details-panel-content'));
expect(screen.getByText('Unset Filters (1)')).toBeInTheDocument();
expect(
screen.getByRole('button', { name: 'search Vaccine Approach' }),
screen.getByRole('button', { name: 'Vaccine Approach' }),
).toBeInTheDocument();

expect(props.onHighlightFilterSource).toBeCalledTimes(0);
userEvent.click(
screen.getByRole('button', { name: 'search Vaccine Approach' }),
);
userEvent.click(screen.getByRole('button', { name: 'Vaccine Approach' }));
expect(props.onHighlightFilterSource).toBeCalledTimes(1);
expect(props.onHighlightFilterSource).toBeCalledWith([
'ROOT_ID',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ import React, { useEffect, useState } from 'react';
import { useSelector } from 'react-redux';
import { Global, css } from '@emotion/react';
import { t, useTheme } from '@superset-ui/core';
import {
MinusCircleFilled,
CheckCircleFilled,
ExclamationCircleFilled,
} from '@ant-design/icons';
import Popover from 'src/components/Popover';
import Collapse from 'src/components/Collapse';
import Icons from 'src/components/Icons';
Expand All @@ -38,6 +33,12 @@ import { Indicator } from 'src/dashboard/components/FiltersBadge/selectors';
import FilterIndicator from 'src/dashboard/components/FiltersBadge/FilterIndicator';
import { RootState } from 'src/dashboard/types';

const iconReset = css`
span {
line-height: 0;
}
`;

export interface DetailsPanelProps {
appliedCrossFilterIndicators: Indicator[];
appliedIndicators: Indicator[];
Expand Down Expand Up @@ -206,7 +207,7 @@ const DetailsPanelPopover = ({
key="applied"
header={
<Title bold color={theme.colors.success.base}>
<CheckCircleFilled />{' '}
<Icons.CheckCircleFilled css={iconReset} iconSize="m" />{' '}
{t('Applied Filters (%d)', appliedIndicators.length)}
</Title>
}
Expand All @@ -227,7 +228,7 @@ const DetailsPanelPopover = ({
key="incompatible"
header={
<Title bold color={theme.colors.alert.base}>
<ExclamationCircleFilled />{' '}
<Icons.ExclamationCircleFilled css={iconReset} iconSize="m" />{' '}
{t(
'Incompatible Filters (%d)',
incompatibleIndicators.length,
Expand All @@ -251,7 +252,7 @@ const DetailsPanelPopover = ({
key="unset"
header={
<Title bold color={theme.colors.grayscale.light1}>
<MinusCircleFilled />{' '}
<Icons.MinusCircleFilled css={iconReset} iconSize="m" />{' '}
{t('Unset Filters (%d)', unsetIndicators.length)}
</Title>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,17 @@ test('Should render', () => {
render(<FilterIndicator {...props} />);

expect(
screen.getByRole('button', { name: 'search Vaccine Approach' }),
screen.getByRole('button', { name: 'Vaccine Approach' }),
).toBeInTheDocument();
expect(screen.getByRole('img', { name: 'search' })).toBeInTheDocument();
expect(screen.getByRole('img')).toBeInTheDocument();
});

test('Should call "onClick"', () => {
const props = createProps();
render(<FilterIndicator {...props} />);

expect(props.onClick).toBeCalledTimes(0);
userEvent.click(
screen.getByRole('button', { name: 'search Vaccine Approach' }),
);
userEvent.click(screen.getByRole('button', { name: 'Vaccine Approach' }));
expect(props.onClick).toBeCalledTimes(1);
});

Expand All @@ -66,7 +64,7 @@ test('Should render "value"', () => {

expect(
screen.getByRole('button', {
name: 'search Vaccine Approach: any, string',
name: 'Vaccine Approach: any, string',
}),
).toBeInTheDocument();
});
Expand All @@ -77,9 +75,7 @@ test('Should render with default props', () => {
render(<FilterIndicator indicator={props.indicator} />);

expect(
screen.getByRole('button', { name: 'search Vaccine Approach' }),
screen.getByRole('button', { name: 'Vaccine Approach' }),
).toBeInTheDocument();
userEvent.click(
screen.getByRole('button', { name: 'search Vaccine Approach' }),
);
userEvent.click(screen.getByRole('button', { name: 'Vaccine Approach' }));
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
* under the License.
*/

import { SearchOutlined } from '@ant-design/icons';
import React, { FC } from 'react';
import { css } from '@superset-ui/core';
import Icons from 'src/components/Icons';
import { getFilterValueForDisplay } from 'src/dashboard/components/nativeFilters/FilterBar/FilterSets/utils';
import {
FilterIndicatorText,
Expand Down Expand Up @@ -46,7 +47,14 @@ const FilterIndicator: FC<IndicatorProps> = ({
<Item onClick={() => onClick([...path, `LABEL-${column}`])}>
<Title bold>
<ItemIcon>
<SearchOutlined />
<Icons.SearchOutlined
iconSize="m"
css={css`
span {
vertical-align: 0;
}
`}
/>
</ItemIcon>
{name}
{resultValue ? ': ' : ''}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { Typography, AntdTooltip } from 'src/components';
import { useDispatch } from 'react-redux';
import Button from 'src/components/Button';
import { updateFilterSet } from 'src/dashboard/actions/nativeFilters';
import { WarningOutlined } from '@ant-design/icons';
import Icons from 'src/components/Icons';
import { ActionButtons } from './Footer';
import { useNativeFiltersDataMask, useFilters, useFilterSets } from '../state';
import { APPLY_FILTERS_HINT, findExistingFilterSet } from './utils';
Expand Down Expand Up @@ -160,7 +160,7 @@ const EditSection: FC<EditSectionProps> = ({
</ActionButtons>
{isDuplicateFilterSet && (
<Warning mark>
<WarningOutlined />
<Icons.WarningOutlined iconSize="m" />
{t('This filter set is identical to: "%s"', foundFilterSet?.name)}
</Warning>
)}
Expand Down
Loading

0 comments on commit 065b288

Please sign in to comment.