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: sections/groups #59

Merged
merged 2 commits into from
Oct 20, 2021
Merged

feat: sections/groups #59

merged 2 commits into from
Oct 20, 2021

Conversation

timc1
Copy link
Owner

@timc1 timc1 commented Oct 13, 2021

Alternative solution to #49.
Closes #36.

Background

While working on the initial version of groups, I noticed the current API required too much leg work for the consumer; i.e., currently a developer can import the standard KBarResults component, which handles everything from keyboard navigation to rendering results.

However, if you wanted to segment results by groups, you'd have to write a component from scratch handling all pointer and keyboard events that KBarResults had built in. This requires the developer to write a lot of code – index management, event listeners, coordinating values from useKBar, etc.

For example, #49 introduced another component, KBarGroupedResults, but what happens when more feature requests roll in? How do we build an API that can easily enable developers to create any type of result view that they want, with all the complexity abstracted away?

Solution

From a high level, we want to:

  • Expose a very simple API
  • Implicitly handle index management, keyboard events so the developer does not have to

If a developer wanted to build sectioned/grouped results, they would do this:

return (
  <Results>
    <div>
      <p>Group 1</p>
      <ResultItem /> 
      <ResultItem /> 
    </div>
    <div>
      <p>Group 2</p>
      <ResultItem />
    </div>
  </Results>
)

wherein ResultItem can be rendered anywhere within Results, and events and index management will be implicitly handled.

We will introduce a few components/hooks:

useMatches: () => ActionGroup[]

This hook uses match-sorter under the hood to take the current list of actions and search query, and returns a filtered list of actions grouped by their action.section value. Additionally, as match-sorter returns a new object on every call, we slightly throttle the call to improve the overall search performance.

Results: ({ children }: { children: React.ReactNode }) => JSX.Element

All results must be wrapped with this components, which leverages a simple context implementation which implicitly handles all keyboard/pointer events alongside an index management solution inspired by @reach/descendants and use-descendants.

useResultItem: (action: Action) => { index: number; active: boolean; handlers: any; }

Every result rendered will need to use this hook, which works alongside Results to get the current index, whether it is active, and and event handlers that need to be spread onto the result.

function ResultItem(action: Action) {
  const { active, handlers } = useResultItem({ action });
  return (
    <div 
      style={{
        background: active ? "#eee" : "none"
      }}
      {...handlers}
    >
      {action.name}
    </div> 
  );
}

@vercel
Copy link

vercel bot commented Oct 13, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/timc/kbar/HkiZDG9FGSGNo7e5smKjSMQXaeZ3
✅ Preview: https://kbar-git-feat-improved-results-timc.vercel.app

children: React.ReactNode;
manager: Context;
}) {
props.manager.reset();
Copy link
Owner Author

Choose a reason for hiding this comment

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

Updates index for all nested useResultItems whenever children rerenders.

This was referenced Oct 13, 2021
setActiveIndex(0);
}, [search, currentRootActionId]);

const getHandlers = (index: number, cb: () => void) => ({
Copy link
Owner Author

Choose a reason for hiding this comment

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

TODO: Make sure results are visible to screen readers.

@timc1 timc1 changed the title feat: improved results api feat: sections/groups Oct 14, 2021
@timc1 timc1 marked this pull request as ready for review October 15, 2021 19:10
@timc1 timc1 requested a review from tommoor October 18, 2021 20:58
@tommoor
Copy link
Collaborator

tommoor commented Oct 18, 2021

My main bit of feedback on the interface is it seems like there is an implicit dependency between Results component and useMatches hook introduced here.

Would there be a downside of making Results expect a functional child that gets passed the results? This would prevent a couple of easy to make mistakes and self-document a little better:

return (
  <Results>
    {(groupedResults) => 
      groupedResults.map(...)
    }
  </Results>
)

@timc1
Copy link
Owner Author

timc1 commented Oct 19, 2021

My main bit of feedback on the interface is it seems like there is an implicit dependency between Results component and useMatches hook introduced here.

Would there be a downside of making Results expect a functional child that gets passed the results? This would prevent a couple of easy to make mistakes and self-document a little better:

We should keep the matches and Results/useResultItem separate – useMatches is a helper hook that will return a set of grouped results, but the user could just calculate their own results if they wanted to

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sectioned actions
2 participants