timc1 / kbar

fast, portable, and extensible cmd+k interface for your site
https://kbar.vercel.app
MIT License
4.84k stars 185 forks source link

Can't change KBarProvider actions after it's being first rendered #239

Closed yannbf closed 2 years ago

yannbf commented 2 years ago

Hey! I really love this library. I started using it on my website with a small logic to show navigation items but filter the one you're currently at.

e.g. If you have routes for Home, Blog, and About, don't show "Home" if you are in that page already. After you navigate, show "Home" again, and hide the navigation related to the page you navigated to.

Unfortunately it seems like once KBarProvider renders once, the values are cached and the list won't change anymore. I wonder if this is a bug in the library or my code.

You can reproduce it here:

  1. access https://yannbraga.dev/
  2. cmd K, notice "Home" is not there.
  3. Select "About me"
  4. cmd K again, notice that "Home" is still not there, and "About" is there.

The correct output would be that "Home" is there, and "About" was hidden.

Here's the filter logic in case it helps.

I'd love to hear your thoughts! Thank you!

hmu332233 commented 2 years ago

@yannbf Hello! I also had a similar issue, so I looked for a document (actions - Dynamic actions) I was able to dynamically control the actions using useRegisterActions 😄

Here my code,

  useRegisterActions(
    keywords.map(keyword => ({
      section: '',
      id: '[KEYWORD ID]',
      name: keyword,
      shortcut: [],
      keywords: '',
      icon: SearchIcon,
      perform: () => {},
    })
), [keywords]);

I think the code written in nextui might be helpful nextui - pages/index.tsx

yannbf commented 2 years ago

Oh wow thanks for that @hmu332233 !! After looking into your implementation, I ended up trying something else and it worked! I think it's slightly simpler because it will work for any other page I add

given my item looks like this:

{
      id: 'talksAction',
      name: 'Talks',
      shortcut: ['g', 't'],
      keywords: 'talk',
      section: 'Navigation',
      perform: () => router.push('/talks'),
      path: '/talks', // <-- I'll need this later
      icon: <Icon kind="github" />,
}

And the RenderResults element looks like:

function RenderResults() {
  const { results, rootActionId } = useDeepMatches()
  const router = useRouter()

  const filteredResults = results.reduce((acc, curr) => {
    const isCurrentPageNavItem =
      typeof curr === 'object' && curr.section === 'Navigation' && curr.path === router.pathname

    if (isCurrentPageNavItem) {
      return acc
    }

    return acc.concat(curr)
  }, [])

  return (
    <KBarResults
      items={filteredResults}
      onRender={({ item, active }) =>
        typeof item === 'string' ? (
          <div style={groupNameStyle}>{item}</div>
        ) : (
          <ResultItem action={item} active={active} currentRootActionId={rootActionId} />
        )
      }
    />
  )
}

Still, I think it would make sense to leave this ticket open as there might be some unnecessary caching going on?

timc1 commented 2 years ago

@yannbf hey! what unnecessary caching are you referring to? useRegisterActions should handle adding and removing actions as the component which renders them mounts/unmounts.

yannbf commented 2 years ago

Hey @timc1 thanks for answering! I was not using that hook, what I meant was that rendering KBarProvider once with 10 actions, for instance, would show 10 actions in the search bar. Then, let's say after a navigation, rerendering the KBarProvider with 8 actions instead, would still yield 10 actions.

Here's the link showing the logic with the bug + how I fixed it https://github.com/yannbf/braga.dev/commit/7ce7b1ed4ad1d4641711daaf4682193e70ea9ab1

timc1 commented 2 years ago

You should only be rendering KBarProvider one time at the root of your app, so navigating wouldn't be rendering a new instance of KBarProvider again – the initial actions you pass to KBarProvider is held in memory and actions can only be added/removed through useRegisterActions

yannbf commented 2 years ago

Alright! That makes sense. I'll be closing the issue. Thank you for your time and explanation!

rajatkulkarni95 commented 1 year ago

You should only be rendering KBarProvider one time at the root of your app

@timc1 what if there's a use case for different shortcuts to toggle different command bars? For example like vscode where Cmd + P opens the Go to File action, while Cmd + Shift + P open the Execute Command action.

I tried with 2 providers, and it works, but useKbar just takes the innermost provider. Any suggestions?