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

fix(defineShortcuts): support minus - key #962

Merged
merged 1 commit into from
Nov 14, 2023
Merged

Conversation

smarroufin
Copy link
Contributor

@smarroufin smarroufin commented Nov 13, 2023

πŸ”— Linked issue

Fixes #954

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Here is a fix to the - shortcut using defineShortcuts, which has been broken when chained shortcuts were introduced.

Though, it fixes a bug but leverages some issues on more advanced usecases (complex keys).
The current separators for combined and chained shortcuts are _ and -. And their combination in the same key is quite complex.
We could image being able to handle _-_ (two hits on _), or e_- (combination of e and -). A non-exhaustive list of shortcuts that should work IMO: ['e_-', 'e__', '_-_', '---', '_--'].

But let's fix this current issue and potentially create a new one for advanced shortcuts, or wait to see if the demand actually exists.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

Copy link

vercel bot commented Nov 13, 2023

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Updated (UTC)
ui βœ… Ready (Inspect) Visit Preview Nov 13, 2023 2:57pm

Copy link
Contributor Author

Ok. Just as I pushed my PR, I realised it does not fix the related issue which is the combination of shift and -.

@appinteractive
Copy link

appinteractive commented Nov 13, 2023

@smarroufin none of the - key listeners worked, so it still should help with the other cases. 🀞

@benjamincanac benjamincanac changed the title fix(shortcuts): minus shortcut fix(defineShortcuts): support minus - key Nov 14, 2023
@benjamincanac benjamincanac merged commit de38afd into dev Nov 14, 2023
2 checks passed
@benjamincanac benjamincanac deleted the fix/minus-shortcut branch November 14, 2023 13:24
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.

defineShortcut - minus button does not work
3 participants