Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.93%. Comparing base (28bc599) to head (cd9c403).
Report is 1103 commits behind head on dev.

@@            Coverage Diff             @@
##              dev    #9896      +/-   ##
==========================================
+ Coverage   89.82%   90.93%   +1.10%     
==========================================
  Files         412      411       -1     
  Lines       11878    12863     +985     
  Branches     2364     2486     +122     
==========================================
+ Hits        10670    11697    +1027     
+ Misses        681      618      -63     
- Partials      527      548      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

1. Should KeyOptions, KeyInterceptorOptions be immutable if they're not intended to be reused/modified as they require an explicit subscribe again? Allowing Keys to be IReadOnlyList<KeyOptions> and not need .ToList().

2. I had success in building the solution after replacing the builders with new, and making key non-optional, turning this:

var keyInterceptorOptions = KeyInterceptorOptions.Create(
    targetClass: "mud-input-control",
    enableLogging: true,
    keys:
    [
        // prevent scrolling page, toggle open/close
        KeyOptions.Of(key: " ", preventDown: "key+none"),
        // prevent scrolling page, instead highlight previous item
        KeyOptions.Of(key: "ArrowUp", preventDown: "key+none"),
        // prevent scrolling page, instead highlight next item
        KeyOptions.Of(key: "ArrowDown", preventDown: "key+none"),
        KeyOptions.Of(key: "Home", preventDown: "key+none"),
        KeyOptions.Of(key: "End", preventDown: "key+none"),
        KeyOptions.Of(key: "Escape"),
        KeyOptions.Of(key: "Enter", preventDown: "key+none"),
        KeyOptions.Of(key: "NumpadEnter", preventDown: "key+none"),
        // select all items instead of all page text
        KeyOptions.Of(key: "a", preventDown: "key+ctrl"),
        // select all items instead of all page text
        KeyOptions.Of(key: "A", preventDown: "key+ctrl"),
        // for our users
        KeyOptions.Of(key: "/./", subscribeDown: true, subscribeUp: true)
    ]);

into:

var keyInterceptorOptions = new KeyInterceptorOptions(
    targetClass: "mud-input-control",
    enableLogging: true,
    // prevent scrolling page, toggle open/close
    new(" ", preventDown: "key+none"),
    // prevent scrolling page, instead highlight previous item
    new("ArrowUp", preventDown: "key+none"),
    // prevent scrolling page, instead highlight next item
    new("ArrowDown", preventDown: "key+none"),
    new("Home", preventDown: "key+none"),
    new("End", preventDown: "key+none"),
    new("Escape"),
    new("Enter", preventDown: "key+none"),
    new("NumpadEnter", preventDown: "key+none"),
    // select all items instead of all page text
    new("a", preventDown: "key+ctrl"),
    // select all items instead of all page text
    new("A", preventDown: "key+ctrl"),
    // for our users
    new("/./", subscribeDown: true, subscribeUp: true)
);

Is this feasible in reality?

• Should KeyOptions, KeyInterceptorOptions be immutable if they're not intended to be reused/modified as they require an explicit subscribe again? Allowing Keys to be IReadOnlyList<KeyOptions> and not need .ToList().

I made KeyInterceptorOptions immutable as it's a new class, but we can't do the same with KeyOptions as it's an existing primitive and we need to have backward compatibility for now, as many 3rd party is using key interceptor.

I had success in building the solution after replacing the builders with new, and making key non-optional, turning this:

I don't think I understand this one.
Do you suggest to remove params from keys parameter? It's just that in this case it has to be the first argument, because you can't have non optional parameter after the optional parameters.
I also don't see how this syntax:

var keyInterceptorOptions = new KeyInterceptorOptions(
    targetClass: "mud-input-control",
    enableLogging: true,
    new(" ", preventDown: "key+none"),
    new("ArrowUp", preventDown: "key+none"),
);

without the [] brackets is possible? I also think actually less readable without the key name?

Or you just want to add two constructors to KeyOptions one default and other same as the KeyOptions.Of arguments if you dislike it?

1. Should KeyOptions, KeyInterceptorOptions be immutable if they're not intended to be reused/modified as they require an explicit subscribe again? Allowing Keys to be IReadOnlyList<KeyOptions> and not need .ToList().

I remember a case where we needed to change the interceptor options at runtime. It was the PR #3393.

1. Should KeyOptions, KeyInterceptorOptions be immutable if they're not intended to be reused/modified as they require an explicit subscribe again? Allowing Keys to be IReadOnlyList<KeyOptions> and not need .ToList().

I remember a case where we needed to change the interceptor options at runtime. It was the PR #3393.

Currently it's not supported, not in the old and new implementation, I event explicitly mentioned that in remarks. If there is no urgent need for it I wouldn't implement it, it's possible to do it but keeping track of settings is not trivial code and you can see that in the browserviewportservice.

Hmm, strange. I thought the Close on ESC needed that update feature because it was added for it.

Edit: If you look at the #3393 description you see that it modifies the options because otherwise the ESC on an open Select would also close the dialog.

Hmm, strange. I thought the Close on ESC needed that update feature because it was added for it.

Edit: If you look at the #3393 description you see that it modifies the options because otherwise the ESC on an open Select would also close the dialog.

You are mixing the interceptor settings that were moved to separate KeyInterceptorOptions (logging, target class and initial key list) with KeyOptions. The mentioned PR is using UpdateKey which updates the KeyOptions on the fly. I thought you were talking about the interceptor settings and you want to change the target class on the fly (as example), which isn't supported.
The UpdateKey is on place:

