-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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: preview mode add keyboard shortcuts #12968
Conversation
Run & review this pull request in StackBlitz Codeflow. |
Hi! Thanks for PR, we will discuss if we want to go forward with this feature in a coming meeting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would like to include in Vite 4.4! I can take care of this refactor if you don't have time at the moment.
packages/vite/src/node/shortcuts.ts
Outdated
action: (server: PreviewServer) => void | ||
} | ||
|
||
export function bindPreviewShortcuts(server: PreviewServer): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's factorise this code a bit with something like that:
type CLIShortcut<Server> = {
key: string
description: string
action: (server: Server) => void
}
function bindShortcuts<Server>(server: Server, shortcuts: CLIShortcut<Server>[]): void {
// almost exactly this code below
}
export function bindPreviewShortcuts(server: PreviewServer): void {
bindShortcuts(server, previewShortcuts)
}
type PreviewShortcut = CLIShortcut<PreviewServer>
And then rename the other usage in this file to add a dev prefix
It would be really nice if you could refactor it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change my mind, there is also the close function that differs.
Given that both are in the same file, I don't think other things will diverge. Factorisation starts at 3 they said.
We could also review the usage of setRawMode
as suggested by @bluwy
When i use
export { bindShortcuts, bindPreviewShortcuts } from './shortcuts'
import { createServer, bindShortcuts } from 'vite'
...
const server = await createServer(...);
await server.listen();
bindShortcuts(server) |
@deot That sounds like something we can expose, but I don't think this is the right place to request for it. I'd suggest opening a feature request, or a PR directly. @ArnaudBarre I wanted to merge this, but there's too many duplicated parts for me 😅 I went ahead and refactored it. Appreciate if you can review it again. |
Co-authored-by: bluwy <bjornlu.dev@gmail.com>
Description
Add
o
andq
shortcut keys for preview mode.Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).