Check if last pressed key is part of hotkey combination

[maxbergmark]

No description provided.

[friendlymatthew]

Just a note: fixes #119

[friendlymatthew]

Thanks @maxbergmark! I'll make sure to review this over the weekend.

[friendlymatthew]

Looks great so far! Current thoughts so far. Will continue reading later!

[maxbergmark]

@friendlymatthew All good suggestions, I fixed them immediately. I also noticed that #127 causes conflicts with my changes. I changed my code to also use key() instead of code(), and handled those conflicts.

[friendlymatthew]

Great! One minor comment, but otherwise this looks good to merge

[maxbergmark]

@friendlymatthew Good suggestion, I'll clean it up.

A quick note: I got a bit worried that I'd broken something: Due to #127, there might need to be some updates to the documentation. I noticed that all my hotkeys that were set up using alphanumeric keys weren't working. After some digging, it seems that the reason is that the event code and the event key are different. While the event code is e.g. "KeyA" or "Digit1", the event key is just "A" or "1". After updating all keyboard shortcuts in my repository it works just like before, but I would describe this as a breaking change unfortunately.

I hope I'm mistaken here and there's something I'm overlooking, but it is something that should be noted and tested thoroughly. I looked at the original issue #121 which included links to the documentation of code() and key(). It seems that the breaking change indeed comes from there.

[maxbergmark]

@friendlymatthew When I merged in the changes from #121, I noticed that this is the only place where event.key() is referenced. So if you've come up with a feature flag for switching between event.code() and event.key(), I'll add it here.

[friendlymatthew]

@maxbergmark thanks for flagging! Would you be interested in attacking #131? Work is taking up all my time, so I'm afraid I won't have time to get this done ASAP.

On another note, would you be interested in joining as a maintainer? You'd be requested for reviews and help steer this project. Let me know! :)

[maxbergmark]

@friendlymatthew I made an attempt at solving #131. I added a feature called event_key and just used it in the clean_key function. That should do the job, but I'd be happy to fix any issues that I might have overlooked (maybe the spacebar handling?). The name of the feature flag was just something I made up, I'll change it if you have a better suggestion.

I'm honored to be invited as a maintainer of this repo, but right now I can't find the time that it would require. I think you're doing great work, and I'll happily keep using the repo, and contribute in discussions and issues when I have some time left over.

[friendlymatthew]

@all-contributors please add @maxbergmark for code

[allcontributors]

@friendlymatthew

I've put up a pull request to add @maxbergmark! 🎉

[friendlymatthew]

Hey @maxbergmark, what do you say we rollback on these two commits, and make these their own PRs? That way we can get this merged in at the time 1afdfb5

[maxbergmark]

@friendlymatthew Sounds like a good plan, it's better practice to have separate PRs.

[friendlymatthew]

@friendlymatthew Sounds like a good plan, it's better practice to have separate PRs.

Great, let me know when you do that. I'll take a final look and then should be good to merge.

[maxbergmark]

@friendlymatthew I reverted the past two commits

[friendlymatthew]

LET'S GOOOOOOOOO! So awesome to see this over the line @maxbergmark!

Just a note, we'll need to immediately handle the .key() calling convention. #135

[friendlymatthew]

LET'S GOOOOOOOOO! So awesome to see this over the line @maxbergmark!

Just a note, we'll need to immediately handle the .key() calling convention. #135

Also, thank you for spending time on this!

[maxbergmark]

Thank you for approving this! If you merge it (I don't have write access) you could re-use the logic for using either the key or code:

#[cfg(not(feature = "ssr"))]
fn clean_key(event: &web_sys::KeyboardEvent) -> String {
    if cfg!(feature = "FEATURE_NAME") {
        match event.key().as_str() {
            " " => "spacebar".to_string(),
            key => key.to_lowercase(),
        }
    } else {
        event.code()
    }
}

[friendlymatthew]

Thank you for approving this! If you merge it (I don't have write access) you could re-use the logic for using either the key or code:

#[cfg(not(feature = "ssr"))]
fn clean_key(event: &web_sys::KeyboardEvent) -> String {
    if cfg!(feature = "FEATURE_NAME") {
        match event.key().as_str() {
            " " => "spacebar".to_string(),
            key => key.to_lowercase(),
        }
    } else {
        event.code()
    }
}

Hmm, I sent an invitation to have write access to this crate for both you and @rkimoakbioinformatics.

Seems like your invite got expired, I just resent it, can you check your email/Github notifications?