Skip to content

Commit

Permalink
Remove instant search from main search
Browse files Browse the repository at this point in the history
Some months ago, we added the ability to search through subtitles and
slide texts of events, among other new features and improvements. This
is great from the UX side, but slows down searching: Meilisearch needs
longer to respond, and the frontend has gotten a lot more complex, so
rendering the results also got more resource intensive.

In the past couple weeks, I tried to tackle this slow down by optimizing
various things across Tobira. But while these all helped, the instant
search in Tobira still does not feel truly instant. And at that point,
I personally find it rather annoying and janky to use. Then I rather
prefer a non-instant search with really fast results. That's what this
commit does.

This matches YouTube's behavior for example. Yes, YouTube has an instant
search, but it's simplified and only shows search suggestions! The main
search requires pressing <enter>. Getting rid of instant-search also
improves some long standing sub-optimal implementations we had.

While this surely requires some getting used to and feels like a
"removed feature", I think that the UX does not actually really suffer
from this. And factoring UI slowness into UX, I think it improves it,
especially on mobile phones.

We might want to add a simplified instant-search like on YouTube at some
later point.
  • Loading branch information
LukasKalbertodt committed Jan 27, 2025
1 parent 0ecc829 commit 7b67cec
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 82 deletions.
44 changes: 15 additions & 29 deletions frontend/src/layout/header/Search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,14 @@ import { HiOutlineSearch } from "react-icons/hi";
import { ProtoButton, screenWidthAtMost } from "@opencast/appkit";
import { LuX } from "react-icons/lu";

import { useRouter, useRouterState } from "../../router";
import { useRouter } from "../../router";
import {
handleCancelSearch,
SearchRoute,
isSearchActive,
isValidSearchItemType,
SEARCH_TIMINGS,
} from "../../routes/Search";
import { focusStyle } from "../../ui";
import { Spinner } from "@opencast/appkit";
import { currentRef } from "../../util";
import { BREAKPOINT as NAV_BREAKPOINT } from "../Navigation";
import { COLORS } from "../../color";
Expand All @@ -27,7 +25,6 @@ type SearchFieldProps = {
export const SearchField: React.FC<SearchFieldProps> = ({ variant }) => {
const { t } = useTranslation();
const router = useRouter();
const { isTransitioning } = useRouterState();
const ref = useRef<HTMLInputElement>(null);

// If the user is unknown, then we are still in the initial loading phase.
Expand Down Expand Up @@ -72,8 +69,6 @@ export const SearchField: React.FC<SearchFieldProps> = ({ variant }) => {
const iconStyle = { position: "absolute", right: paddingSpinner, top: paddingSpinner } as const;


const lastTimeout = useRef<ReturnType<typeof setTimeout> | undefined>(undefined);

const onSearchRoute = isSearchActive();
const getSearchParam = (searchParameter: string) => {
const searchParams = new URLSearchParams(document.location.search);
Expand All @@ -84,13 +79,17 @@ export const SearchField: React.FC<SearchFieldProps> = ({ variant }) => {
const defaultValue = getSearchParam("q");


const search = (expression: string) => {
const search = (q: string) => {
if (!(q in SEARCH_TIMINGS)) {
SEARCH_TIMINGS[q] = {};
}
SEARCH_TIMINGS[q].startSearch = window.performance.now();
const filters = {
itemType: isValidSearchItemType(getSearchParam("f")),
start: getSearchParam("start"),
end: getSearchParam("end"),
};
router.goto(SearchRoute.url({ query: expression, ...filters }), onSearchRoute);
router.goto(SearchRoute.url({ query: q, ...filters }));
};

return (
Expand All @@ -114,7 +113,6 @@ export const SearchField: React.FC<SearchFieldProps> = ({ variant }) => {
}} />
<form onSubmit={event => {
event.preventDefault();
clearTimeout(lastTimeout.current);
search(currentRef(ref).value);

// Hide mobile keyboard on enter. The mobile keyboard hides lots
Expand All @@ -140,19 +138,6 @@ export const SearchField: React.FC<SearchFieldProps> = ({ variant }) => {
// the search button in the header (mobile only). This
// only happens on non-search routes.
autoFocus={variant === "mobile" && !onSearchRoute}
onChange={e => {
const q = e.target.value;
if (!(q in SEARCH_TIMINGS)) {
SEARCH_TIMINGS[q] = {};
}
SEARCH_TIMINGS[q].input = window.performance.now();

clearTimeout(lastTimeout.current);
lastTimeout.current = setTimeout(() => {
SEARCH_TIMINGS[q].startSearch = window.performance.now();
search(e.target.value);
}, 30);
}}
css={{
flex: 1,
color: COLORS.neutral60,
Expand All @@ -178,12 +163,13 @@ export const SearchField: React.FC<SearchFieldProps> = ({ variant }) => {
/>
</label>
</form>
{isTransitioning && isSearchActive() && <Spinner
size={spinnerSize}
css={iconStyle}
/>}
{!isTransitioning && isSearchActive() && <ProtoButton
onClick={() => handleCancelSearch(router, ref)}
<ProtoButton
// Just clear the search input
onClick={() => {
const input = currentRef(ref);
input.value = "";
input.focus();
}}
css={{
":hover, :focus": {
color: COLORS.neutral90,
Expand All @@ -195,7 +181,7 @@ export const SearchField: React.FC<SearchFieldProps> = ({ variant }) => {
color: COLORS.neutral60,
...iconStyle,
}}
><LuX size={spinnerSize} css={{ display: "block" }} /></ProtoButton>}
><LuX size={spinnerSize} css={{ display: "block" }} /></ProtoButton>
</div>
);
};
21 changes: 3 additions & 18 deletions frontend/src/layout/header/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useEffect } from "react";
import React from "react";
import { LuArrowLeft, LuMenu, LuX } from "react-icons/lu";
import { HiOutlineSearch } from "react-icons/hi";
import { useTranslation } from "react-i18next";
Expand All @@ -12,8 +12,6 @@ import { SearchField } from "./Search";
import { Logo } from "./Logo";
import { ColorSchemeSettings, LanguageSettings, UserBox } from "./UserBox";
import { COLORS } from "../../color";
import { useRouter } from "../../router";
import { handleCancelSearch, isSearchActive, SearchRoute } from "../../routes/Search";


type Props = {
Expand All @@ -23,18 +21,7 @@ type Props = {

export const Header: React.FC<Props> = ({ hideNavIcon = false, loginMode = false }) => {
const menu = useMenu();
const router = useRouter();
const onNarrowScreen = window.matchMedia(`(max-width: ${NAV_BREAKPOINT}px)`).matches;
useEffect(() => (
router.listenAtNav(({ newRoute }) => {
if (onNarrowScreen && (menu.state === "search") !== (newRoute === SearchRoute)) {
menu.toggleMenu("search");
}
})
));

const onSearchRoute = isSearchActive();
const content = match((onSearchRoute && onNarrowScreen) ? "search" : menu.state, {
const content = match(menu.state, {
"closed": () => <DefaultMode hideNavIcon={hideNavIcon} />,
"search": () => <SearchMode />,
"burger": () => <OpenMenuMode />,
Expand Down Expand Up @@ -68,13 +55,11 @@ const LoginMode: React.FC = () => <>
const SearchMode: React.FC = () => {
const { t } = useTranslation();
const menu = useMenu();
const onSearchRoute = isSearchActive();
const router = useRouter();

return <>
<ActionIcon
title={t("general.action.back")}
onClick={() => onSearchRoute ? handleCancelSearch(router) : menu.close()}
onClick={() => menu.close()}
css={{ marginLeft: 8 }}
>
<LuArrowLeft />
Expand Down
29 changes: 0 additions & 29 deletions frontend/src/routes/Search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
import { LetterText } from "lucide-react";
import {
ReactNode,
RefObject,
startTransition,
Suspense,
useCallback,
Expand Down Expand Up @@ -234,7 +233,6 @@ const SearchPage: React.FC<Props> = ({ q, outcome }) => {
// eslint-disable-next-line no-console
console.table([{
q: q,
debounce: diff(info.input, info.startSearch),
routing: diff(info.startSearch, info.routeMatch),
query: diff(info.routeMatch, info.queryReturned),
backend: outcome.__typename === "SearchResults" ? outcome.duration : null,
Expand All @@ -243,15 +241,6 @@ const SearchPage: React.FC<Props> = ({ q, outcome }) => {
LAST_PRINTED_TIMINGS_QUERY = q;
});

useEffect(() => {
const handleEscape = ((ev: KeyboardEvent) => {
if (ev.key === "Escape") {
handleCancelSearch(router);
}
});
document.addEventListener("keyup", handleEscape);
return () => document.removeEventListener("keyup", handleEscape);
});

let body;
if (outcome.__typename === "EmptyQuery") {
Expand Down Expand Up @@ -1159,23 +1148,6 @@ const Item: React.FC<ItemProps> = ({ link, breakpoint = 0, children }) => (
</li>
);

// If a user initiated the search in Tobira (i.e. neither coming from an
// external link nor using the browser bar to manually visit the /~search route),
// we can redirect to the previous page. Otherwise we redirect to Tobira's homepage.
export const handleCancelSearch = ((router: RouterControl, ref?: RefObject<HTMLInputElement>) => {
if (ref?.current) {
// Why is this necessary? When a user reloads the search page and then navigates
// away within Tobira, the search input isn't cleared like it would be usually.
// So it needs to be done manually.
ref.current.value = "";
}
if (router.internalOrigin) {
window.history.back();
} else {
router.goto("/");
}
});

/**
* Slices a string with byte indices. Never cuts into UTF-8 chars, but
* arbitrarily decides in what output to place them.
Expand Down Expand Up @@ -1271,7 +1243,6 @@ const highlightCss = (color: string) => ({

// This is for profiling search performance. We might remove this later again.
export const SEARCH_TIMINGS: Record<string, {
input?: DOMHighResTimeStamp;
startSearch?: DOMHighResTimeStamp;
routeMatch?: DOMHighResTimeStamp;
queryReturned?: DOMHighResTimeStamp;
Expand Down
6 changes: 0 additions & 6 deletions frontend/src/ui/LoadingIndicator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { useRef } from "react";
import { Transition } from "react-transition-group";
import { match } from "@opencast/appkit";

import { isSearchActive } from "../routes/Search";
import { useRouterState } from "../router";
import { COLORS } from "../color";

Expand All @@ -12,11 +11,6 @@ export const LoadingIndicator: React.FC = () => {
const { isTransitioning } = useRouterState();
const ref = useRef<HTMLDivElement>(null);

// If search is active, there is a loading indicator next to the search input.
if (isSearchActive()) {
return null;
}

const START_DURATION = 1200;
const EXIT_DURATION = 150;

Expand Down
2 changes: 2 additions & 0 deletions frontend/tests/search.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ test("Search", async ({ page, browserName, standardData, activeSearchIndex }) =>

await test.step("Should allow search queries to be executed", async () => {
await searchField.fill(query);
await searchField.press("Enter");
await expect(page).toHaveURL(`~search?q=${query}`);
});

Expand Down Expand Up @@ -71,6 +72,7 @@ const startSearch = async (page: Page, query: string, startUrl?: string) => {
const searchField = page.getByPlaceholder("Search");
await searchField.click();
await searchField.fill(query);
await searchField.press("Enter");
await expect(page).toHaveURL(`~search?${new URLSearchParams({ q: query })}`);
};

Expand Down

0 comments on commit 7b67cec

Please sign in to comment.