Skip to content

Commit

Permalink
fix(quicksearch): opened pages don't scroll to top (#8911)
Browse files Browse the repository at this point in the history
* fix(quicksearch): opened pages don't scroll to top

https://mozilla-hub.atlassian.net/browse/MP-400

do this by removing use of useNavigate, and using "proper links"

also attempt to pass modifier keys to link click when using the keyboard
browser support is inconsistent, but it works in some cases

* fix test

* add glean click back
  • Loading branch information
LeoMcA authored May 23, 2023
1 parent 04acbe9 commit 964251b
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 61 deletions.
105 changes: 48 additions & 57 deletions client/src/search.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React, { useEffect, useMemo, useRef, useState } from "react";
import { useNavigate } from "react-router-dom";
import { useCombobox } from "downshift";
import useSWR from "swr";

Expand Down Expand Up @@ -32,7 +31,7 @@ type ResultItem = {
positions: Set<number>;
};

function quicksearchPing(input) {
function quicksearchPing(input: string) {
return `quick-search: ${input}`;
}

Expand Down Expand Up @@ -145,7 +144,6 @@ function BreadcrumbURI({
}

type InnerSearchNavigateWidgetProps = SearchProps & {
onResultPicked?: () => void;
defaultSelection: [number, number];
};

Expand Down Expand Up @@ -180,12 +178,10 @@ function InnerSearchNavigateWidget(props: InnerSearchNavigateWidgetProps) {
onChangeInputValue,
isFocused,
onChangeIsFocused,
onResultPicked,
defaultSelection,
} = props;

const formId = `${id}-form`;
const navigate = useNavigate();
const locale = useLocale();
const gleanClick = useGleanClick();

Expand Down Expand Up @@ -255,6 +251,10 @@ function InnerSearchNavigateWidget(props: InnerSearchNavigateWidgetProps) {
[searchPath]
);

const resultClick: React.MouseEventHandler<HTMLAnchorElement> = () => {
gleanClick(quicksearchPing(inputValue));
};

const {
getInputProps,
getItemProps,
Expand All @@ -275,22 +275,19 @@ function InnerSearchNavigateWidget(props: InnerSearchNavigateWidgetProps) {
isOpen: isFocused,
defaultIsOpen: isFocused,
defaultHighlightedIndex: 0,
onSelectedItemChange: ({ type, selectedItem }) => {
if (type !== useCombobox.stateChangeTypes.InputBlur && selectedItem) {
gleanClick(quicksearchPing(inputValue));
navigate(selectedItem.url);
onChangeInputValue("");
reset();
toggleMenu();
inputRef.current?.blur();
if (onResultPicked) {
onResultPicked();
}
window.scroll({
top: 0,
left: 0,
behavior: "smooth",
});
stateReducer: (state, { type, changes }) => {
// https://github.com/downshift-js/downshift/tree/v7.6.0/src/hooks/useCombobox#statereducer
// this prevents the menu from being closed when the user selects an item with 'Enter' or mouse
switch (type) {
case useCombobox.stateChangeTypes.InputKeyDownEnter:
case useCombobox.stateChangeTypes.ItemClick:
return {
...changes, // default Downshift new state changes on item selection.
isOpen: state.isOpen, // but keep menu open.
highlightedIndex: state.highlightedIndex, // with the item highlighted.
};
default:
return changes; // otherwise business as usual.
}
},
});
Expand All @@ -313,7 +310,7 @@ function InnerSearchNavigateWidget(props: InnerSearchNavigateWidgetProps) {
const item = resultItems[highlightedIndex];
if (item && preloadSupported()) {
const timeout = setTimeout(() => {
preload(`${item.url}/index.json`);
preload(`${item.url}`);
}, PRELOAD_WAIT_MS);
return () => {
clearTimeout(timeout);
Expand Down Expand Up @@ -369,15 +366,8 @@ function InnerSearchNavigateWidget(props: InnerSearchNavigateWidgetProps) {
>
<a
href={searchPath}
onClick={(event: React.MouseEvent) => {
if (event.ctrlKey || event.metaKey) {
// Open in new tab, don't navigate current tab.
event.stopPropagation();
} else {
// Open in same tab, navigate via combobox.
event.preventDefault();
}
}}
onClick={resultClick}
onAuxClick={resultClick}
tabIndex={-1}
>
No document titles found.
Expand All @@ -400,16 +390,8 @@ function InnerSearchNavigateWidget(props: InnerSearchNavigateWidgetProps) {
>
<a
href={item.url}
onClick={(event: React.MouseEvent) => {
if (event.ctrlKey || event.metaKey) {
// Open in new tab, don't navigate current tab.
gleanClick(quicksearchPing(inputValue));
event.stopPropagation();
} else {
// Open in same tab, navigate via combobox.
event.preventDefault();
}
}}
onClick={resultClick}
onAuxClick={resultClick}
tabIndex={-1}
>
{resultsWithHighlighting[i]}
Expand All @@ -428,15 +410,8 @@ function InnerSearchNavigateWidget(props: InnerSearchNavigateWidgetProps) {
>
<a
href={searchPath}
onClick={(event: React.MouseEvent) => {
if (event.ctrlKey || event.metaKey) {
// Open in new tab, don't navigate current tab.
event.stopPropagation();
} else {
// Open in same tab, navigate via combobox.
event.preventDefault();
}
}}
onClick={resultClick}
onAuxClick={resultClick}
tabIndex={-1}
>
Not seeing what you're searching for?
Expand Down Expand Up @@ -497,13 +472,29 @@ function InnerSearchNavigateWidget(props: InnerSearchNavigateWidgetProps) {
reset();
toggleMenu();
inputRef.current?.blur();
} else if (
event.key === "Enter" &&
inputValue.trim() &&
highlightedIndex === -1
) {
inputRef.current!.blur();
formRef.current!.submit();
} else if (event.key === "Enter") {
if (inputValue.trim() && highlightedIndex === -1) {
inputRef.current!.blur();
formRef.current!.submit();
} else {
const { ctrlKey, shiftKey, altKey, metaKey } = event;
document
.querySelector<HTMLAnchorElement>(
`#${id}-item-${highlightedIndex} a`
)
?.dispatchEvent(
new MouseEvent("click", {
// so react receives the event:
bubbles: true,
// we attempt to pass modifier keys through
// but browser support is incredibly varied:
ctrlKey,
shiftKey,
altKey,
metaKey,
})
);
}
}
},
onChange(event) {
Expand Down
4 changes: 1 addition & 3 deletions client/src/ui/molecules/search/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,9 @@ function useQueryParamState() {
export function Search({
id,
isHomepageSearch,
onResultPicked,
}: {
id: string;
isHomepageSearch?: boolean;
onResultPicked?: () => void;
}) {
const [value, setValue] = useQueryParamState();
const [isFocused, setIsFocused] = useState(false);
Expand All @@ -51,7 +49,7 @@ export function Search({
<div
className={isHomepageSearch ? "homepage-hero-search" : "header-search"}
>
<SearchNavigateWidget {...searchProps} onResultPicked={onResultPicked} />
<SearchNavigateWidget {...searchProps} />
</div>
);
}
2 changes: 1 addition & 1 deletion testing/tests/headless.search.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ test.describe("Autocomplete search", () => {
await page.waitForLoadState("networkidle");
expect(await page.innerText("h1")).toBe("<foo>: A test tag");
// Should have been redirected too...
expect(page.url()).toBe(testURL("/en-US/docs/Web/Foo"));
expect(page.url()).toBe(testURL("/en-US/docs/Web/Foo/"));
});

test("find nothing by title search", async ({ page }) => {
Expand Down

0 comments on commit 964251b

Please sign in to comment.