Raku / doc-website

Tooling to build/run the documentation website
Artistic License 2.0
7 stars 10 forks source link

Exploring various options for keyboard shortcuts #337

Closed opoku closed 7 months ago

opoku commented 8 months ago

A few experiments in this one PR as part of Issue #308. I'm interested in hearing any feedback about which option works better .

  1. Changes the search binding from Alt+F to just F (more specifically lowercase F). I added a condition in the event listener to ensure the event target was from the document body. This prevents the event listener from swallowing the single characters that it matches while typing in the input field.
  2. I added a binding in the search plugin to unfocus (or blur) the input field and close the search results when the Escape key is pressed.
  3. Checked the event.keyCode against the unicode value for the bound character to fix Alt (aka Option) modifier to work on Mac. I'd rather move away from this approach because the keyCode property is deprecated and may have unexpected results on different keyboard layouts.
finanalyst commented 8 months ago

@opoku I think nearly all the changes are good. I do not understand, though, the rationale for

e.keyCode === pageOptionsState[ attr ].letter.toUpperCase().charCodeAt(0) )

Is this a Mac-specific test? Does it mean that if a person with a non-Querty keyboard (eg Cyrillic) presses a key that would be associated with 'f' on an English keyboard ( 'A' on a Cyrillic keyboard) would get the search focus?

I'm going to add these to the new-raku site for testing.

opoku commented 8 months ago

@finanalyst thats exactly what I meant by by

I'd rather move away from this approach because the keyCode property is deprecated and may have unexpected results on different keyboard layouts.

The keyCode attribute is very qwerty-centric but it is what would enable the keyboard modifiers ( Alt/Meta ) to work on Mac.

finanalyst commented 8 months ago

@opuku Suppose we eliminate all reliance on keyCode, and have single letters for all shortcuts?

opoku commented 8 months ago

Yes. I'd recommend this. This also doesn't preclude possibly customizing bindings later but would allow a reasonable set of widely usable defaults.

finanalyst commented 8 months ago

@opoku I have tested this locally and it works for me. I'm building new-raku, and it will be working there in about 10min from now.

As for a broader set of short-cuts, I agree with you. Lets get the F working first, and have some community feed-back, then we can extend to other keys if people like F.

finanalyst commented 8 months ago

Working on new-raku.

Make sure to delete page options in local storage, then refresh page. Otherwise, browser will be waiting for old configuration

coke commented 8 months ago

pressing f on new-raku does nothing here. option-f, however, jumps to the search input (and once there, option-f generates a ƒ)

finanalyst commented 8 months ago

Make sure you purge the local shortcut storage. The saved keys won't trigger the new functionality

On Fri, 12 Jan 2024, 20:56 Will Coleda, @.***> wrote:

pressing f on new-raku does nothing here. option-f, however, jumps to the search input (and once there, option-f generates a ƒ)

— Reply to this email directly, view it on GitHub https://github.com/Raku/doc-website/pull/337#issuecomment-1889939235, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACYZHFWY4MGKKNC4FXGANTYOGPQXAVCNFSM6AAAAABBSNVUP6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBZHEZTSMRTGU . You are receiving this because you were mentioned.Message ID: @.***>

coke commented 8 months ago

works on mac.

coke commented 8 months ago

Note that the help still says alt-f

opoku commented 8 months ago

I'll put in some more cleanup work here if we're settled on f for now. Like the placeholder and on the settings dialog and other minor things I had noticed. Do you see anything else that the PR needs before approval? @finanalyst @coke

opoku commented 8 months ago

Updated the placeholder and modal text. I held back on other styling changes. I might create a separate PR so as not to conflate the two items.

opoku commented 8 months ago

I think this needs a way to migrate the persisted pageOptions so that the new shortcut is active on page load.

finanalyst commented 8 months ago

@opoku I read this, but I need some time to react, I just have a load of things to do first. Thanks for the work.

Since you have looked at search, which uses autocomplete, would you have a look at the ToC? It also has an autocomplete but uses another library. I think it might be better to use one library. The plugin is filtered-toc