Skip to content

Commit

Permalink
Dropdown param search fix (getredash#5304)
Browse files Browse the repository at this point in the history
* fixed QueryBasedParamterInput optionFilterProp

* added optionFilterProp fallback for SelectWithVirtualScroll

* simplified syntax

* removed optionFilterProp from QueryBasedParameterInput.jsx

Co-authored-by: Gabriel Dutra <nesk.frz@gmail.com>

* restricted SelectWithVirtualScroll props

* Added e2e test for parameter filters

* moved filter assertion to more suitable place

* created helper for option filter prop assertion

* moved option filter prop assertion to proper place, added result update assertion

* refactor openAndSearchAntdDropdown helper

* Fix parameter_spec

Co-authored-by: Gabriel Dutra <nesk.frz@gmail.com>
  • Loading branch information
2 people authored and halx4 committed Jan 28, 2021
1 parent 3093f82 commit f32cc8c
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 9 deletions.
2 changes: 0 additions & 2 deletions client/app/components/ParameterValueInput.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ class ParameterValueInput extends React.Component {
<SelectWithVirtualScroll
className={this.props.className}
mode={parameter.multiValuesOptions ? "multiple" : "default"}
optionFilterProp="children"
value={normalize(value)}
onChange={this.onSelect}
options={map(enumOptionsArray, opt => ({ label: String(opt), value: opt }))}
Expand All @@ -120,7 +119,6 @@ class ParameterValueInput extends React.Component {
<QueryBasedParameterInput
className={this.props.className}
mode={parameter.multiValuesOptions ? "multiple" : "default"}
optionFilterProp="children"
parameter={parameter}
value={value}
queryId={queryId}
Expand Down
1 change: 0 additions & 1 deletion client/app/components/QueryBasedParameterInput.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ export default class QueryBasedParameterInput extends React.Component {
value={this.state.value}
onChange={onSelect}
options={map(options, ({ value, name }) => ({ label: String(name), value }))}
optionFilterProp="children"
showSearch
showArrow
notFoundContent={isEmpty(options) ? "No options available" : null}
Expand Down
11 changes: 9 additions & 2 deletions client/app/components/SelectWithVirtualScroll.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ interface VirtualScrollLabeledValue extends LabeledValue {
label: string;
}

interface VirtualScrollSelectProps extends SelectProps<string> {
interface VirtualScrollSelectProps extends Omit<SelectProps<string>, "optionFilterProp" | "children"> {
options: Array<VirtualScrollLabeledValue>;
}
function SelectWithVirtualScroll({ options, ...props }: VirtualScrollSelectProps): JSX.Element {
Expand All @@ -32,7 +32,14 @@ function SelectWithVirtualScroll({ options, ...props }: VirtualScrollSelectProps
return false;
}, [options]);

return <AntdSelect<string> dropdownMatchSelectWidth={dropdownMatchSelectWidth} options={options} {...props} />;
return (
<AntdSelect<string>
dropdownMatchSelectWidth={dropdownMatchSelectWidth}
options={options}
optionFilterProp="label" // as this component expects "options" prop
{...props}
/>
);
}

export default SelectWithVirtualScroll;
32 changes: 28 additions & 4 deletions client/cypress/integration/query/parameter_spec.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
function openAndSearchAntdDropdown(testId, paramOption) {
cy.getByTestId(testId)
.find(".ant-select-selection-search-input")
.type(paramOption, { force: true });
}

describe("Parameter", () => {
const expectDirtyStateChange = edit => {
cy.getByTestId("ParameterName-test-parameter")
Expand Down Expand Up @@ -107,11 +113,13 @@ describe("Parameter", () => {
});

it("updates the results after selecting a value", () => {
cy.getByTestId("ParameterName-test-parameter")
.find(".ant-select")
.click();
openAndSearchAntdDropdown("ParameterName-test-parameter", "value2"); // asserts option filter prop

cy.contains(".ant-select-item-option", "value2").click();
// only the filtered option should be on the DOM
cy.get(".ant-select-item-option")
.should("have.length", 1)
.and("contain", "value2")
.click();

cy.getByTestId("ParameterApplyButton").click();
// ensure that query is being executed
Expand Down Expand Up @@ -219,6 +227,22 @@ describe("Parameter", () => {
});
});

it("updates the results after selecting a value", () => {
openAndSearchAntdDropdown("ParameterName-test-parameter", "value2"); // asserts option filter prop

// only the filtered option should be on the DOM
cy.get(".ant-select-item-option")
.should("have.length", 1)
.and("contain", "value2")
.click();

cy.getByTestId("ParameterApplyButton").click();
// ensure that query is being executed
cy.getByTestId("QueryExecutionStatus").should("exist");

cy.getByTestId("TableVisualization").should("contain", "2");
});

it("supports multi-selection", () => {
cy.clickThrough(`
ParameterSettings-test-parameter
Expand Down

0 comments on commit f32cc8c

Please sign in to comment.