Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fiction list filter (PP-1110) #106

Merged
merged 6 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/components/AdvancedSearchBuilder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ export const fields = [
{ value: "audience", label: "audience" },
{ value: "author", label: "author" },
{ value: "title", label: "title" },
{
value: "fiction",
label: "fiction",
options: ["fiction", "nonfiction"],
operators: ["eq"],
},
];

export const operators = [
Expand Down
41 changes: 38 additions & 3 deletions src/components/AdvancedSearchFilterInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,23 @@ export default function AdvancedSearchFilterInput({

const handleKeyChange = (value) => {
setFilterKey(value);
const selected = fields.find((field) => field.value === value);

// Set the value to either the first option, or blank.
if (selected.options && selected.options.length) {
setFilterValue(selected.options[0]);
} else {
setFilterValue("");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There could be a problem here. This PR is introducing the potential for a reduced set of operators, so we'll also need to ensure that we've got a valid operator for the newly current field. Ideally, we'd only change the filterOp if the current one is not supported.

It would be useful to add a test simulating going from an op on a field with broader set ops to a field that doesn't support that op, then adding the filter, and finally verifying that the key, value, and op are as expected.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Thank you, totally missed this one.


// Update operator if the filter doesn't support it.
if (
selected.operators &&
selected.operators.length &&
!selected.operators.find((op) => op === filterOp)
) {
setFilterOp(selected.operators[0]);
}
};

const handleOpChange = () => {
Expand Down Expand Up @@ -64,6 +81,14 @@ export default function AdvancedSearchFilterInput({
};

const selectedField = fields.find((field) => field.value === filterKey);
const selectedFieldOperators = selectedField.operators
? operators.filter((op) => selectedField.operators.includes(op.value))
: operators;

const selectedFieldOptions = selectedField.options ?? [];
const selectedFieldElementType = selectedFieldOptions.length
? "select"
: "input";

return (
<form className="advanced-search-filter-input">
Expand All @@ -89,8 +114,9 @@ export default function AdvancedSearchFilterInput({
onChange={handleOpChange}
ref={opSelect}
value={filterOp}
className="filter-operator"
>
{operators.map(({ value, label }) => (
{selectedFieldOperators.map(({ value, label }) => (
<option
aria-selected={value === filterOp}
key={value}
Expand All @@ -104,14 +130,23 @@ export default function AdvancedSearchFilterInput({
<EditableInput
aria-label="filter value"
description={selectedField.helpText}
elementType="input"
elementType={selectedFieldElementType}
type="text"
optionalText={false}
placeholder={selectedField.placeholder}
ref={valueInput}
value={filterValue}
onChange={handleValueChange}
/>
className="filter-value"
>
{selectedFieldOptions.length === 0
? null
: selectedFieldOptions.map((value) => (
<option key={value} value={value}>
{value}
</option>
))}
</EditableInput>

<Button
className="inverted inline"
Expand Down
86 changes: 85 additions & 1 deletion src/components/__tests__/AdvancedSearchFilterInput-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,96 @@ describe("AdvancedSearchFilterInput", () => {
});
});

it("should restrict the operator selection, if the filter requests it", () => {
const fictionRadioButton = wrapper.find(
'input[type="radio"][value="fiction"]'
);
fictionRadioButton.getDOMNode().checked = true;
fictionRadioButton.simulate("change");

const options = wrapper.find(".filter-operator select > option");
const fictionField = fields.find((element) => {
return element.value === "fiction";
});

expect(options).to.have.length(fictionField.operators.length);
});

it("should use the currently selected operator when changing filters", () => {
const sourceFilterButton = wrapper.find(
'input[type="radio"][value="data_source"]'
);
const fictionFilterButton = wrapper.find(
'input[type="radio"][value="fiction"]'
);
const fictionField = fields.find((element) => {
return element.value === "fiction";
});

const operatorOptions = wrapper.find(".filter-operator select");
operatorOptions.getDOMNode().value = "regex";
operatorOptions.simulate("change");

sourceFilterButton.getDOMNode().checked = true;
sourceFilterButton.simulate("change");

expect(operatorOptions.getDOMNode().value).to.equal("regex");

// unless the operator is not supported by the filter.
fictionFilterButton.getDOMNode().checked = true;
fictionFilterButton.simulate("change");

expect(operatorOptions.getDOMNode().value).to.equal(
fictionField.operators[0]
);

// the new operator will be persisted when changing back to source filter
sourceFilterButton.getDOMNode().checked = true;
sourceFilterButton.simulate("change");

expect(operatorOptions.getDOMNode().value).to.equal(
fictionField.operators[0]
);
});

it("should render a text input for value entry", () => {
const valueInput = wrapper.find('input[type="text"]');
const valueSelect = wrapper.find(".filter-value select");
const valueInput = wrapper.find('.filter-value input[type="text"]');

expect(valueSelect).to.have.length(0);
expect(valueInput).to.have.length(1);
});

it("should render a select for value selection, if the filter requests it", () => {
const fictionRadioButton = wrapper.find(
'input[type="radio"][value="fiction"]'
);
fictionRadioButton.getDOMNode().checked = true;
fictionRadioButton.simulate("change");

const valueSelect = wrapper.find(".filter-value select");
const valueInput = wrapper.find('.filter-value input[type="text"]');

expect(valueSelect).to.have.length(1);
expect(valueInput).to.have.length(0);
});

it("should clear the current value when changing the filter", () => {
const valueInput = wrapper.find(".filter-value input");

valueInput.getDOMNode().value = "ABC";
valueInput.simulate("change");

const filterRadioButton = wrapper.find(
'input[type="radio"][value="data_source"]'
);

filterRadioButton.getDOMNode().checked = true;
filterRadioButton.simulate("change");

expect(valueInput.getDOMNode().value).to.equal("");
});

it("should render an Add Filter button", () => {
const button = wrapper.find("button");

Expand Down
12 changes: 10 additions & 2 deletions src/stylesheets/advanced_search.scss
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,16 @@
}
}

input[type="text"] {
width: 40em;
.filter-value {
input[type="text"],select {
width: 40em;
}
}

.filter-operator {
select {
width: 15em;
}
}

.filter-key-options {
Expand Down
Loading