KeyInterceptor: Simplify setup

[danielchalmers]

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:

private string _elementId = "switch_" + Guid.NewGuid().ToString().Substring(0, 8);
private IKeyInterceptor? _keyInterceptor;

[Inject]
private IKeyInterceptorFactory KeyInterceptorFactory { get; set; } = null!;

protected override async Task OnAfterRenderAsync(bool firstRender)
{
    if (firstRender)
    {
        _keyInterceptor = KeyInterceptorFactory.Create();

        await _keyInterceptor.Connect(_elementId, new KeyInterceptorOptions
        {
            //EnableLogging = true,
            TargetClass = "mud-switch-base",
            Keys = {
                new KeyOptions { Key="ArrowUp", PreventDown = "key+none" }, // prevent scrolling page, instead increment
                new KeyOptions { Key="ArrowDown", PreventDown = "key+none" }, // prevent scrolling page, instead decrement
                new KeyOptions { Key=" ", PreventDown = "key+none", PreventUp = "key+none" },
            },
        });

        _keyInterceptor.KeyDown += HandleKeyDown;
    }
    await base.OnAfterRenderAsync(firstRender);
}

protected override void Dispose(bool disposing)
{
    base.Dispose(disposing);

    if (disposing)
    {
        if (_keyInterceptor != null)
        {
            _keyInterceptor.KeyDown -= HandleKeyDown;
            if (IsJSRuntimeAvailable)
            {
                _keyInterceptor.Dispose();
            }
        }
    }
}

Usage After (first draft):

private string _elementId = "switch_" + Guid.NewGuid().ToString().Substring(0, 8);
private IKeyInterceptor? _keyInterceptor;

[Inject]
private IKeyInterceptorFactory KeyInterceptorFactory { get; set; } = null!;

protected override async Task OnAfterRenderAsync(bool firstRender)
{
    if (firstRender)
    {
        _keyInterceptor = await KeyInterceptorFactory.Connect(
            "mud-switch-base",
            //enableLogging: true,
            new("ArrowUp", preventDown: "key+none"),
            new("ArrowDown", preventDown: "key+none"),
            new(" ", preventDown: "key+none", preventUp: "key+none")
        );

        _keyInterceptor.KeyDown += HandleKeyDown;
    }
    await base.OnAfterRenderAsync(firstRender);
}

protected override void Dispose(bool disposing)
{
    base.Dispose(disposing);

    if (disposing)
    {
        if (_keyInterceptor is not null)
        {
            _keyInterceptor.KeyDown -= HandleKeyDown;
            _keyInterceptor.Dispose(); // IsJSRuntimeAvailable is implied?
        }
    }
}

Just the difference in creation (first draft):

-_keyInterceptor = KeyInterceptorFactory.Create();
-await _keyInterceptor.Connect(_elementId, new KeyInterceptorOptions
-{
-    //EnableLogging = true,
-    TargetClass = "mud-switch-base",
-    Keys = {
-        new KeyOptions { Key="ArrowUp", PreventDown = "key+none" }, // prevent scrolling page, instead increment
-        new KeyOptions { Key="ArrowDown", PreventDown = "key+none" }, // prevent scrolling page, instead decrement
-        new KeyOptions { Key=" ", PreventDown = "key+none", PreventUp = "key+none" },
-    },
-});

+_keyInterceptor = await KeyInterceptorFactory.Connect(
+    "mud-switch-base",
+    //enableLogging: true,
+    new("ArrowUp", preventDown: "key+none"),
+    new("ArrowDown", preventDown: "key+none"),
+    new(" ", preventDown: "key+none", preventUp: "key+none")
+);

• Eliminates KeyInterceptorOptions which is used homogenously so shouldn't be a problem.
• Eliminates the elementID field so there is one less piece of setup to keep track of
• Less pieces overall
• Keys array is turned into params.

Could also

• turn _keyInterceptor.KeyDown += HandleKeyDown; into a callback parameter.
• create _elementId from KeyInterceptorFactory.Connect via a tuple

Have you seen this feature anywhere else?

No response

Describe alternatives you've considered

No response

Pull Request

• [X] I would like to do a Pull Request

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[github-actions[bot]]

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.

[danielchalmers]

@ScarletKuro @henon @mckaragoz Just an idea I had. Not finished but wonder what you think.

E.g. KeyDown and KeyUp could also be callbacks passed through KeyInterceptorFactory.Connect to allow for async

[ScarletKuro]

My idea was to completely rewrite it, make it a scoped service to throw the factory and use ObserverManager, and the subscribe would have a callback either an Action or Func(to support Task and non task callbacks) overload, just like BrowserViewportService does. The elemendId is the unique subscription identificator. I have even done it, but just being lazy with the tests.

[danielchalmers]

My idea was to completely rewrite it, make it a scoped service to throw the factory and use ObserverManager, and the subscribe would have a callback either an Action or Func(to support Task and non task callbacks) overload, just like BrowserViewportService does. The elemendId is the unique subscription identificator. I have even done it, but just being lazy with the tests.

@ScarletKuro Please let me know if I can help finish it so I can use it in more components

[ScarletKuro]

Crated a draft https://github.com/MudBlazor/MudBlazor/pull/9003 Idk if we want this in v7 or v8, but I just wanted to show how I see the API should be done Will later think if we can simplify the KeyInterceptorOptions as @danielchalmers proposed. But main aim was the async / void support.

[henon]

Idk if we want this in v7 or v8

If it isn't breaking we can do it anytime after v7 has stabilized.

[ScarletKuro]

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. It's better to keep old services for a while until you are completely sure the news ones are mature and has not bugs, just like with the PopoverSerivce and BrowserViewportService.

[danielchalmers]

FWIW I want to implement keyboard handling in several controls so I would be glad to use it in v7