Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Co-authored-by: Robin <robin.kehl@singular-it.de>
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
There was a problem hiding this comment.
Pull request overview
Implements a Cmd/Ctrl+K “command bar” (quick actions) feature and wires it into the app’s command registration and localization system.
Changes:
- Added a new
CommandBarmodal component with filtering + keyboard navigation. - Introduced a global command registry composable and initial command registrations (search + install/run/create copy actions).
- Added global keybinding (Cmd/Ctrl+K) and new i18n strings for command labels/descriptions.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
lunaria/files/zh-CN.json |
Adds Chinese translations for command bar placeholder and copy actions. |
lunaria/files/en-US.json |
Adds English translations for command bar placeholder and copy actions. |
i18n/locales/zh-CN.json |
Mirrors Chinese command bar/copy-action strings into app locale files. |
i18n/locales/en.json |
Mirrors English command bar/copy-action strings into app locale files. |
app/plugins/commands.ts |
Registers a global “search packages” command for the command bar. |
app/composables/useCommandRegistry.ts |
Adds the command registry API and scoped/global registration helpers. |
app/components/InstallCommandTerminal.vue |
Registers scoped commands for copying install/run/create commands. |
app/components/CommandBar.vue |
New command bar UI (modal) + filtering, selection, triggering, and input/select flows. |
app/app.vue |
Adds global Cmd/Ctrl+K handler and mounts CommandBar. |
|
hey @Gianthard-cyh sorry this PR fell through the cracks a bit. would you still like to see this through or would you prefer for someone to take it over? |
📝 WalkthroughWalkthroughThis change introduces a command bar system triggered by Ctrl/Cmd+K. A new CommandBar component provides a modal command palette with search input, keyboard navigation support, and command filtering. A composable-based command registry system enables both global and lifecycle-bound command registration. The root application integrates the CommandBar and keyboard shortcut handler. A packages:search command is registered that navigates to search or focuses existing search input. Package-related commands in Terminal/Install.vue register installation, run, and create actions. Translations are added across English and Chinese locales for command UI strings, and accessibility tests verify CommandBar compliance. Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
app/components/Terminal/Install.vue (2)
99-106: Inconsistent command ID prefix:package:installvspackages:copy-run/packages:copy-create.The first command uses the singular
package:prefix while the other two use pluralpackages:. Pick one convention for consistency. The global command in the plugin also usespackages:search, sopackages:(plural) seems to be the intended namespace.♻️ Suggested fix
registerScopedCommand({ - id: 'package:install', + id: 'packages:copy-install', name: t('command.copy_install'), description: t('command.copy_install_desc'),
5-6: Minor:useI18n()vs auto-imported$t().The rest of the codebase uses the auto-imported
$t()global in<script setup>rather than destructuringuseI18n(). This works but is a minor deviation from the established pattern. Based on learnings, in this Nuxt 4 project with nuxtjs/i18n v10,$t()is auto-imported and works correctly inside<script setup>callbacks, and this is the established pattern across pages.app/plugins/commands.ts (1)
11-22: Minor duplication with the/keyboard shortcut handler inapp.vue.The search-input focus/navigate logic here is nearly identical to the
/key handler inapp.vue(lines 52–68). Not blocking, but if this pattern grows you might want to extract a sharedfocusOrNavigateToSearch()utility.app/components/CommandBar.vue (1)
57-61:anytypes inViewStateweaken type safety.
items: any[]andresolve: (val: any) => voidin theSELECTvariant lose type information. While full generics on a union ref are awkward, at minimum you could useCommand(or a dedicatedSelectIteminterface with{ id: string; name: string }) for the items and resolve value to keep the contract explicit. As per coding guidelines,**/*.{ts,tsx,vue}files should ensure strictly type-safe code.
| <div | ||
| class="fixed inset-0 z-[1000] flex items-start justify-center bg-black/50 backdrop-blur-sm" | ||
| v-show="show" | ||
| > |
There was a problem hiding this comment.
Clicking the backdrop does not close the modal.
Users expect that clicking outside a modal dialogue dismisses it. The overlay div has no click handler. Without this, keyboard-only close (Escape) is the only way out, and pointer users are left stranded.
Proposed fix
class="fixed inset-0 z-[1000] flex items-start justify-center bg-black/50 backdrop-blur-sm"
v-show="show"
+ `@click.self`="close"
>Using @click.self ensures only clicks directly on the backdrop (not bubbled from the inner container) trigger close.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div | |
| class="fixed inset-0 z-[1000] flex items-start justify-center bg-black/50 backdrop-blur-sm" | |
| v-show="show" | |
| > | |
| <div | |
| class="fixed inset-0 z-[1000] flex items-start justify-center bg-black/50 backdrop-blur-sm" | |
| v-show="show" | |
| `@click.self`="close" | |
| > |
| <div class="w-xl max-h-lg overflow-auto" v-if="view.type != 'INPUT'"> | ||
| <div | ||
| v-for="item in filteredCmdList" | ||
| :key="item.id" | ||
| class="px-4 py-2 not-first:mt-2 hover:bg-bg-elevated select-none cursor-pointer rounded-md transition" | ||
| :class="{ | ||
| 'bg-bg-subtle': item.id === selected, | ||
| 'trigger-anim': item.id === triggeringId, | ||
| }" | ||
| @click="onTrigger(item.id)" | ||
| > | ||
| <div class="text-fg">{{ item.name }}</div> | ||
| <div class="text-fg-subtle text-sm">{{ item.description }}</div> | ||
| </div> |
There was a problem hiding this comment.
Command list lacks ARIA listbox semantics.
The filtered command list is keyboard-navigable via ArrowUp/Down, but there is no role="listbox" on the container or role="option" on items, and no aria-activedescendant on the input to link the focused option. Screen readers will not announce the selected command.
Sketch of the fix
- <div class="w-xl max-h-lg overflow-auto" v-if="view.type != 'INPUT'">
+ <div class="w-xl max-h-lg overflow-auto" role="listbox" id="command-listbox" v-if="view.type !== 'INPUT'">
<div
v-for="item in filteredCmdList"
:key="item.id"
+ :id="`cmd-${item.id}`"
+ role="option"
+ :aria-selected="item.id === selected"
class="px-4 py-2 not-first:mt-2 hover:bg-bg-elevated select-none cursor-pointer rounded-md transition"Also add aria-activedescendant to the input, bound to `cmd-${selected}`, and aria-controls="command-listbox".
| const filteredCmdList = computed(() => { | ||
| if (view.value.type === 'INPUT') { | ||
| return [] | ||
| } | ||
|
|
||
| const list = view.value.type === 'SELECT' ? view.value.items : commands.value | ||
|
|
||
| if (!inputVal.value) { | ||
| return list | ||
| } | ||
| const filter = inputVal.value.trim().toLowerCase() | ||
| return list.filter( | ||
| (item: any) => | ||
| item.name.toLowerCase().includes(filter) || | ||
| item.description?.toLowerCase().includes(filter) || | ||
| item.id.includes(filter), | ||
| ) | ||
| }) |
There was a problem hiding this comment.
filteredCmdList accesses .name / .description on any items without guards.
In SELECT mode, view.value.items is typed any[], so item.name may not exist. If a select handler provides items without a name field, this will render undefined and toLowerCase() will throw at runtime (Line 106).
Either tighten the item type (preferred) or add a guard:
Defensive guard (if typing is deferred)
return list.filter(
- (item: any) =>
- item.name.toLowerCase().includes(filter) ||
- item.description?.toLowerCase().includes(filter) ||
+ (item: { id: string; name?: string; description?: string }) =>
+ item.name?.toLowerCase().includes(filter) ||
+ item.description?.toLowerCase().includes(filter) ||
item.id.includes(filter),
)| function onTrigger(id: string) { | ||
| triggeringId.value = id | ||
|
|
||
| if (view.value.type === 'ROOT') { | ||
| const selectedItem = filteredCmdList.value.find((item: any) => item.id === id) | ||
| selectedItem?.handler?.(cmdCtx) | ||
| setTimeout(() => { | ||
| triggeringId.value = '' | ||
| if (view.value.type === 'ROOT') { | ||
| close() | ||
| } | ||
| }, 100) | ||
| } else if (view.value.type === 'INPUT') { | ||
| view.value.resolve(inputVal.value) | ||
| close() | ||
| } else if (view.value.type === 'SELECT') { | ||
| const selectedItem = filteredCmdList.value.find((item: any) => item.id === id) | ||
| view.value.resolve(selectedItem) | ||
| close() | ||
| } | ||
| } |
There was a problem hiding this comment.
Handler result is fire-and-forget — unhandled rejections will be silent.
selectedItem?.handler?.(cmdCtx) returns a Promise<void> but is neither awaited nor .catch()-ed. If a handler throws after the ctx.input() / ctx.select() call, the rejection is swallowed. At minimum, add a .catch() to log/surface the error.
Proposed fix
- selectedItem?.handler?.(cmdCtx)
+ selectedItem?.handler?.(cmdCtx).catch((err: unknown) => {
+ console.error(`[CommandBar] handler "${id}" failed:`, err)
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function onTrigger(id: string) { | |
| triggeringId.value = id | |
| if (view.value.type === 'ROOT') { | |
| const selectedItem = filteredCmdList.value.find((item: any) => item.id === id) | |
| selectedItem?.handler?.(cmdCtx) | |
| setTimeout(() => { | |
| triggeringId.value = '' | |
| if (view.value.type === 'ROOT') { | |
| close() | |
| } | |
| }, 100) | |
| } else if (view.value.type === 'INPUT') { | |
| view.value.resolve(inputVal.value) | |
| close() | |
| } else if (view.value.type === 'SELECT') { | |
| const selectedItem = filteredCmdList.value.find((item: any) => item.id === id) | |
| view.value.resolve(selectedItem) | |
| close() | |
| } | |
| } | |
| function onTrigger(id: string) { | |
| triggeringId.value = id | |
| if (view.value.type === 'ROOT') { | |
| const selectedItem = filteredCmdList.value.find((item: any) => item.id === id) | |
| selectedItem?.handler?.(cmdCtx)?.catch((err: unknown) => { | |
| console.error(`[CommandBar] handler "${id}" failed:`, err) | |
| }) | |
| setTimeout(() => { | |
| triggeringId.value = '' | |
| if (view.value.type === 'ROOT') { | |
| close() | |
| } | |
| }, 100) | |
| } else if (view.value.type === 'INPUT') { | |
| view.value.resolve(inputVal.value) | |
| close() | |
| } else if (view.value.type === 'SELECT') { | |
| const selectedItem = filteredCmdList.value.find((item: any) => item.id === id) | |
| view.value.resolve(selectedItem) | |
| close() | |
| } | |
| } |
| const handleKeydown = useThrottleFn((e: KeyboardEvent) => { | ||
| if (view.value.type === 'INPUT' && e.key === 'Enter') { | ||
| e.preventDefault() | ||
| onTrigger('') // Trigger for input doesn't need ID | ||
| return | ||
| } | ||
|
|
||
| if ((e.key === 'ArrowDown' || e.key === 'ArrowUp') && !filteredCmdList.value.length) { | ||
| e.preventDefault() | ||
| return | ||
| } | ||
|
|
||
| const currentIndex = filteredCmdList.value.findIndex((item: any) => item.id === selected.value) | ||
|
|
||
| if (e.key === 'ArrowDown') { | ||
| e.preventDefault() | ||
| const nextIndex = (currentIndex + 1) % filteredCmdList.value.length | ||
| selected.value = filteredCmdList.value[nextIndex]?.id || '' | ||
| } else if (e.key === 'ArrowUp') { | ||
| e.preventDefault() | ||
| const prevIndex = | ||
| (currentIndex - 1 + filteredCmdList.value.length) % filteredCmdList.value.length | ||
| selected.value = filteredCmdList.value[prevIndex]?.id || '' | ||
| } else if (e.key === 'Enter') { | ||
| e.preventDefault() | ||
| onTrigger(selected.value) | ||
| } else if (e.key === 'Escape') { | ||
| e.preventDefault() | ||
| close() | ||
| } | ||
| }, 50) |
There was a problem hiding this comment.
useThrottleFn at 50 ms throttles all keystrokes, including typing.
Because handleKeydown is the sole @keydown handler on the text input, every character the user types is also throttled to 50 ms. On fast typists this may drop characters or feel laggy. The throttle is presumably intended only for arrow-key navigation. Consider either:
- Moving the throttle inside the arrow/Enter branches only, or
- Returning early for printable characters before the throttle takes effect.
| export interface CommandContext { | ||
| input: (options: CommandInputOptions) => Promise<string | undefined> | ||
| select: <T>(options: CommandSelectOptions<T>) => Promise<T | undefined> | ||
| } |
There was a problem hiding this comment.
CommandContext promises can dangle if the modal is closed without resolution.
input() and select() return Promise<string | undefined> / Promise<T | undefined>, but the CommandBar's close() does not resolve these promises — it simply resets the view state. Any handler await-ing ctx.input(...) will hang indefinitely if the user presses Escape or clicks the backdrop.
Consider resolving with undefined when the modal closes (e.g., store the pending resolve in a ref and call resolve(undefined) in close()).
| export const useCommandRegistry = () => { | ||
| const commands = useState<Map<string, Command>>('commands', () => new Map()) | ||
|
|
||
| const register = (command: Command) => { | ||
| const serverCommand = { | ||
| ...command, | ||
| handler: undefined, | ||
| } | ||
| if (import.meta.server) { | ||
| commands.value.set(command.id, serverCommand) | ||
| } else { | ||
| commands.value.set(command.id, command) | ||
| } | ||
| return () => { | ||
| commands.value.delete(command.id) | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| register, | ||
| commands: computed(() => Array.from(commands.value.values())), | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Nuxt 3 useState Map serialization SSR hydration
💡 Result:
In Nuxt 3, useState() is persisted from SSR → client by serializing its value into the Nuxt payload. By default, this means you must store JSON-serializable data (plain objects/arrays/primitives). Putting a Map in useState can therefore fail (e.g., “Cannot stringify arbitrary non‑POJOs”) or hydrate inconsistently. [1][2]
Fix options
- Store a serializable shape, rebuild the
Map
// SSR-safe
const entries = useState<[string, number][]>('myMapEntries', () => [])
const myMap = computed(() => new Map(entries.value))- Enable rich payload serialization (supports
Map/Set/Date/etc.)
Nuxt added an experimental option that supports richer JS types in the SSR payload, including Map and Set: [3]
// nuxt.config.ts
export default defineNuxtConfig({
experimental: {
renderJsonPayloads: true
}
})- Custom payload serializer/deserializer
If you need custom class-like types (or want explicit control), Nuxt recommends adding a custom payload serializer/deserializer viadefinePayloadPlugin. [1]
Sources: Nuxt state management + JSON-serialization constraint for useState [2], useState troubleshooting + payload plugin mention [1], Nuxt payload enhancements (renderJsonPayloads supports Map/Set) [3].
[1] https://nuxt.com/docs/4.x/api/composables/use-state
[2] https://nuxt.com/docs/getting-started/state-management/
[3] https://dev.nuxt.com/blog/v3-4
🏁 Script executed:
# Check the actual file structure and content
fd useCommandRegistry.tsRepository: npmx-dev/npmx.dev
Length of output: 98
🏁 Script executed:
# Look for the full file to understand context
cat -n $(fd useCommandRegistry.ts)Repository: npmx-dev/npmx.dev
Length of output: 2435
🏁 Script executed:
# Check for payload plugins or SSR-related configuration
fd "payload" --type f | head -20Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
# Search for useState usage with Map or non-serializable objects
rg "useState.*Map|useState.*Set" -t ts -t tsxRepository: npmx-dev/npmx.dev
Length of output: 88
🏁 Script executed:
# Check for nuxt.config configuration
fd "nuxt.config" -t fRepository: npmx-dev/npmx.dev
Length of output: 95
🏁 Script executed:
# Look at the nuxt config to see if experimental.renderJsonPayloads is enabled
cat -n $(fd "nuxt.config" -t f)Repository: npmx-dev/npmx.dev
Length of output: 9998
🏁 Script executed:
# Check for any definePayloadPlugin usage
rg "definePayloadPlugin" -t ts -t tsxRepository: npmx-dev/npmx.dev
Length of output: 88
🏁 Script executed:
# Check Nuxt version in package.json to understand compatibility
cat package.json | jq '.dependencies | {nuxt, "@nuxt/framework"}' 2>/dev/null || rg '"nuxt"' package.json -A 1 -B 1Repository: npmx-dev/npmx.dev
Length of output: 109
🏁 Script executed:
# Check how commands are used/accessed in the codebase to understand impact
rg "\.commands" --type ts -A 2 -B 2 | head -50Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
# Check how the commands are actually used to understand the impact better
rg "useCommandRegistry\|registerGlobalCommand\|registerScopedCommand" --type ts -B 2 -A 5 | head -80Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
# Search more broadly for usage patterns
rg "commands" app/composables/ --type ts -B 3 -A 3Repository: npmx-dev/npmx.dev
Length of output: 3055
🏁 Script executed:
# Check if there are any payload plugins at all
find . -name "*payload*" -o -name "*serializ*" 2>/dev/null | grep -v node_modules | head -20Repository: npmx-dev/npmx.dev
Length of output: 43
Replace Map with Record<string, Command> for SSR serialisation safety.
Nuxt's useState() serialises its payload as JSON when transferring server state to the client. Map is not JSON-serialisable by default and will cause hydration issues. Since experimental.renderJsonPayloads is not enabled in your Nuxt config, use a plain object instead.
Recommended fix — use a plain object
- const commands = useState<Map<string, Command>>('commands', () => new Map())
+ const commands = useState<Record<string, Command>>('commands', () => ({}))
const register = (command: Command) => {
const serverCommand = {
...command,
handler: undefined,
}
if (import.meta.server) {
- commands.value.set(command.id, serverCommand)
+ commands.value[command.id] = serverCommand
} else {
- commands.value.set(command.id, command)
+ commands.value[command.id] = command
}
return () => {
- commands.value.delete(command.id)
+ delete commands.value[command.id]
}
}
return {
register,
- commands: computed(() => Array.from(commands.value.values())),
+ commands: computed(() => Object.values(commands.value)),
}Alternatively, you can enable experimental.renderJsonPayloads: true in nuxt.config.ts to support native Map serialisation (Nuxt 4.3.0 supports this).
| export const registerScopedCommand = (command: Command) => { | ||
| const { register } = useCommandRegistry() | ||
| let unregister: () => void | ||
|
|
||
| onMounted(() => { | ||
| unregister = register(command) | ||
| }) | ||
|
|
||
| onUnmounted(() => { | ||
| unregister() | ||
| }) |
There was a problem hiding this comment.
unregister is used uninitialised if onUnmounted fires without a prior onMounted.
This is an edge case (e.g., SSR-only or <Suspense> fallback teardown), but calling unregister() when it is undefined would throw.
Defensive initialisation
const { register } = useCommandRegistry()
- let unregister: () => void
+ let unregister: (() => void) | undefined
onMounted(() => {
unregister = register(command)
})
onUnmounted(() => {
- unregister()
+ unregister?.()
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const registerScopedCommand = (command: Command) => { | |
| const { register } = useCommandRegistry() | |
| let unregister: () => void | |
| onMounted(() => { | |
| unregister = register(command) | |
| }) | |
| onUnmounted(() => { | |
| unregister() | |
| }) | |
| export const registerScopedCommand = (command: Command) => { | |
| const { register } = useCommandRegistry() | |
| let unregister: (() => void) | undefined | |
| onMounted(() => { | |
| unregister = register(command) | |
| }) | |
| onUnmounted(() => { | |
| unregister?.() | |
| }) |
| register({ | ||
| id: 'packages:search', | ||
| name: t('command.package_search'), | ||
| description: t('command.package_search_desc'), | ||
| handler: async () => { | ||
| const searchInput = document.querySelector<HTMLInputElement>( | ||
| 'input[type="search"], input[name="q"]', | ||
| ) | ||
|
|
||
| if (searchInput) { | ||
| searchInput.focus() | ||
| return | ||
| } | ||
|
|
||
| router.push('/search') | ||
| }, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for Command type definition and the command registry implementation
rg -n 'interface Command|type Command' --type=ts -A10Repository: npmx-dev/npmx.dev
Length of output: 1803
🏁 Script executed:
#!/bin/bash
# Read the entire useCommandRegistry.ts file to see the register() implementation
cat -n app/composables/useCommandRegistry.tsRepository: npmx-dev/npmx.dev
Length of output: 2435
🏁 Script executed:
#!/bin/bash
# Read the entire commands.ts plugin file
cat -n app/plugins/commands.tsRepository: npmx-dev/npmx.dev
Length of output: 799
Command name and description won't update when the user changes locale.
The translation strings t('command.package_search') and t('command.package_search_desc') are evaluated once at plugin initialisation and stored as plain strings in the command registry. The Command interface defines name and description as static string types (not reactive), so if the user switches language, the command bar will continue displaying the original translations.
To fix this, either modify the Command interface to support ComputedRef<string> values for name and description, or add a watch listener to re-register the command when the locale changes.
@serhalp Thanks for checking! I’ll continue working on this PR and keep it updated. |
closes #81.
API Overview
There are two ways to register commands depending on their lifecycle:
registerGlobalCommand: Registers a command that persists globally. Use this for app-wide actions like "Search Packages", "Toggle Theme", or navigation. Usually registered inplugins/command.ts.registerScopedCommand: Registers a command that only exists while a component is mounted. Use this for context-aware actions like "Copy Install Command" (only relevant when viewing a package). It automatically unregisters when the component unmounts. You can safely use component variables (closures) inside the handler.Interactive UI (
ctx)The
handlerreceives actxobject to interact with the Command Palette UI. Both methods return aPromisethat resolves when the user finishes.ctx.input({ prompt }): Opens a text input.ctx.select({ prompt, items }): Opens a list for selection.Example Usage
Here is a quick snippet showing how to use them: