ransome1 / sleek

todo.txt manager for Linux, Windows and MacOS, free and open-source (FOSS)
https://github.com/ransome1/sleek/wiki
MIT License
1.34k stars 104 forks source link

Show key bindings on hover #438

Closed lquenti closed 1 year ago

lquenti commented 1 year ago

Did you check if a similar feature request has already been reported? Please do so first: https://github.com/ransome1/sleek/issues (hope I haven't missed it xoxo)

Is your feature request related to a problem? Please describe. I currently start to migrate from paper todos to this tool and love it so far. But I am sometimes still unsure whether a key binding exist as an alternative for a specific action.

Describe the solution you'd like That, on hovering a button, it shows the equivalent binding, i.e. showing n when hovering over adding a new item.

Describe alternatives you've considered None, think this the canonical way.

lquenti commented 1 year ago

Also, if you like the idea I'd love to try it out myself, though I don't have much experience with webdev myself.

bennettscience commented 1 year ago

I started toying with this and is this along the lines of what you're thinking?

https://user-images.githubusercontent.com/3878010/205990225-4fe7b5e3-2376-4393-baf7-ab95b0676e04.mp4

I added a shortcut= property for controls with keybindings and then targeted those elements to display on hover. Personally, I'm not a fan of things flying in around all over the place, but for users who want to shift to the keybindings over the mouse, this might be the least obtrusive method.

ransome1 commented 1 year ago

@bennettscience thanks for picking up this one :) I do think it is a pretty good idea to have the keybindings presented close to the function itself.

But I also share your worries about a direct hover being too intrusive. In my opinion it is, especially if there is no delay between entering the button with your mouse and showing the hover box. Also a simple N as the content is not self explanatory in my opinion.

In fact I think we should stick to the standard here, which is an actual tool tip, set by the title attribute. This doesn't add any unnecessary JavaScript or HTML elements, it keeps things simple.

We also should continue sticking to present the actual action behind the button and adding the shortcut in brackets, like Google does it in Drive. It takes roughly a second before it appears, so users can avoid seeing it, if they already know what's behind the function. image

In your example that would be New todo (N) . I also double checked some desktop apps. Like in Firefox this is standard behavior.

What do you think?

bennettscience commented 1 year ago

I could go either way. I think the animated tips look more like a native app and electron just provides the wrapper. The title tags can still be included and help with the accessibility piece. I'll look at some more samples and see if we can find a middle ground.

That said, I've never used Electron and while I was adding bindings to the menu UI, it would double-fire the event (keybindings worked fine without the accelerator key set. When I set that key, the event would trigger multiple times). I might take a look at event handling to see if that could be handled a little different to allow for the menus to include the keybindings as well.

On Tue, Dec 6, 2022, 2:17 PM ransome @.***> wrote:

Assigned #438 https://github.com/ransome1/sleek/issues/438 to @bennettscience https://github.com/bennettscience.

— Reply to this email directly, view it on GitHub https://github.com/ransome1/sleek/issues/438#event-7969993784, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5SY6SF7M6IQA2FKRAS4E3WL6GNVANCNFSM6AAAAAARYPSYMQ . You are receiving this because you were assigned.Message ID: @.***>

bennettscience commented 1 year ago

Here's another idea I had this morning. Leaving the title attribute allows for web-native accessibility. Adding aria-label and related adds some more screen reader accessibility on interactive pieces. But, to address this feature request directly, what do you think about adding a keyboard event listener on the Alt key to expose all currently-visible keyboard shortcuts inline with their element?

https://user-images.githubusercontent.com/3878010/206208845-31524856-71d3-4f3b-befa-7ce0343c9c08.mp4

This allows for:

ransome1 commented 1 year ago

Here's another idea I had this morning. Leaving the title attribute allows for web-native accessibility. Adding aria-label and related adds some more screen reader accessibility on interactive pieces. But, to address this feature request directly, what do you think about adding a keyboard event listener on the Alt key to expose all currently-visible keyboard shortcuts inline with their element? 2022-12-07_09-39-31.mp4

This allows for:

* In-context tooltips

* No need for a `hover` event on elements because the title attribute can also be expanded to include the tip

@bennettscience I like that, it's helpful and not intrusive :)

We can reuse this on another thing that came up some time ago. Somebody was asking for a feature which would help him or her to create several todos in a row without the modal being closed. We could achieve this with the alt button (holding alt and clicking on create). Without a feature like this, nobody would understand this functionality.

I also like the idea of using Electrons accelator function in the main.js. I remember we were using this in the beginning but for some reason abandoned it. Maybe this time we can make it work for all shortcuts.

I still believe we should add the keybinding to the title tag somehow. But that would need to happen dynamically, as it all comes out the translation files. And I don't think we should put the shortcuts into those files.

In general the keyboard cuts are an essential feature of sleek, which help users significantly. And I need to admit, I have not give those shortcuts the most attention recently. Really glad you're working on it.

github-actions[bot] commented 1 year ago

This is an automated response. We acknowledge your report, and we appreciate your engagement. However, as there has been no recent activity in this thread, it has been marked as stale. If you have any further feedback or if the matter is still relevant, please do not hesitate to respond. Otherwise, this thread will be automatically closed in 15 days from now.