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

[OUR415-313] Adds prop/slot for the "Clear Search" button in the SearchResults component #261

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion app/components/search/Refinements/ClearSearchButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const ClearSearchButton = () => {
mobileFullWidth={false}
onClick={handleOnClick}
>
Clear Search
<span data-testid={"clear-search-button"}>Clear Search</span>
</Button>
);
};
Expand Down
1 change: 0 additions & 1 deletion app/components/search/SearchResults/SearchResult.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React, { forwardRef } from "react";
import { TransformedSearchHit } from "models";
import { Link } from "react-router-dom";
import { LabelTag } from "components/ui/LabelTag";
import { Tooltip } from "react-tippy";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note: Noticed this was unused.

import { formatPhoneNumber, renderAddressMetadata } from "utils";
import { removeAsterisksAndHashes } from "utils/strings";
import ReactMarkdown from "react-markdown";
Expand Down
33 changes: 33 additions & 0 deletions app/components/search/SearchResults/SearchResults.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import React from "react";
import { InstantSearch } from "react-instantsearch-core";
import { render, screen, waitFor } from "@testing-library/react";
import SearchResults from "components/search/SearchResults/SearchResults";
import { createSearchClient } from "../../../../test/helpers/createSearchClient";
import ClearSearchButton from "components/search/Refinements/ClearSearchButton";
Copy link
Collaborator

Choose a reason for hiding this comment

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

chore: let's remove this line


describe("SearchResults", () => {
test("renders a Clear Search button if passed as a param", async () => {
const searchClient = createSearchClient();

render(
<InstantSearch
searchClient={searchClient}
indexName="fake_test_search_index"
initialUiState={{
fake_test_search_index: {
query: "fake query",
},
}}
>
<SearchResults
mobileMapIsCollapsed={false}
clearSearchButton={<ClearSearchButton />}
/>
</InstantSearch>
);

await waitFor(() => {
expect(screen.getByTestId("clear-search-button")).toBeInTheDocument();
});
});
});
17 changes: 12 additions & 5 deletions app/components/search/SearchResults/SearchResults.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useCallback } from "react";
import React, { ReactNode, useCallback } from "react";
import { SearchMap } from "components/search/SearchMap/SearchMap";
Copy link
Collaborator

Choose a reason for hiding this comment

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

chore: let's remove this too. notice we've let a lot of warnings through. i have some capacity and am happy to submit a PR to get rid of them

import { SearchResult } from "components/search/SearchResults/SearchResult";
import {
Expand All @@ -11,18 +11,25 @@ import {
useSearchBox,
} from "react-instantsearch";
import styles from "./SearchResults.module.scss";
import ClearSearchButton from "../Refinements/ClearSearchButton";
import { Loader } from "components/ui";
import { Loader } from "components/ui/Loader";
import ResultsPagination from "components/search/Pagination/ResultsPagination";

export enum SearchMapActions {
SearchThisArea,
}

/**
* Renders the search results list and the map
*
* @property clearSearchButton Must be a ClearSearchButton component
* @returns
*/
const SearchResults = ({
mobileMapIsCollapsed,
clearSearchButton,
}: {
mobileMapIsCollapsed: boolean;
clearSearchButton?: ReactNode;
}) => {
const { refine: refinePagination } = usePagination();
const {
Expand Down Expand Up @@ -51,15 +58,15 @@ const SearchResults = ({
<br /> Try a different location, filter, or search term.
</div>

{query && <ClearSearchButton />}
{query && clearSearchButton}
</div>
);

const searchResultsHeader = () => {
return (
<div className={styles.searchResultsHeader}>
<h2>{searchResults.nbHits} results</h2>
<ClearSearchButton />
{clearSearchButton}
</div>
);
};
Expand Down
6 changes: 5 additions & 1 deletion app/pages/SearchResultsPage/SearchResultsPage.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import React, { useState } from "react";
import SearchResults from "components/search/SearchResults/SearchResults";
import Sidebar from "components/search/Sidebar/Sidebar";
import { Header } from "components/search/Header/Header";

Check warning on line 4 in app/pages/SearchResultsPage/SearchResultsPage.tsx

View workflow job for this annotation

GitHub Actions / build_and_test_app

'Header' is defined but never used
import styles from "./SearchResultsPage.module.scss";
import { DEFAULT_AROUND_PRECISION, useAppContext } from "utils";
import { Configure } from "react-instantsearch-core";
import classNames from "classnames";
import ClearSearchButton from "components/search/Refinements/ClearSearchButton";

// NOTE: The .searchResultsPage is added plain so that it can be targeted by print-specific css
export const SearchResultsPage = () => {
Expand All @@ -28,7 +29,10 @@
/>

<div className={styles.results}>
<SearchResults mobileMapIsCollapsed={isMapCollapsed} />
<SearchResults
mobileMapIsCollapsed={isMapCollapsed}
clearSearchButton={<ClearSearchButton />}
/>
</div>
</div>
</div>
Expand Down
3 changes: 3 additions & 0 deletions jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ const config: Config = {
moduleNameMapper: {
// Ensures style imports don't break tests
"\\.(css|scss)$": "<rootDir>/test/jest/__mocks__/styleMock.ts",
"react-markdown": "<rootDir>/test/jest/__mocks__/react-markdown.tsx",
"\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$":
"<rootDir>/test/jest/__mocks__/fileMock.js",
},

// An array of regexp pattern strings, matched against all module paths before considered 'visible' to the module loader
Expand Down
2 changes: 1 addition & 1 deletion test/helpers/createSearchClient.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
interface Options {
[key: string]: any;

Check warning on line 2 in test/helpers/createSearchClient.ts

View workflow job for this annotation

GitHub Actions / build_and_test_app

Unexpected any. Specify a different type
}

/**
Expand Down Expand Up @@ -30,9 +30,9 @@
* @param options Additional customizations of the search response
Copy link
Collaborator

Choose a reason for hiding this comment

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

question (if-minor): Is this type any because it could be anything? Hoping to avoid type any as much as possible, and if it is needed, we should add a lint ignore comment (it wouldn't let me comment on the actual line (line 2)

* @returns
*/
export function createSearchClient(options: Options) {
export function createSearchClient(options?: Options) {
return {
search: (requests: any) =>

Check warning on line 35 in test/helpers/createSearchClient.ts

View workflow job for this annotation

GitHub Actions / build_and_test_app

Unexpected any. Specify a different type
Promise.resolve({
results: requests.map(() => ({
hits: [],
Expand Down
3 changes: 3 additions & 0 deletions test/jest/__mocks__/fileMock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Webpack is not available in tests to handle the css import parsing and handling. Our test config tells jest to
// replace any style imports it sees with this empty object.
export default "";

Check warning on line 3 in test/jest/__mocks__/fileMock.ts

View workflow job for this annotation

GitHub Actions / build_and_test_app

Assign literal to a variable before exporting as module default
11 changes: 11 additions & 0 deletions test/jest/__mocks__/react-markdown.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import React from "react";

const ReactMarkdown = ({
children,
}: {
children: string | JSX.Element | JSX.Element[] | (() => JSX.Element);
}) => {
return <>{children}</>;
};

export default ReactMarkdown;
Loading