Skip to content

Commit

Permalink
chore: Select component refactoring - FilterControl - Iteration 5 (ap…
Browse files Browse the repository at this point in the history 
…ache#15777)

* Refactor Select for AdhocFilterEditPopoverSqlTabContent

* Refactor Selects

* Fix numeric options

* Clean up

* Fix Select value

* Add showSearch

* Display null label

* Implement StyledSelect

* Update superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/index.jsx

Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>

* Fix aria-label

* Revert MetricControls changes

* Update ariaLabel

* Reconcile with latest Select changes

Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
  • Loading branch information
@geido @michael-s-molina
2 people authored and Emmanuel Bavoux committed Nov 14, 2021
1 parent 1e8233a commit 7f67049
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 128 deletions.
2 changes: 1 addition & 1 deletion superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe('AdhocFilters', () => {
});

xit('Set simple adhoc filter', () => {
cy.get('[data-test=adhoc-filter-simple-value] .Select__control').click();
cy.get('[aria-label="Comparator option"] .Select__control').click();
cy.get('[data-test=adhoc-filter-simple-value] input[type=text]')
.focus()
.type('Jack{enter}', { delay: 20 });
Expand Down
5 changes: 0 additions & 5 deletions ...t-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,6 @@ const FilterPopoverContentContainer = styled.div`
max-width: none;
}
.filter-edit-clause-dropdown {
width: ${({ theme }) => theme.gridUnit * 30}px;
margin-right: ${({ theme }) => theme.gridUnit}px;
}
.filter-edit-clause-info {
font-size: ${({ theme }) => theme.typography.sizes.xs}px;
padding-left: ${({ theme }) => theme.gridUnit}px;
Expand Down
165 changes: 56 additions & 109 deletions ...xplore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/
import React, { useEffect, useState } from 'react';
import { NativeSelect as Select } from 'src/components/Select';
import { Select } from 'src/components';
import { t, SupersetClient, styled } from '@superset-ui/core';
import {
Operators,
Expand All @@ -36,25 +36,7 @@ import AdhocFilter, {
EXPRESSION_TYPES,
CLAUSES,
} from 'src/explore/components/controls/FilterControl/AdhocFilter';
import { Input, SelectProps } from 'src/common/components';

const SelectWithLabel = styled(Select)`
.ant-select-selector {
margin-bottom: ${({ theme }) => theme.gridUnit * 4}px;
}
.ant-select-selector::after {
content: '${(
pr: SelectProps<any> & {
labelText: string | boolean;
},
) => pr.labelText || '\\A0'}';
display: inline-block;
white-space: nowrap;
color: ${({ theme }) => theme.colors.grayscale.light1};
width: max-content;
}
`;
import { Input } from 'src/common/components';

export interface SimpleColumnType {
id: number;
Expand Down Expand Up @@ -132,10 +114,10 @@ export const useSimpleTabFilterProps = (props: Props) => {
HAVING_OPERATORS.indexOf(operator) === -1)
);
};
const onSubjectChange = (id: string | number) => {
const onSubjectChange = (id: string) => {
const option = props.options.find(
option =>
('id' in option && option.id === id) ||
('column_name' in option && option.column_name === id) ||
('optionName' in option && option.optionName === id),
);

Expand Down Expand Up @@ -222,18 +204,13 @@ export const useSimpleTabFilterProps = (props: Props) => {
};

const AdhocFilterEditPopoverSimpleTabContent: React.FC<Props> = props => {
const selectProps = {
name: 'select-column',
showSearch: true,
};
const {
onSubjectChange,
onOperatorChange,
isOperatorRelevant,
onComparatorChange,
} = useSimpleTabFilterProps(props);
const [suggestions, setSuggestions] = useState<Record<string, any>>([]);
const [currentSuggestionSearch, setCurrentSuggestionSearch] = useState('');
const [
loadingComparatorSuggestions,
setLoadingComparatorSuggestions,
Expand Down Expand Up @@ -279,10 +256,6 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC<Props> = props => {
<FilterDefinitionOption option={option} />
);

const clearSuggestionSearch = () => {
setCurrentSuggestionSearch('');
};

const getOptionsRemaining = () => {
const { comparator } = props.adhocFilter;
// if select is multi/value is array, we show the options not selected
Expand All @@ -299,7 +272,9 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC<Props> = props => {

let columns = props.options;
const { subject, operator, comparator, operatorId } = props.adhocFilter;

const subjectSelectProps = {
ariaLabel: t('Select subject'),
value: subject ?? undefined,
onChange: onSubjectChange,
notFoundContent: t(
Expand Down Expand Up @@ -332,115 +307,87 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC<Props> = props => {
'%s operator(s)',
OPERATORS_OPTIONS.filter(op => isOperatorRelevant(op, subject)).length,
),
value: OPERATOR_ENUM_TO_OPERATOR_TYPE[operatorId]?.display,
value: operatorId,
onChange: onOperatorChange,
autoFocus: !!subjectSelectProps.value && !operator,
name: 'select-operator',
ariaLabel: t('Select operator'),
};

const shouldFocusComparator =
!!subjectSelectProps.value && !!operatorSelectProps.value;

const comparatorSelectProps: SelectProps<any> & {
labelText: string | boolean;
} = {
const comparatorSelectProps = {
allowClear: true,
showSearch: true,
mode: MULTI_OPERATORS.has(operatorId) ? 'tags' : undefined,
tokenSeparators: [',', '\n', '\t', ';'],
allowNewOptions: true,
ariaLabel: t('Comparator option'),
mode: MULTI_OPERATORS.has(operatorId)
? ('multiple' as const)
: ('single' as const),
loading: loadingComparatorSuggestions,
value: comparator,
onChange: onComparatorChange,
notFoundContent: t('Type a value here'),
disabled: DISABLE_INPUT_OPERATORS.includes(operatorId),
placeholder: createSuggestionsPlaceholder(),
labelText:
comparator && comparator.length > 0 && createSuggestionsPlaceholder(),
autoFocus: shouldFocusComparator,
};

const labelText =
comparator && comparator.length > 0 && createSuggestionsPlaceholder();

const SelectWithLabel = styled(Select)`
.ant-select-selector::after {
content: ${() => labelText || '\\A0'};
display: inline-block;
white-space: nowrap;
color: ${({ theme }) => theme.colors.grayscale.light1};
width: max-content;
}
`;

return (
<>
<Select
css={theme => ({
marginTop: theme.gridUnit * 4,
marginBottom: theme.gridUnit * 4,
})}
{...selectProps}
options={columns.map(column => ({
value:
('column_name' in column && column.column_name) ||
('optionName' in column && column.optionName) ||
'',
label:
('saved_metric_name' in column && column.saved_metric_name) ||
('column_name' in column && column.column_name) ||
('label' in column && column.label),
key:
('id' in column && column.id) ||
('optionName' in column && column.optionName) ||
undefined,
customLabel: renderSubjectOptionLabel(column),
}))}
{...subjectSelectProps}
filterOption={(input, option) =>
option && option.filterBy
? option.filterBy.toLowerCase().indexOf(input.toLowerCase()) >= 0
: false
}
getPopupContainer={triggerNode => triggerNode.parentNode}
>
{columns.map(column => (
<Select.Option
value={
('id' in column && column.id) ||
('optionName' in column && column.optionName) ||
''
}
filterBy={
('saved_metric_name' in column && column.saved_metric_name) ||
('column_name' in column && column.column_name) ||
('label' in column && column.label)
}
key={
('id' in column && column.id) ||
('optionName' in column && column.optionName) ||
undefined
}
>
{renderSubjectOptionLabel(column)}
</Select.Option>
))}
</Select>
/>
<Select
css={theme => ({ marginBottom: theme.gridUnit * 4 })}
{...selectProps}
options={OPERATORS_OPTIONS.filter(op =>
isOperatorRelevant(op, subject),
).map(option => ({
value: option,
label: OPERATOR_ENUM_TO_OPERATOR_TYPE[option].display,
key: option,
}))}
{...operatorSelectProps}
filterOption={(input, option) =>
option && option.children
? option.children.toLowerCase().indexOf(input.toLowerCase()) >= 0
: false
}
getPopupContainer={triggerNode => triggerNode.parentNode}
>
{OPERATORS_OPTIONS.filter(op => isOperatorRelevant(op, subject)).map(
option => (
<Select.Option value={option} key={option}>
{OPERATOR_ENUM_TO_OPERATOR_TYPE[option].display}
</Select.Option>
),
)}
</Select>
/>
{MULTI_OPERATORS.has(operatorId) || suggestions.length > 0 ? (
<SelectWithLabel
data-test="adhoc-filter-simple-value"
options={suggestions.map((suggestion: string) => ({
value: suggestion,
label: String(suggestion),
}))}
{...comparatorSelectProps}
getPopupContainer={triggerNode => triggerNode.parentNode}
onSearch={val => setCurrentSuggestionSearch(val)}
onSelect={clearSuggestionSearch}
onBlur={clearSuggestionSearch}
>
{suggestions.map((suggestion: string) => (
<Select.Option value={suggestion} key={suggestion}>
{String(suggestion)}
</Select.Option>
))}

{/* enable selecting an option not included in suggestions */}
{currentSuggestionSearch &&
!suggestions.some(
(suggestion: string) => suggestion === currentSuggestionSearch,
) && (
<Select.Option value={currentSuggestionSearch}>
{currentSuggestionSearch}
</Select.Option>
)}
</SelectWithLabel>
/>
) : (
<Input
data-test="adhoc-filter-simple-value"
Expand Down
30 changes: 17 additions & 13 deletions ...c/explore/components/controls/FilterControl/AdhocFilterEditPopoverSqlTabContent/index.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
*/
import React from 'react';
import PropTypes from 'prop-types';
import { NativeSelect as Select } from 'src/components/Select';
import { t } from '@superset-ui/core';
import { Select } from 'src/components';
import { styled, t } from '@superset-ui/core';
import { SQLEditor } from 'src/components/AsyncAceEditor';
import sqlKeywords from 'src/SqlLab/utils/sqlKeywords';

Expand All @@ -44,6 +44,13 @@ const propTypes = {
activeKey: PropTypes.string.isRequired,
};

const StyledSelect = styled(Select)`
${({ theme }) => `
width: ${theme.gridUnit * 30}px;
marginRight: ${theme.gridUnit}px;
`}
`;

export default class AdhocFilterEditPopoverSqlTabContent extends React.Component {
constructor(props) {
super(props);
Expand All @@ -54,7 +61,7 @@ export default class AdhocFilterEditPopoverSqlTabContent extends React.Component
this.handleAceEditorRef = this.handleAceEditorRef.bind(this);

this.selectProps = {
name: 'select-column',
ariaLabel: t('Select column'),
};
}

Expand Down Expand Up @@ -111,22 +118,19 @@ export default class AdhocFilterEditPopoverSqlTabContent extends React.Component
})
.filter(Boolean),
);
const selectOptions = Object.keys(CLAUSES).map(clause => ({
label: clause,
value: clause,
}));

return (
<span>
<div className="filter-edit-clause-section">
<Select
<StyledSelect
options={selectOptions}
{...this.selectProps}
{...clauseSelectProps}
className="filter-edit-clause-dropdown"
getPopupContainer={triggerNode => triggerNode.parentNode}
>
{Object.keys(CLAUSES).map(clause => (
<Select.Option value={clause} key={clause}>
{clause}
</Select.Option>
))}
</Select>
/>
<span className="filter-edit-clause-info">
<strong>WHERE</strong> {t('Filters by columns')}
<br />
Expand Down

0 comments on commit 7f67049

Please sign in to comment.