I had success in building the solution after replacing the builders with new, and making key non-optional, turning this:

I don't think I understand this one. Do you suggest to remove params from keys parameter? It's just that in this case it has to be the first argument, because you can't have non optional parameter after the optional parameters. I also don't see how this syntax:

var keyInterceptorOptions = new KeyInterceptorOptions(
    targetClass: "mud-input-control",
    enableLogging: true,
    new(" ", preventDown: "key+none"),
    new("ArrowUp", preventDown: "key+none"),
);

without the [] brackets is possible? I also think actually less readable without the key name?

Or you just want to add two constructors to KeyOptions one default and other same as the KeyOptions.Of arguments if you dislike it?

Right, I see what you mean about the optional parameters. Maybe making key, keys, targetClass required since they would always be specified while keeping a single constructor in each class for simplicity:

public KeyOptions(
    string? key,
    bool subscribeDown = false,
    bool subscribeUp = false,
    string preventDown = "none",
    string preventUp = "none",
    string stopDown = "none",
    string stopUp = "none")
{
    Key = key;
    PreventDown = preventDown;
    PreventUp = preventUp;
    SubscribeDown = subscribeDown;
    SubscribeUp = subscribeUp;
    StopDown = stopDown;
    StopUp = stopUp;
}

/// <summary>
/// Configuration options for key interception.
/// </summary>
public class KeyInterceptorOptions
{
    /// <summary>
    /// The CSS class of the target HTML element that should be observed for keyboard events.
    /// </summary>
    /// <remarks>
    /// Note: This must be a single class name.
    /// </remarks>
    public string TargetClass { get; }

    /// <summary>
    /// Specifies whether resize events should be logged in the browser's console.
    /// </summary>
    public bool EnableLogging { get; }

    /// <summary>
    /// A list of key options that define the keys to intercept and their respective configurations.
    /// </summary>
    public IReadOnlyList<KeyOptions> Keys { get; }

    /// <summary>
    /// Creates a new instance of <see cref="KeyInterceptorOptions"/> with the specified parameters.
    /// </summary>
    /// <param name="targetClass">The CSS class of the target HTML element.</param>
    /// <param name="keys">An array of <see cref="KeyOptions"/> defining the keys to intercept and their configurations.</param>
    /// <param name="enableLogging">Specifies whether to enable logging of resize events in the browser's console.</param>
    /// <returns>A new instance of <see cref="KeyInterceptorOptions"/>.</returns>
    public KeyInterceptorOptions(
        string targetClass,
        KeyOptions[] keys,
        bool enableLogging = false)
    {
        TargetClass = targetClass;
        Keys = keys;
        EnableLogging = enableLogging;
    }
}

-protected override async Task OnAfterRenderAsync(bool firstRender)
-{
-    if (firstRender)
-    {
-        var keyInterceptorOptions = KeyInterceptorOptions.Create(
-            targetClass: "mud-switch-base",
-            keys:
-            [
-                // prevent scrolling page, instead increment
-                KeyOptions.Of(key: "ArrowUp", preventDown: "key+none"),
-                // prevent scrolling page, instead decrement
-                KeyOptions.Of(key: "ArrowDown", preventDown: "key+none"),
-                KeyOptions.Of(key: " ", preventDown: "key+none", preventUp: "key+none")
-            ]);
-        await KeyInterceptorService.SubscribeAsync(_elementId, keyInterceptorOptions, keyDown: HandleKeyDown);
-    }
-    await base.OnAfterRenderAsync(firstRender);
-}

+protected override async Task OnAfterRenderAsync(bool firstRender)
+{
+    if (firstRender)
+    {
+        var keyInterceptorOptions = new KeyInterceptorOptions(
+            "mud-switch-base",
+            [
+                // prevent scrolling page, instead increment
+                new("ArrowUp", preventDown: "key+none"),
+                // prevent scrolling page, instead decrement
+                new("ArrowDown", preventDown: "key+none"),
+                new(" ", preventDown: "key+none", preventUp: "key+none")
+            ]
+        );
+        await KeyInterceptorService.SubscribeAsync(_elementId, keyInterceptorOptions, keyDown: HandleKeyDown);
+    }
+    await base.OnAfterRenderAsync(firstRender);
+}

See previous edit for optional targetClass

@danielchalmers done, tho I ended up adding several constructors to KeyInterceptorOptions.
It can actually allow you this syntax:

var keyInterceptorOptions = new KeyInterceptorOptions(
    targetClass: "mud-input-control",
    new(" ", preventDown: "key+none"),
    new("ArrowUp", preventDown: "key+none"),
);

and

var keyInterceptorOptions = new KeyInterceptorOptions(
    targetClass: "mud-input-control",
    enableLogging: true,
    new(" ", preventDown: "key+none"),
    new("ArrowUp", preventDown: "key+none"),
);

also [] can be used as discussed above
But as I said, I'm not really fan of such short entry so I will leave my ported code as is, but others can use it.

If there are no other suggestion then this can be merged.

Added to v8.0.0 Migration Guide #9953

Ah, I bamboozled myself.
I thought KeyInterceptorOptions was my own class, and not the class that existed before.
So when we made KeyInterceptorOptions immutable as @danielchalmers suggested #9896 (comment), we did break the CodeBeam.MudBlazor.Extension.
My fault, I forgot about it, since code logic was made 6 months ago.