Skip to content
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

feat: override shortcuts #659

Merged
merged 9 commits into from
Jul 29, 2022
Merged

Conversation

manniL
Copy link
Contributor

@manniL manniL commented Jul 19, 2022

Resolves #555

This PR alters the shortcut setup so that users can change the default shortcuts via defineShortcutsSetup, e.g. removing some or disabling all of them.

packages/types/src/config.ts Outdated Show resolved Hide resolved
packages/types/src/config.ts Outdated Show resolved Hide resolved
@manniL manniL marked this pull request as draft July 20, 2022 09:59
@manniL manniL marked this pull request as ready for review July 20, 2022 11:59
@manniL
Copy link
Contributor Author

manniL commented Jul 20, 2022

Okay, I've changed the implementation and passed the default shortcuts to the setupShortcuts function accordingly so users can manipulate them.

Because injections only allow a single argument though, I had to change the signature of a the method which would lead to a breaking change.
To avoid that, having an option for another two argument would be nicer.

Also I was thinking of giving the default shortcuts string identifiers or similar so it'd be easier to disable only a few of them without relying on e.g. array positions

@manniL
Copy link
Contributor Author

manniL commented Jul 20, 2022

Nevermind, I altered the injection algorithm for unlimited arguments now 😋

@manniL manniL requested a review from antfu July 20, 2022 13:54
@antfu antfu merged commit b478c43 into slidevjs:main Jul 29, 2022
@manniL manniL deleted the feat/shortcut-override branch July 29, 2022 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Custom Shortcuts] nav.toggleOverview() does only toggle once
2 participants