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

feat: nested search #105

Merged
merged 19 commits into from
Nov 5, 2021
2 changes: 0 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,3 @@ jobs:
run: npm run lint
- name: tsc
run: npm run typecheck
- name: test
run: npm run test
98 changes: 75 additions & 23 deletions example/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import * as React from "react";
import { KBarAnimator } from "../../src/KBarAnimator";
import { KBarProvider } from "../../src/KBarContextProvider";
import KBarPortal from "../../src/KBarPortal";
import useMatches, { NO_GROUP } from "../../src/useMatches";
import { NO_GROUP } from "../../src/useMatches";
import useDeepMatches from "../../src/useDeepMatches";
import KBarPositioner from "../../src/KBarPositioner";
import KBarSearch from "../../src/KBarSearch";
import KBarResults from "../../src/KBarResults";
Expand All @@ -14,8 +15,9 @@ import Docs from "./Docs";
import SearchDocsActions from "./SearchDocsActions";
import { createAction } from "../../src/utils";
import { useAnalytics } from "./utils";
import { Action } from "../../src";
import Blog from "./Blog";
import { ActionImpl } from "../../src/action";
import { ActionId } from "../../src";

const searchStyle = {
padding: "12px 16px",
Expand Down Expand Up @@ -102,7 +104,7 @@ const App = () => {
{
id: "darkTheme",
name: "Dark",
keywords: "dark",
keywords: "dark theme",
section: "",
perform: () =>
document.documentElement.setAttribute("data-theme-dark", ""),
Expand All @@ -111,7 +113,7 @@ const App = () => {
{
id: "lightTheme",
name: "Light",
keywords: "light",
keywords: "light theme",
section: "",
perform: () =>
document.documentElement.removeAttribute("data-theme-dark"),
Expand All @@ -123,10 +125,7 @@ const App = () => {
<KBarPortal>
<KBarPositioner>
<KBarAnimator style={animatorStyle}>
<KBarSearch
style={searchStyle}
placeholder="Type a command or search…"
/>
<KBarSearch style={searchStyle} />
<RenderResults />
</KBarAnimator>
</KBarPositioner>
Expand All @@ -152,25 +151,33 @@ const App = () => {
};

function RenderResults() {
const groups = useMatches();
const flattened = React.useMemo(
() =>
groups.reduce((acc, curr) => {
acc.push(curr.name);
acc.push(...curr.actions);
return acc;
}, []),
[groups]
);
const { throttledGroups: matches, throttledRootActionId: rootActionId } =
useDeepMatches();

const flattened = React.useMemo(() => {
let res = [];
for (let i = 0; i < matches.length; i++) {
let match = matches[i];
if (match !== NO_GROUP) res.push(match.name);
for (let j = 0; j < match.actions.length; j++) {
res.push(match.actions[j]);
}
}
return res;
Copy link
Owner Author

Choose a reason for hiding this comment

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

Average run times with 100k results:

reduce: .75ms
flatMap: 9ms
for loop: .5ms

Copy link
Collaborator

Choose a reason for hiding this comment

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

for loops fo life

Copy link
Owner Author

Choose a reason for hiding this comment

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

Idea is for no one to have 100k actions but who knows, go crazy

Copy link
Collaborator

Choose a reason for hiding this comment

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

I still think this flattening should be contained within the lib :P

Copy link
Owner Author

@timc1 timc1 Nov 3, 2021

Choose a reason for hiding this comment

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

Makes sense, useDeepMatches can return the flattened results as users have to flatten the results regardless: a42585d

}, [matches]);

return (
<KBarResults
items={flattened.filter((i) => i !== NO_GROUP)}
items={flattened}
onRender={({ item, active }) =>
typeof item === "string" ? (
<div style={groupNameStyle}>{item}</div>
) : (
<ResultItem action={item} active={active} />
<ResultItem
action={item}
active={active}
currentRootActiveId={rootActionId}
/>
)
}
/>
Expand All @@ -182,12 +189,27 @@ const ResultItem = React.forwardRef(
{
action,
active,
currentRootActiveId,
}: {
action: Action;
action: ActionImpl;
active: boolean;
currentRootActiveId: ActionId;
},
ref: React.Ref<HTMLDivElement>
) => {
const ancestors = React.useMemo(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could ancestors not be provided by onRender ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

We can but we will get this behavior, notice the slight bit of ancestor flash when we hit backspace and go to the parent action:

Kapture.2021-11-03.at.13.54.08.mp4

Since we throttle the matching, the actions displayed and the currentRootActionId will not be synced perfectly. The current solution is to return the currentRootActionId alongside the matches so we ensure they're synced:

const { results, rootActionId } = useDeepMatches();

I may be missing something totally here bc I really don't like returning that rootActionId as part of useDeepMatches lol

let ancestors = [];
(function collect(action: ActionImpl) {
if (action.parent && action.parent.id !== currentRootActiveId) {
ancestors.push(action.parent);
if (action.parent.parent) {
collect(action.parent.parent);
}
}
})(action);
return ancestors;
}, [action, currentRootActiveId]);

return (
<div
ref={ref}
Expand All @@ -203,10 +225,39 @@ const ResultItem = React.forwardRef(
cursor: "pointer",
}}
>
<div style={{ display: "flex", gap: "8px", alignItems: "center" }}>
<div
style={{
display: "flex",
gap: "8px",
alignItems: "center",
fontSize: 14,
}}
>
{action.icon && action.icon}
<div style={{ display: "flex", flexDirection: "column" }}>
<span>{action.name}</span>
<div>
{ancestors.length > 0 &&
ancestors.map((ancestor) => (
<React.Fragment key={ancestor.id}>
<span
style={{
opacity: 0.5,
marginRight: 8,
}}
>
{ancestor.name}
</span>
<span
style={{
marginRight: 8,
}}
>
&rsaquo;
</span>
</React.Fragment>
))}
<span>{action.name}</span>
</div>
{action.subtitle && (
<span style={{ fontSize: 12 }}>{action.subtitle}</span>
)}
Expand All @@ -221,6 +272,7 @@ const ResultItem = React.forwardRef(
padding: "4px 6px",
background: "rgba(0 0 0 / .1)",
borderRadius: "4px",
fontSize: 14,
}}
>
{sc}
Expand Down
2 changes: 1 addition & 1 deletion example/src/Blog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const greatgrandchild = createAction({

export default function Blog() {
const [actions, setActions] = React.useState([
...Array.from(Array(4)).map((_, i) =>
...Array.from(Array(100000)).map((_, i) =>
createAction({
name: i.toString(),
shortcut: [],
Expand Down
2 changes: 1 addition & 1 deletion example/src/SearchDocsActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export default function SearchDocsActions() {
name: "Search docs…",
shortcut: ["?"],
keywords: "find",
section: "",
section: "Documentation",
}
: null,
[searchActions]
Expand Down
Loading