-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
KeyInterceptor: Simplify setup #8992
Comments
Thanks for wanting to do a PR, @danielchalmers ! We try to merge all non-breaking bugfixes and will deliberate the value of new features for the community. Please note there is no guarantee your pull request will be merged, so if you want to be sure before investing the work, feel free to contact the team first. |
@ScarletKuro @henon @mckaragoz Just an idea I had. Not finished but wonder what you think. E.g. |
My idea was to completely rewrite it, make it a scoped service to throw the factory and use |
@ScarletKuro Please let me know if I can help finish it so I can use it in more components |
Crated a draft #9003 |
If it isn't breaking we can do it anytime after v7 has stabilized. |
I planned to keep the old service aka just mark it as obsolete. |
FWIW I want to implement keyboard handling in several controls so I would be glad to use it in v7 |
Feature request type
Other
Component name
No response
Is your feature request related to a problem?
Thinking about ways to make it simpler to set up the KeyInterceptor. Will use it more with #8901 so am looking at small ways to make it easier to maintain and stay consistent between components with less chance for subtle bugs due to misconfiguration.
Describe the solution you'd like
Usage Before:
Usage After (first draft):
Just the difference in creation (first draft):
Could also
_keyInterceptor.KeyDown += HandleKeyDown;
into a callback parameter._elementId
fromKeyInterceptorFactory.Connect
via a tupleHave you seen this feature anywhere else?
No response
Describe alternatives you've considered
No response
Pull Request
Code of Conduct
The text was updated successfully, but these errors were encountered: