From c3c70575ddb2fc7283ca9302a342910e91ae818a Mon Sep 17 00:00:00 2001 From: reglim Date: Tue, 11 Apr 2023 14:32:11 +0200 Subject: [PATCH] Improvement: Migrate Search Results from seperate page to home page I added a new SearchProvider that handles saving the current query, and provides a list of projects that contain that query. Fuzzy search is now handled by Fuse.js. fixes: #472 --- docat/docat/utils.py | 2 +- docat/tests/test_rename.py | 2 +- docat/tests/test_upload.py | 2 +- web/package.json | 1 + web/src/App.tsx | 12 +- web/src/components/SearchBar.tsx | 60 ++++--- web/src/components/SearchResults.tsx | 107 ------------- .../data-providers/ProjectDataProvider.tsx | 18 +-- web/src/data-providers/SearchProvider.tsx | 75 +++++++++ web/src/models/SearchResult.ts | 13 -- web/src/pages/Claim.tsx | 26 +-- web/src/pages/Delete.tsx | 2 +- web/src/pages/Home.tsx | 35 ++-- web/src/pages/Search.tsx | 81 ---------- web/src/repositories/ProjectRepository.ts | 60 ------- web/src/style/components/SearchBar.module.css | 18 +-- .../style/components/SearchResults.module.css | 27 ---- web/src/style/pages/Home.module.css | 6 + .../repositories/ProjectRepository.test.ts | 150 ------------------ web/yarn.lock | 5 + 20 files changed, 177 insertions(+), 525 deletions(-) delete mode 100644 web/src/components/SearchResults.tsx create mode 100644 web/src/data-providers/SearchProvider.tsx delete mode 100644 web/src/models/SearchResult.ts delete mode 100644 web/src/pages/Search.tsx delete mode 100644 web/src/style/components/SearchResults.module.css diff --git a/docat/docat/utils.py b/docat/docat/utils.py index 7b9e18d0a..945eff968 100644 --- a/docat/docat/utils.py +++ b/docat/docat/utils.py @@ -101,7 +101,7 @@ def is_forbidden_project_name(name: str) -> bool: a page on the docat website. """ name = name.lower().strip() - return name in ["upload", "claim", "delete", "search", "help"] + return name in ["upload", "claim", "delete", "help"] def get_all_projects(upload_folder_path: Path, include_hidden: bool) -> Projects: diff --git a/docat/tests/test_rename.py b/docat/tests/test_rename.py index 8a922b41b..99272f593 100644 --- a/docat/tests/test_rename.py +++ b/docat/tests/test_rename.py @@ -86,7 +86,7 @@ def test_rename_rejects_forbidden_project_name(client_with_claimed_project): assert create_response.status_code == 201 with patch("os.rename") as rename_mock: - for project_name in ["upload", "claim", "delete", "Search ", "help"]: + for project_name in ["upload", "claim", "Delete ", "help"]: rename_response = client_with_claimed_project.put(f"/api/some-project/rename/{project_name}", headers={"Docat-Api-Key": "1234"}) assert rename_response.status_code == 400 assert rename_response.json() == { diff --git a/docat/tests/test_upload.py b/docat/tests/test_upload.py index 8542894e6..5d28806af 100644 --- a/docat/tests/test_upload.py +++ b/docat/tests/test_upload.py @@ -159,7 +159,7 @@ def test_upload_rejects_forbidden_project_name(client_with_claimed_project): """ with patch("docat.app.remove_docs") as remove_mock: - for project_name in ["upload", "claim", "delete", "Search ", "help"]: + for project_name in ["upload", "claim", " Delete ", "help"]: response = client_with_claimed_project.post( f"/api/{project_name}/1.0.0", files={"file": ("index.html", io.BytesIO(b"

Hello World

"), "plain/text")} ) diff --git a/web/package.json b/web/package.json index 9718be9b9..e9b2d61e0 100644 --- a/web/package.json +++ b/web/package.json @@ -21,6 +21,7 @@ "@types/react-dom": "^18.0.10", "@typescript-eslint/parser": "^5.54.1", "eslint": "^8.0.1", + "fuse.js": "^6.6.2", "http-proxy-middleware": "^2.0.6", "lodash": "^4.17.21", "react": "^18.2.0", diff --git a/web/src/App.tsx b/web/src/App.tsx index 63691ac14..43a57bd26 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -9,11 +9,11 @@ import Help from './pages/Help' import Home from './pages/Home' import NotFound from './pages/NotFound' import Upload from './pages/Upload' -import Search from './pages/Search' import EscapeSlashForDocsPath from './pages/EscapeSlashForDocsPath' import { MessageBannerProvider } from './data-providers/MessageBannerProvider' +import { SearchProvider } from './data-providers/SearchProvider' -function App(): JSX.Element { +function App (): JSX.Element { const router = createHashRouter([ { path: '/', @@ -39,10 +39,6 @@ function App(): JSX.Element { path: 'help', element: }, - { - path: 'search', - element: - }, { path: ':project', children: [ @@ -83,7 +79,9 @@ function App(): JSX.Element { - + + + diff --git a/web/src/components/SearchBar.tsx b/web/src/components/SearchBar.tsx index 658518bd0..b09b552dd 100644 --- a/web/src/components/SearchBar.tsx +++ b/web/src/components/SearchBar.tsx @@ -1,33 +1,41 @@ +import _ from 'lodash' import { TextField } from '@mui/material' -import { useNavigate } from 'react-router-dom' -import React from 'react' +import React, { useCallback } from 'react' import styles from '../style/components/SearchBar.module.css' -import { Search } from '@mui/icons-material' +import { useSearch } from '../data-providers/SearchProvider' -export default function SearchBar(): JSX.Element { - const navigate = useNavigate() - const [searchQuery, setSearchQuery] = React.useState('') +export default function SearchBar (): JSX.Element { + const { query, setQuery } = useSearch() + const [searchQuery, setSearchQuery] = React.useState(query) + + const updateSearchQueryInDataProvider = useCallback( + _.debounce( + (query: string): void => { + setQuery(query) + }, 500), + []) + + const onSearch = (e: React.ChangeEvent): void => { + setSearchQuery(e.target.value) + updateSearchQueryInDataProvider.cancel() + updateSearchQueryInDataProvider(e.target.value) + } return ( - <> - navigate('/search')} - /> -
- setSearchQuery(e.target.value)} - onKeyDown={(e) => { - if (e.key === 'Enter') { - navigate(`/search?query=${searchQuery}`) - } - }} - variant="standard" - > -
- +
+ { + if (e.key === 'Enter') { + e.preventDefault() + setQuery(searchQuery) + } + }} + variant="standard" + > +
) } diff --git a/web/src/components/SearchResults.tsx b/web/src/components/SearchResults.tsx deleted file mode 100644 index 4ab86279d..000000000 --- a/web/src/components/SearchResults.tsx +++ /dev/null @@ -1,107 +0,0 @@ -import { debounce, uniqueId } from 'lodash' -import React, { useEffect, useMemo, useState } from 'react' -import { Link } from 'react-router-dom' -import { SearchResult } from '../models/SearchResult' - -import styles from '../style/components/SearchResults.module.css' - -interface Props { - searchQuery: string - results: SearchResult -} - -export default function SearchResults(props: Props): JSX.Element { - const [resultElements, setResultElements] = useState([]) - - useEffect(() => { - searchResultElements.cancel() - searchResultElements() - }, [props.results]) - - /** - * Used to create the result elements before updating the UI, - * as this can lag if there are many results. - * @param res ApiSearchResponse - * @returns Array of JSX.Element - */ - const searchResultElements = useMemo( - () => - debounce(() => { - const projects = props.results.projects.map((p) => ( - - )) - - const versions = props.results.versions.map((v) => ( - - )) - - setResultElements([...projects, ...versions]) - }, 1000), - [props.results] - ) - - /** - * Used to replace any unsafe characters in the displayed name - * with their html counterparts because we need to set the text as - * html to be able to highlight the search query. - * @param text content to be escaped - * @returns excaped text - */ - const escapeHtml = (text: string): string => { - return text - .replaceAll('&', '&') - .replaceAll('<', '<') - .replaceAll('>', '>') - .replaceAll('"', '"') - .replaceAll("'", ''') - } - - /** - * Used to highlight the query in the displayed link text. - * @param text link text - * @returns html string with highlighted query - */ - const highlighedText = (text: string): string => { - text = escapeHtml(text) - return text.replaceAll( - props.searchQuery, - `${ - props.searchQuery - }` - ) - } - - if (resultElements.length === 0) { - if ( - props.results.projects.length > 0 || - props.results.versions.length > 0 - ) { - return
- } - - if (props.searchQuery.trim().length === 0) { - return
No results
- } - return ( -
- No results for "{props.searchQuery}" -
- ) - } - - return
{resultElements}
-} diff --git a/web/src/data-providers/ProjectDataProvider.tsx b/web/src/data-providers/ProjectDataProvider.tsx index 736fa295c..0ad628e3d 100644 --- a/web/src/data-providers/ProjectDataProvider.tsx +++ b/web/src/data-providers/ProjectDataProvider.tsx @@ -7,18 +7,15 @@ import React, { createContext, useContext, useEffect, useState } from 'react' import ProjectsResponse, { Project } from '../models/ProjectsResponse' import { useMessageBanner } from './MessageBannerProvider' -import ProjectRepository from '../repositories/ProjectRepository' interface ProjectState { projects: Project[] | null - projectsWithHiddenVersions: Project[] | null loadingFailed: boolean reload: () => void } const Context = createContext({ projects: null, - projectsWithHiddenVersions: null, loadingFailed: false, reload: (): void => { console.warn('ProjectDataProvider not initialized') @@ -32,7 +29,7 @@ const Context = createContext({ * * If reloading is required, call the reload function. */ -export function ProjectDataProvider({ children }: any): JSX.Element { +export function ProjectDataProvider ({ children }: any): JSX.Element { const { showMessage } = useMessageBanner() const loadData = (): void => { @@ -47,9 +44,8 @@ export function ProjectDataProvider({ children }: any): JSX.Element { } const data: ProjectsResponse = await response.json() - setProjects({ - projects: ProjectRepository.filterHiddenVersions(data.projects), - projectsWithHiddenVersions: data.projects, + setState({ + projects: data.projects, loadingFailed: false, reload: loadData }) @@ -61,9 +57,8 @@ export function ProjectDataProvider({ children }: any): JSX.Element { type: 'error' }) - setProjects({ + setState({ projects: null, - projectsWithHiddenVersions: null, loadingFailed: true, reload: loadData }) @@ -71,9 +66,8 @@ export function ProjectDataProvider({ children }: any): JSX.Element { })() } - const [projects, setProjects] = useState({ + const [state, setState] = useState({ projects: null, - projectsWithHiddenVersions: null, loadingFailed: false, reload: loadData }) @@ -82,7 +76,7 @@ export function ProjectDataProvider({ children }: any): JSX.Element { loadData() }, []) - return {children} + return {children} } export const useProjects = (): ProjectState => useContext(Context) diff --git a/web/src/data-providers/SearchProvider.tsx b/web/src/data-providers/SearchProvider.tsx new file mode 100644 index 000000000..7b2e948c2 --- /dev/null +++ b/web/src/data-providers/SearchProvider.tsx @@ -0,0 +1,75 @@ +/* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-assignment */ +/* + We need any, because we don't know the type of the children, + and we need the return those children again which is an "unsafe return" +*/ + +import React, { createContext, useContext, useEffect, useState } from 'react' +import { Project } from '../models/ProjectsResponse' +import { useProjects } from './ProjectDataProvider' +import Fuse from 'fuse.js' + +interface SearchState { + filteredProjects: Project[] | null + query: string + setQuery: (query: string) => void +} + +const Context = createContext({ + filteredProjects: null, + query: '', + setQuery: (): void => { + console.warn('SearchDataProvider not initialized') + } +}) + +export function SearchProvider ({ children }: any): JSX.Element { + const { projects } = useProjects() + + const filterProjects = (query: string): Project[] | null => { + if (projects == null) { + return null + } + + if (query.trim() === '') { + return projects + } + + const fuse = new Fuse(projects, { + keys: ['name'], + includeScore: true + }) + + // sort by match score + return fuse + .search(query) + .sort((x, y) => (x.score ?? 0) - (y.score ?? 0)) + .map((result) => result.item) + } + + const setQuery = (query: string): void => { + setState({ + query, + filteredProjects: filterProjects(query), + setQuery + }) + } + + const [state, setState] = useState({ + filteredProjects: null, + query: '', + setQuery + }) + + useEffect(() => { + setState({ + query: '', + filteredProjects: filterProjects(''), + setQuery + }) + }, [projects]) + + return {children} +} + +export const useSearch = (): SearchState => useContext(Context) diff --git a/web/src/models/SearchResult.ts b/web/src/models/SearchResult.ts deleted file mode 100644 index 629aacef5..000000000 --- a/web/src/models/SearchResult.ts +++ /dev/null @@ -1,13 +0,0 @@ -export interface ProjectSearchResult { - name: string -} - -export interface VersionSearchResult { - project: string - version: string -} - -export interface SearchResult { - projects: ProjectSearchResult[] - versions: VersionSearchResult[] -} diff --git a/web/src/pages/Claim.tsx b/web/src/pages/Claim.tsx index 93d79b425..4f806aba5 100644 --- a/web/src/pages/Claim.tsx +++ b/web/src/pages/Claim.tsx @@ -7,8 +7,8 @@ import { useMessageBanner } from '../data-providers/MessageBannerProvider' import { useProjects } from '../data-providers/ProjectDataProvider' import ProjectRepository from '../repositories/ProjectRepository' -export default function Claim(): JSX.Element { - const { projectsWithHiddenVersions: projects, loadingFailed } = useProjects() +export default function Claim (): JSX.Element { + const { projects, loadingFailed } = useProjects() const { showMessage } = useMessageBanner() const [project, setProject] = useState('none') @@ -78,19 +78,19 @@ export default function Claim(): JSX.Element { {token !== '' ? ( - - {token} - + + {token} + ) : ( - <> + <> )}