-
Notifications
You must be signed in to change notification settings - Fork 332
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
Add ability to customize quick open shortcut key bindings (fixes #84) #85
Add ability to customize quick open shortcut key bindings (fixes #84) #85
Conversation
@davej oh very nice of you! you know i love me some svelte, but i'd obviously love a lighter build step always, but given the choice between not having this feature and having it, i'll take it! please keep going and follow your judgment, and I'll happily take what I can get. |
index.js
Outdated
label: 'Settings', | ||
accelerator: 'CommandorControl+,', |
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.
- doesnt really need an accelerator right? its not something we expect people to use often
2a. we could move it up and close to name it Change Quick Open Shortcut since it only does one thing right now?
2b. alternative idea (which we can also reject independently), maybe we move the "Super Prompt Enter Key" functionality into the settings menu since that's also not a frequent thing
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.
No, doesn't really need an accelerator. Will move it up and change the name. Will leave 2b to be tackled in a different PR for now.
package.json
Outdated
"build": "npm run make", | ||
"buildAndSign": "NODE_ENV=sign npm run make", | ||
"publish": "electron-forge publish --arch arm64,x64", | ||
"lint": "echo \"No linting configured\"" | ||
"lint": "echo \"No linting configured\"", | ||
"vite:dev": "vite dev", | ||
"vite:build": "vite build", | ||
"vite:preview": "vite preview" |
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.
todo: make sure the new vite:build is incorporated in the build script, and works
src/components/Button.svelte
Outdated
class:block={!!block} | ||
class:large={size === 'large'} | ||
class:small={size === 'small'} | ||
class:primary={variant === 'primary'} | ||
class:secondary={variant === 'secondary'} | ||
class:danger={variant === 'danger'} | ||
class:text={variant === 'text'} | ||
class:link={variant === 'link'} | ||
on:click |
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.
holy god button batman
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.
(not a negative comment, its fine, ship 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.
Nah, I'll refactor all this stuff and request a code review at the end. Just wanted to get the go-ahead that this PR was actually desired before spending time refining this stuff.
wow thats above and beyond! thanks so much Dave! hopefully the conversion to vanilla js wasnt too onerous (use chatgpt lol?) when i pulled down this branch and tried locally, it didnt work - just was stuck at the recording stage i would normally open up devtools but cant do here. i also dunno if these errors that popped up on the terminal are indicative of whats going on |
Was ok actually. ChatGPT got me bootstrapped alright! Took a bit of finessing with the DOM mutations then.
Did you try typing a keyboard shortcut when recording? You make a good point though, it should be more obvious what you need to do. Just made a quick UX improvement. See vid below: CleanShot.2023-07-26.at.9.40.39.mp4
I don't see those errors. Possibly a conflict with OBS or virtual camera/mic app? Melvin-Abraham/Google-Assistant-Unofficial-Desktop-Client#128 |
ok brilliant. this now works for me! i'll happily take it |
This is so slick! |
feel like theres a biiit more styling and microcopy to sweat the details, but happy to ship this for now |
This PR still needs a lot of work + polish + design but functionally it works.
I'm pausing to make sure that this is something that is desired. I'm aware that this adds quite a lot to the build process which is not ideal but I already had these components created in Svelte from a different project. It would be significant extra work to convert these to vanilla JS components.
I'll wait for feedback on this draft before continuing with this PR. /cc @swyxio
Fixes: #84
CleanShot.2023-07-25.at.12.04.25.mp4
Note: The video above doesn't show the complete keybinding on-screen because Electron intercepts the final character press.