[WIP] Implemented `keybind` module to handle keyboard shortcuts

StractOrg / stract

web search done right

https://stract.com

GNU Affero General Public License v3.0

2.13k stars 47 forks source link

[WIP] Implemented `keybind` module to handle keyboard shortcuts #186

Closed lamemakes closed 6 months ago

lamemakes commented 6 months ago

Closes #146. The goal here was to create a simple system that could be reused to handle keyboard shortcuts. Thus as Stract grows and potentially implements more features/views, new shortcuts can be easily added. Most of the TSDoc comments speak for themselves but in summary:

[x] Created new class Keybind to handle all needed Keybindings for a route
- For simplicity, limited to a single "normal" key that can be combined with shift, alt, & ctrl
- Class obj is initialized using params for the keys for the shortcut, and their callback
[x] Added search binding callbacks in keybind.ts
- Done here to prevent bloat in /search/+page.svelte
- exported in a single var to prevent unwieldy imports in /search/+page.svelte
[x] Mapped search keyboard shortcuts/functionality according to DuckDuckGo's
[x] Added new preference option to allow the user to globally disable keyboard shortcuts

Let me know whatever feedback you've got, totally willing to refactor to your preferences. Cheers!

lamemakes commented 6 months ago

Drafting as my main branch was out of sync so this needs merge help but will still take feedback in the meantime @mikkeldenker!

lamemakes commented 6 months ago

This feedback is phenomenal! I really appreciate you taking the time to give in depth feedback. Svelte is completely new to me as I'm coming from an Angular/Vue/Django background so these pointers have been incredibly helpful. Especially cool all of the things you can do with bindings in regards to HTML Elements. I had no idea and wince a bit whenever I had to do DOM manipulation. I'll have some revisions up shortly!

lamemakes commented 6 months ago

Changes will be up tomorrow - didn't catch the keybindings in Searchbar so those will also be included

lamemakes commented 6 months ago

Was hoping this method of implementation didn't clutter things but the aim was re-usability as Stract grows. Implementing it in the search bar felt like some overhead was definitely eliminated, along with code not being duplicated which was nice. Once feedback is given on those two comments I'll move the PR out of draft. Cheers!

lamemakes commented 5 months ago

This is great, thanks a ton for the fixes! Are you sure the keybind class isn't adding bloat? I can totally do a quick refactor and toss it all in the +page.svelte like the search's bindings. Cheers!

mikkeldenker commented 5 months ago

I actually quite like to have it in a separate file to avoid routes/search/+page.svelte becoming too enormous. I guess we can move keybind.ts into routes/search to indicate this is the only place we currently use it though.