Skip to content

Commit

Permalink
fix(explore): DndColumnSelect not handling controls with "multi: fals…
Browse files Browse the repository at this point in the history
…e" (apache#14737)

* fix(explore): DndColumnSelect not handling controls with multi={false}

* Implement translations for singular and plural cases

* Fix test
  • Loading branch information
kgabryje authored May 24, 2021
1 parent 6ef7891 commit 27c16d1
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* under the License.
*/
import React, { useState } from 'react';
import { tn } from '@superset-ui/core';
import { ColumnMeta } from '@superset-ui/chart-controls';
import { isEmpty } from 'lodash';
import { LabelProps } from 'src/explore/components/controls/DndColumnSelectControl/types';
Expand All @@ -28,7 +29,7 @@ import { DndItemType } from 'src/explore/components/DndItemType';
import { StyledColumnOption } from 'src/explore/components/optionRenderers';

export const DndColumnSelect = (props: LabelProps) => {
const { value, options } = props;
const { value, options, multi = true } = props;
const optionSelector = new OptionSelector(options, value);
const [values, setValues] = useState<ColumnMeta[]>(optionSelector.values);

Expand All @@ -44,6 +45,7 @@ export const DndColumnSelect = (props: LabelProps) => {
};

const canDrop = (item: DatasourcePanelDndItem) =>
(multi || optionSelector.values.length === 0) &&
!optionSelector.has((item.value as ColumnMeta).column_name);

const onClickClose = (index: number) => {
Expand Down Expand Up @@ -77,6 +79,8 @@ export const DndColumnSelect = (props: LabelProps) => {
canDrop={canDrop}
valuesRenderer={valuesRenderer}
accept={DndItemType.Column}
displayGhostButton={multi || optionSelector.values.length === 0}
ghostButtonText={tn('Drop column', 'Drop columns', multi ? 2 : 1)}
{...props}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,10 @@ const defaultProps = {

test('renders with default props', () => {
render(<DndMetricSelect {...defaultProps} />, { useDnd: true });
expect(screen.getByText('Drop column or metric')).toBeInTheDocument();
});

test('renders with default props and multi = true', () => {
render(<DndMetricSelect {...defaultProps} multi />, { useDnd: true });
expect(screen.getByText('Drop columns or metrics')).toBeInTheDocument();
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/

import React, { useCallback, useEffect, useMemo, useState } from 'react';
import { ensureIsArray, Metric, t } from '@superset-ui/core';
import { ensureIsArray, Metric, tn } from '@superset-ui/core';
import { ColumnMeta } from '@superset-ui/chart-controls';
import { isEqual } from 'lodash';
import { usePrevious } from 'src/common/hooks/usePrevious';
Expand Down Expand Up @@ -268,7 +268,11 @@ export const DndMetricSelect = (props: any) => {
canDrop={canDrop}
valuesRenderer={valuesRenderer}
accept={[DndItemType.Column, DndItemType.Metric]}
ghostButtonText={t('Drop columns or metrics')}
ghostButtonText={tn(
'Drop column or metric',
'Drop columns or metrics',
multi ? 2 : 1,
)}
displayGhostButton={multi || value.length === 0}
{...props}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export interface LabelProps<T = string[] | string> {
value?: T;
onChange: (value?: T) => void;
options: { string: ColumnMeta };
multi?: boolean;
}

export interface DndColumnSelectProps<
Expand Down

0 comments on commit 27c16d1

Please sign in to comment.