feat(components): add Kbd component #251
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a new LumexKbd keyboard-key component (component, slots, styles, constants, enum), tests, documentation pages/examples/preview wrappers, and a NavigationStore entry for LumexKbd. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant LumexKbd
participant Styles as Styles.Kbd
participant Consts as KbdConstants
App->>LumexKbd: Render(Keys[], ChildContent)
LumexKbd->>Styles: Style(TwMerge)
Styles-->>LumexKbd: slot resolvers (Base/Abbr/Content)
loop keys
LumexKbd->>Consts: KeyboardKeys.TryGetValue(key)
Consts-->>LumexKbd: symbol or null
LumexKbd-->>App: emit <abbr title=key>symbol</abbr>
end
LumexKbd-->>App: emit <span>ChildContent</span> inside <kbd>
note right of App: Result: <kbd> with key badges and content
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #251 +/- ##
==========================================
- Coverage 96.95% 93.24% -3.72%
==========================================
Files 70 149 +79
Lines 1542 2617 +1075
Branches 150 392 +242
==========================================
+ Hits 1495 2440 +945
- Misses 28 92 +64
- Partials 19 85 +66 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (16)
src/LumexUI/Common/Enums/KeyboardKey.cs (1)
10-106: Enum looks solid; consider cross‑platform aliases.Add Alt/Meta/Windows (or aliases) to avoid platform ambiguity (Option vs Alt, Command vs Meta/Win). Ensure glyph/title mapping covers them.
Apply if desired:
public enum KeyboardKey { + /// <summary>Alt key (⌥ on macOS as Option).</summary> + Alt, + /// <summary>Meta key (⌘ on macOS as Command).</summary> + Meta, + /// <summary>Windows key (⊞).</summary> + Windows, Command, Shift, Control, Option, Enter, Delete, Escape, Tab, CapsLock, Up, Right, Down, Left, PageUp, PageDown, Home, End, Help, Space, }docs/LumexUI.Docs.Client/Common/Navigation/NavigationStore.cs (1)
37-37: Good addition to Components nav. Also add to Components API for parity.Keep docs navigation consistent by exposing LumexKbd in Components API too.
.Add( new( nameof( LumexDropdownMenu ) ) ) + .Add( new( nameof( LumexKbd ) ) ) //.Add( nameof( LumexInputBase<T> ) )docs/LumexUI.Docs.Client/Pages/Components/Kbd/Examples/Usage.razor (1)
1-1: Add import and a11y label.Ensure KeyboardKey is in scope and provide an aria-label for SR users.
-<LumexKbd Keys="@([KeyboardKey.Command])">K</LumexKbd> +@using LumexUI.Common +<LumexKbd Keys="@([KeyboardKey.Command])" aria-label="Command + K">K</LumexKbd>docs/LumexUI.Docs.Client/Pages/Components/Kbd/Examples/Keys.razor (1)
1-5: Import enum and add aria-labels for combos.Improves discoverability for screen readers.
-<div class="flex items-center gap-2"> - <LumexKbd Keys="@([KeyboardKey.Command])">K</LumexKbd> - <LumexKbd Keys="@([KeyboardKey.Command, KeyboardKey.Shift])">N</LumexKbd> - <LumexKbd Keys="@([KeyboardKey.Option, KeyboardKey.Command])">P</LumexKbd> -</div> +@using LumexUI.Common +<div class="flex items-center gap-2"> + <LumexKbd Keys="@([KeyboardKey.Command])" aria-label="Command + K">K</LumexKbd> + <LumexKbd Keys="@([KeyboardKey.Command, KeyboardKey.Shift])" aria-label="Command + Shift + N">N</LumexKbd> + <LumexKbd Keys="@([KeyboardKey.Option, KeyboardKey.Command])" aria-label="Option + Command + P">P</LumexKbd> +</div>src/LumexUI/Components/Kbd/LumexKbd.razor (2)
6-16: Add an accessible label on the root.Expose a combined keys label for SRs (e.g., “Command + K”). Either compute it or allow consumers to pass it via AdditionalAttributes.
If computing internally, consider:
-<LumexComponent As="@As" - Class="@GetStyles( nameof( S.Base ) )" - Style="@RootStyle" - @attributes="AdditionalAttributes"> +<LumexComponent As="@As" + Class="@GetStyles( nameof( S.Base ) )" + Style="@RootStyle" + aria-label="@KeysLabel" + @attributes="AdditionalAttributes"> @@ <span class="@GetStyles( nameof( S.Content ) )">@ChildContent</span> </LumexComponent>And in code-behind expose
protected string? KeysLabel => Keys is null ? null : string.Join(" + ", Keys.Select(k => k.ToString()));
10-13: Don’t rely on enum ToString for titles; use friendly names.“Control” vs “Ctrl”, “Option” vs “Alt”, etc. Suggest a mapping function for titles.
- <abbr class="@GetStyles( nameof( S.Abbr ) )" title="@key">@GetKeySymbol(key)</abbr> + <abbr class="@GetStyles( nameof( S.Abbr ) )" title="@GetKeyTitle(key)">@GetKeySymbol(key)</abbr>Implement
GetKeyTitle(KeyboardKey key)in the code-behind to return localized, platform-friendly labels.docs/LumexUI.Docs.Client/Pages/Components/Kbd/Kbd.razor (1)
14-22: Minor copy tweak (optional).“Class … styles the wrapper of the kbd contents.” Consider “Base class for the wrapper element.” Not blocking.
src/LumexUI/Components/Kbd/KbdSlots.cs (1)
16-27: Seal the slots type to lock the contract.Prevent inheritance so the slot surface (Base/Abbr/Content) stays stable.
-[ExcludeFromCodeCoverage] -public class KbdSlots : SlotBase +[ExcludeFromCodeCoverage] +public sealed class KbdSlots : SlotBasesrc/LumexUI/Styles/Kbd.cs (2)
44-45: Prefer null over empty string for no-op slot.Prevents merging an empty class token.
- [nameof( KbdSlots.Content )] = "", + [nameof( KbdSlots.Content )] = null,
26-41: Optional: add select-none to avoid accidental text selection.Minor UX polish; common for KBD pills.
[nameof( KbdSlots.Base )] = new ElementClass() .Add( "px-1.5" ) .Add( "py-0.5" ) .Add( "inline-flex" ) + .Add( "select-none" ) .Add( "space-x-0.5" )tests/LumexUI.Tests/Components/Kbd/KbdTests.razor (4)
4-10: Missing TailwindMerge using may break TwMerge registration.Add TailwindMerge using to ensure TwMerge resolves in this file.
@using LumexUI.Styles +@using TailwindMerge
25-26: Remove unused local RenderFragment.It’s defined but not used.
- RenderFragment content = @<span data-testid="kbd-children" />;
42-57: Strengthen assertions for multiple keys.Also assert the number of elements equals key count.
var root = cut.FindByTestId("kbd-multikeys"); root.Should().NotBeNull(); @@ root.InnerHtml.Should().Contain("⌘"); root.InnerHtml.Should().Contain("⇧"); root.InnerHtml.Should().Contain("K"); + cut.FindAll("abbr").Count.Should().Be(2);
12-21: Consider asserting base slot class is applied.Quick check that Base slot styles resolve (e.g., bg-default-100).
src/LumexUI/Components/Kbd/LumexKbd.razor.cs (2)
34-34: Make _slots read-only to reflect intent.We don’t mutate entries post-build; expose as IReadOnlyDictionary.
-private Dictionary<string, ComponentSlot> _slots = []; +private IReadOnlyDictionary<string, ComponentSlot> _slots = new Dictionary<string, ComponentSlot>();
82-94: Throw a more precise exception for missing/unknown slots.NotImplementedException obscures the cause; prefer KeyNotFound/ArgumentOutOfRange.
- if( !_slots.TryGetValue( slot, out var styles ) ) - { - throw new NotImplementedException(); - } + if( !_slots.TryGetValue( slot, out var styles ) ) + throw new KeyNotFoundException( $"Unknown slot '{slot}'." ); @@ - _ => throw new NotImplementedException() + _ => throw new ArgumentOutOfRangeException( nameof( slot ), slot, "Unknown slot." )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/LumexUI.Docs.Client/Common/Navigation/NavigationStore.cs(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Kbd/Examples/Keys.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Kbd/Examples/Usage.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Kbd/Kbd.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Kbd/PreviewCodes/Keys.razor(1 hunks)docs/LumexUI.Docs.Client/Pages/Components/Kbd/PreviewCodes/Usage.razor(1 hunks)src/LumexUI/Common/Enums/KeyboardKey.cs(1 hunks)src/LumexUI/Components/Kbd/KbdSlots.cs(1 hunks)src/LumexUI/Components/Kbd/LumexKbd.razor(1 hunks)src/LumexUI/Components/Kbd/LumexKbd.razor.cs(1 hunks)src/LumexUI/Styles/Kbd.cs(1 hunks)tests/LumexUI.Tests/Components/Kbd/KbdTests.razor(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/LumexUI/Components/Kbd/KbdSlots.cs (1)
src/LumexUI/Components/Kbd/LumexKbd.razor.cs (1)
ExcludeFromCodeCoverage(79-94)
src/LumexUI/Styles/Kbd.cs (2)
src/LumexUI/Components/Kbd/LumexKbd.razor.cs (1)
ExcludeFromCodeCoverage(79-94)src/LumexUI/Utilities/Variants/SlotCollection.cs (1)
SlotCollection(9-34)
src/LumexUI/Components/Kbd/LumexKbd.razor.cs (4)
src/LumexUI/Components/Bases/LumexComponentBase.cs (1)
LumexComponentBase(18-58)src/LumexUI/Utilities/Variants/TwVariants.cs (1)
ComponentSlot(112-115)src/LumexUI/Styles/Kbd.cs (1)
ExcludeFromCodeCoverage(13-48)src/LumexUI/Components/Kbd/KbdSlots.cs (1)
ExcludeFromCodeCoverage(15-27)
🔇 Additional comments (5)
docs/LumexUI.Docs.Client/Pages/Components/Kbd/PreviewCodes/Usage.razor (1)
1-5: LGTM.Matches existing preview patterns (rendermode + nested example).
docs/LumexUI.Docs.Client/Pages/Components/Kbd/PreviewCodes/Keys.razor (1)
1-5: LGTM.Consistent with other preview code pages.
src/LumexUI/Components/Kbd/KbdSlots.cs (1)
11-27: LGTM — shape matches usage in LumexKbd and Styles.Kbd.src/LumexUI/Components/Kbd/LumexKbd.razor.cs (1)
19-33: A11y: confirm markup uses semantic KBD + accessible labels.Ensure each has a title or aria-label and symbols are aria-hidden with a text alternative (e.g., sr-only with key name).
src/LumexUI/Styles/Kbd.cs (1)
18-19: Verify DI lifetime of TwMerge is Singleton. ServiceCollectionExtensions.AddTwMerge calls services.AddTailwindMerge(...) (external extension). (github.com) Tests register TwMerge as a singleton (e.g. tests/LumexUI.Tests/Components/Accordion/AccordionTests.cs). Confirm AddTailwindMerge actually registers TwMerge as Singleton; if it does not, register Services.AddSingleton() or adjust/remove any style-caching to avoid surprising consumers.
| private static ComponentVariant? _variant; | ||
|
|
||
| public static ComponentVariant Style( TwMerge twMerge ) | ||
| { | ||
| var twVariants = new TwVariants( twMerge ); | ||
|
|
||
| return _variant ??= twVariants.Create( new VariantConfig() | ||
| { | ||
| Slots = new SlotCollection | ||
| { | ||
| [nameof( KbdSlots.Base )] = new ElementClass() | ||
| .Add( "px-1.5" ) | ||
| .Add( "py-0.5" ) | ||
| .Add( "inline-flex" ) | ||
| .Add( "space-x-0.5" ) | ||
| .Add( "rtl:space-x-reverse" ) | ||
| .Add( "items-center" ) | ||
| .Add( "font-sans" ) | ||
| .Add( "font-normal" ) | ||
| .Add( "text-center" ) | ||
| .Add( "text-small" ) | ||
| .Add( "shadow-small" ) | ||
| .Add( "bg-default-100" ) | ||
| .Add( "text-foreground-600" ) | ||
| .Add( "rounded-small" ), | ||
|
|
||
| [nameof( KbdSlots.Abbr )] = "no-underline", | ||
|
|
||
| [nameof( KbdSlots.Content )] = "", | ||
| } | ||
| } ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Cache is bound to first TwMerge instance — make it per‑instance.
Current static cache captures the first TwMerge; different DI lifetimes/configs won’t be respected thereafter.
-using System.Diagnostics.CodeAnalysis;
+using System.Diagnostics.CodeAnalysis;
+using System.Runtime.CompilerServices;
@@
-internal static class Kbd
+internal static class Kbd
{
- private static ComponentVariant? _variant;
+ private static readonly ConditionalWeakTable<TwMerge, ComponentVariant> _cache = new();
@@
- public static ComponentVariant Style( TwMerge twMerge )
+ public static ComponentVariant Style( TwMerge twMerge )
{
- var twVariants = new TwVariants( twMerge );
-
- return _variant ??= twVariants.Create( new VariantConfig()
- {
- Slots = new SlotCollection
- {
- [nameof( KbdSlots.Base )] = new ElementClass()
+ if( !_cache.TryGetValue( twMerge, out var variant ) )
+ {
+ var twVariants = new TwVariants( twMerge );
+ variant = twVariants.Create( new VariantConfig()
+ {
+ Slots = new SlotCollection
+ {
+ [nameof( KbdSlots.Base )] = new ElementClass()
.Add( "px-1.5" )
.Add( "py-0.5" )
.Add( "inline-flex" )
.Add( "space-x-0.5" )
.Add( "rtl:space-x-reverse" )
.Add( "items-center" )
.Add( "font-sans" )
.Add( "font-normal" )
.Add( "text-center" )
.Add( "text-small" )
.Add( "shadow-small" )
.Add( "bg-default-100" )
.Add( "text-foreground-600" )
- .Add( "rounded-small" ),
+ .Add( "rounded-small" ),
- [nameof( KbdSlots.Abbr )] = "no-underline",
+ [nameof( KbdSlots.Abbr )] = "no-underline",
- [nameof( KbdSlots.Content )] = "",
- }
- } );
+ [nameof( KbdSlots.Content )] = null,
+ }
+ } );
+ _cache.Add( twMerge, variant );
+ }
+ return variant;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static ComponentVariant? _variant; | |
| public static ComponentVariant Style( TwMerge twMerge ) | |
| { | |
| var twVariants = new TwVariants( twMerge ); | |
| return _variant ??= twVariants.Create( new VariantConfig() | |
| { | |
| Slots = new SlotCollection | |
| { | |
| [nameof( KbdSlots.Base )] = new ElementClass() | |
| .Add( "px-1.5" ) | |
| .Add( "py-0.5" ) | |
| .Add( "inline-flex" ) | |
| .Add( "space-x-0.5" ) | |
| .Add( "rtl:space-x-reverse" ) | |
| .Add( "items-center" ) | |
| .Add( "font-sans" ) | |
| .Add( "font-normal" ) | |
| .Add( "text-center" ) | |
| .Add( "text-small" ) | |
| .Add( "shadow-small" ) | |
| .Add( "bg-default-100" ) | |
| .Add( "text-foreground-600" ) | |
| .Add( "rounded-small" ), | |
| [nameof( KbdSlots.Abbr )] = "no-underline", | |
| [nameof( KbdSlots.Content )] = "", | |
| } | |
| } ); | |
| } | |
| using System.Runtime.CompilerServices; | |
| using System.Diagnostics.CodeAnalysis; | |
| internal static class Kbd | |
| { | |
| private static readonly ConditionalWeakTable<TwMerge, ComponentVariant> _cache = new(); | |
| public static ComponentVariant Style( TwMerge twMerge ) | |
| { | |
| if( !_cache.TryGetValue( twMerge, out var variant ) ) | |
| { | |
| var twVariants = new TwVariants( twMerge ); | |
| variant = twVariants.Create( new VariantConfig() | |
| { | |
| Slots = new SlotCollection | |
| { | |
| [nameof( KbdSlots.Base )] = new ElementClass() | |
| .Add( "px-1.5" ) | |
| .Add( "py-0.5" ) | |
| .Add( "inline-flex" ) | |
| .Add( "space-x-0.5" ) | |
| .Add( "rtl:space-x-reverse" ) | |
| .Add( "items-center" ) | |
| .Add( "font-sans" ) | |
| .Add( "font-normal" ) | |
| .Add( "text-center" ) | |
| .Add( "text-small" ) | |
| .Add( "shadow-small" ) | |
| .Add( "bg-default-100" ) | |
| .Add( "text-foreground-600" ) | |
| .Add( "rounded-small" ), | |
| [nameof( KbdSlots.Abbr )] = "no-underline", | |
| [nameof( KbdSlots.Content )] = null, | |
| } | |
| } ); | |
| _cache.Add( twMerge, variant ); | |
| } | |
| return variant; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/LumexUI/Styles/Kbd.cs around lines 16 to 47, the static field _variant
caches the ComponentVariant from the first TwMerge instance which prevents
different TwMerge configurations or DI lifetimes from being respected; change
the implementation to avoid a static/module-level cache — either create/return a
fresh variant per call (remove _variant) or maintain the cache per TwMerge
instance (e.g. a dictionary keyed by TwMerge identity or attach variant to
TwMerge) so each TwMerge configuration gets its own ComponentVariant, and ensure
thread-safety if using a shared dictionary.
desmondinho
left a comment
desmondinho
left a comment
There was a problem hiding this comment.
Fantastic job! I love it :)
I’ve pointed out just a few very small things, mostly related to code style.
Thank you so much for your contribution! ❤️
| /// The Space key (Spacebar). | ||
| /// </summary> | ||
| Space, | ||
| } |
There was a problem hiding this comment.
Let's also add Fn (fn), Win (Win) and Alt (⎇).
| namespace LumexUI; | ||
|
|
||
| /// <summary> | ||
| /// Represents the slot names for the <see cref="LumexKbd"/>, |
There was a problem hiding this comment.
Could you please adjust this summary, so it follows the current convention (see BadgeSlots for reference)?
| Class="@GetStyles( nameof( S.Base ) )" | ||
| Style="@RootStyle" | ||
| @attributes="AdditionalAttributes"> | ||
| @foreach (var key in Keys) |
There was a problem hiding this comment.
This file requires formatting.
| namespace LumexUI; | ||
|
|
||
| /// <summary> | ||
| /// <see cref="LumexKbd"/> is a component to display which key or combination of keys performs a given action. |
There was a problem hiding this comment.
Could you please adjust this summary, so it follows the current convention (see LumexBadge for reference)?
Something like "A component that represents a keyboard key or combination of keys used to perform a specific action."
|
|
||
| private static string? GetKeySymbol( KeyboardKey key ) | ||
| { | ||
| // It's better a dictionary? |
There was a problem hiding this comment.
Yes, let's move it into the separate KeyboardKeys static file in the same folder. You can then move it directly in the razor file -- no need for this method.
…s selection in a constants file
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/LumexUI/Components/Kbd/LumexKbd.razor.cs (1)
14-16: Summary text matches the convention—nice.This addresses prior feedback to align with LumexBadge-style summaries.
🧹 Nitpick comments (7)
src/LumexUI/Components/Kbd/KbdConstants.cs (3)
18-42: Make the map read-only to prevent accidental mutation.Expose an IReadOnlyDictionary to avoid downstream writes and concurrency surprises.
- public static readonly Dictionary<KeyboardKey, string> KeyboardKeys = new() + public static IReadOnlyDictionary<KeyboardKey, string> KeyboardKeys { get; } = new Dictionary<KeyboardKey, string> { [KeyboardKey.Command] = "⌘", [KeyboardKey.Shift] = "⇧", [KeyboardKey.Control] = "⌃", [KeyboardKey.Option] = "⌥", [KeyboardKey.Alt] = "⎇", [KeyboardKey.Win] = "⊞", [KeyboardKey.Fn] = "fn", [KeyboardKey.Enter] = "↵", - [KeyboardKey.Delete] = "⌫", + [KeyboardKey.Delete] = "⌫", [KeyboardKey.Escape] = "⎋", [KeyboardKey.Tab] = "⇥", [KeyboardKey.CapsLock] = "⇪", [KeyboardKey.Up] = "↑", [KeyboardKey.Right] = "→", [KeyboardKey.Down] = "↓", [KeyboardKey.Left] = "←", [KeyboardKey.PageUp] = "⇞", [KeyboardKey.PageDown] = "⇟", [KeyboardKey.Home] = "↖", [KeyboardKey.End] = "↘", [KeyboardKey.Help] = "?", [KeyboardKey.Space] = "␣", - }; + };If needed, add:
using System.Collections.Generic;
28-28: Confirm intended “Delete” semantics (Backspace vs Forward Delete).Current glyph “⌫” is typically Backspace (macOS labels this key “Delete”). If you intended Forward Delete, consider “⌦”. Please confirm design choice across platforms before we lock tests/docs.
- [KeyboardKey.Delete] = "⌫", + // Use if you intend Forward Delete: + [KeyboardKey.Delete] = "⌦",
20-27: Mac-centric control glyphs—verify cross‑platform UX.Symbols like ⌘, ⌃, ⌥ are Apple conventions; Windows users may expect “Ctrl”, “Alt”, “Win” text. Decide if this map is intentionally mac‑first, or if LumexKbd should offer a platform/style toggle.
src/LumexUI/Components/Kbd/LumexKbd.razor.cs (4)
17-18: Seal the component class.Prevents unintended inheritance and enables minor runtime optimizations.
-public partial class LumexKbd : LumexComponentBase, ISlotComponent<KbdSlots> +public sealed partial class LumexKbd : LumexComponentBase, ISlotComponent<KbdSlots>
34-34: Store slots as read-only to reflect usage.The field isn’t mutated outside assignment; use IReadOnlyDictionary for clearer intent.
-private Dictionary<string, ComponentSlot> _slots = []; +private IReadOnlyDictionary<string, ComponentSlot> _slots = new Dictionary<string, ComponentSlot>();
44-49: Initialize styles once.Slots are variant-defined and independent of parameters; initialize in OnInitialized to avoid recomputing each parameter set.
-/// <inheritdoc/> -protected override void OnParametersSet() -{ - var kbd = Styles.Kbd.Style( TwMerge ); - _slots = kbd(); -} +protected override void OnInitialized() +{ + var kbd = Styles.Kbd.Style( TwMerge ); + _slots = kbd(); +}
51-66: Use more specific exceptions.Key not found and unknown slot are not “not implemented” conditions.
if( !_slots.TryGetValue( slot, out var styles ) ) { - throw new NotImplementedException(); + throw new KeyNotFoundException( $"Slot '{slot}' not found for {nameof(LumexKbd)}." ); } ... - _ => throw new NotImplementedException() + _ => throw new ArgumentOutOfRangeException( nameof(slot), slot, "Unknown slot." )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/LumexUI/Common/Enums/KeyboardKey.cs(1 hunks)src/LumexUI/Components/Kbd/KbdConstants.cs(1 hunks)src/LumexUI/Components/Kbd/KbdSlots.cs(1 hunks)src/LumexUI/Components/Kbd/LumexKbd.razor(1 hunks)src/LumexUI/Components/Kbd/LumexKbd.razor.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/LumexUI/Components/Kbd/KbdSlots.cs
- src/LumexUI/Common/Enums/KeyboardKey.cs
- src/LumexUI/Components/Kbd/LumexKbd.razor
🧰 Additional context used
🧬 Code graph analysis (1)
src/LumexUI/Components/Kbd/LumexKbd.razor.cs (4)
src/LumexUI/Components/Bases/LumexComponentBase.cs (1)
LumexComponentBase(18-58)src/LumexUI/Utilities/Variants/TwVariants.cs (1)
ComponentSlot(112-115)src/LumexUI/Components/Kbd/KbdSlots.cs (1)
ExcludeFromCodeCoverage(14-26)src/LumexUI/Styles/Kbd.cs (1)
ExcludeFromCodeCoverage(13-48)
| /// <summary> | ||
| /// Gets or sets the collection of keys to be used as input for the component. | ||
| /// </summary> | ||
| [Parameter] public IEnumerable<KeyboardKey> Keys { get; set; } = []; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Guard against missing glyphs in the .razor markup.
If the markup indexes the dictionary directly (e.g., C.KeyboardKeys[key]), an unmapped enum value will throw. Prefer TryGetValue with a fallback (e.g., key.ToString()).
Suggested markup pattern:
@{ var ok = C.KeyboardKeys.TryGetValue(key, out var g); }
<abbr title="@key">@((ok ? g : key.ToString()))</abbr>
🏁 Script executed:
#!/bin/bash
# Verify no direct indexer use on KeyboardKeys that could throw at runtime.
rg -nP -C2 '(?:KbdConstants|KeyboardKeys)\s*\[' --glob '!**/bin/**' --glob '!**/obj/**'Length of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for 'KeyboardKeys' occurrences..."
rg -n --hidden -S -C2 '\bKeyboardKeys\b' --glob '!**/bin/**' --glob '!**/obj/**' || true
echo "Searching for 'KeyboardKeys[' indexing..."
rg -n --hidden -S -C2 '\bKeyboardKeys\s*\[' --glob '!**/bin/**' --glob '!**/obj/**' || true
echo "Searching for 'KbdConstants' occurrences..."
rg -n --hidden -S -C2 '\bKbdConstants\b' --glob '!**/bin/**' --glob '!**/obj/**' || true
echo "Searching for '.KeyboardKeys[' and 'KeyboardKeys.TryGetValue' patterns..."
rg -n --hidden -S -C2 '\.KeyboardKeys\s*\[' --glob '!**/bin/**' --glob '!**/obj/**' || true
rg -n --hidden -S -C2 'KeyboardKeys\.TryGetValue' --glob '!**/bin/**' --glob '!**/obj/**' || true
echo "Searching for 'C.KeyboardKeys' pattern..."
rg -n --hidden -S -C2 'C\.KeyboardKeys' --glob '!**/bin/**' --glob '!**/obj/**' || trueLength of output: 1139
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "----- top-level listing -----"
ls -la | sed -n '1,200p' || true
echo "----- git info -----"
if git rev-parse --is-inside-work-tree >/dev/null 2>&1; then
echo "Repo root: $(git rev-parse --show-toplevel)"
echo "HEAD: $(git rev-parse --short HEAD)"
echo "Tracked files: $(git ls-files | wc -l)"
else
echo "Not inside a git work tree"
fi
echo "----- git grep (fast, if repo present) -----"
git grep -n --full-name -E 'KeyboardKeys|KbdConstants|C\.KeyboardKeys' || true
git grep -n --full-name -E 'KeyboardKeys\s*\[' || true
git grep -n --full-name 'KeyboardKeys.TryGetValue' || true
echo "----- ripgrep (no VCS ignore) -----"
rg --version || true
rg -n --no-ignore-vcs -S -C2 -F 'KeyboardKeys' --hidden --glob '!**/bin/**' --glob '!**/obj/**' || true
rg -n --no-ignore-vcs -S -C2 -E 'KeyboardKeys\s*\[' --hidden --glob '!**/bin/**' --glob '!**/obj/**' || true
rg -n --no-ignore-vcs -S -C2 'KeyboardKeys\.TryGetValue' --hidden --glob '!**/bin/**' --glob '!**/obj/**' || true
rg -n --no-ignore-vcs -S -C2 'C\.KeyboardKeys' --hidden --glob '!**/bin/**' --glob '!**/obj/**' || true
echo "----- Search .razor and .razor.cs files specifically -----"
find . -type f \( -iname '*.razor' -o -iname '*.razor.cs' \) -print0 | xargs -0 -r rg -n --no-ignore-vcs -S -C2 'KeyboardKeys' || true
echo "----- Find files by name patterns -----"
find . -type f \( -iname '*kbd*' -o -iname '*lumex*' -o -iname '*keyboard*' \) -maxdepth 6 -print || true
TARGET='src/LumexUI/Components/Kbd/LumexKbd.razor.cs'
if [ -f "$TARGET" ]; then
echo "----- Showing $TARGET (first 200 lines) -----"
sed -n '1,200p' "$TARGET"
else
echo "$TARGET not found"
fiLength of output: 10607
Guard against missing glyphs — stop indexing C.KeyboardKeys directly
Direct dictionary indexing at src/LumexUI/Components/Kbd/LumexKbd.razor:18 will throw for unmapped KeyboardKey values; use TryGetValue with a fallback (e.g., key.ToString()).
@{ var ok = C.KeyboardKeys.TryGetValue(key, out var g); }
@((ok ? g : key.ToString()))
🤖 Prompt for AI Agents
In src/LumexUI/Components/Kbd/LumexKbd.razor.cs around lines 29 to 33, the Keys
property is initialized with an invalid literal ([]) and elsewhere the code
indexes C.KeyboardKeys directly which will throw for unmapped KeyboardKey
values; change the default to Array.Empty<KeyboardKey>() or
Enumerable.Empty<KeyboardKey>() and replace direct dictionary indexing with
C.KeyboardKeys.TryGetValue(key, out var g) and use g when true or key.ToString()
as a fallback when false so missing glyphs don’t cause exceptions.
|
Huge thanks for your input! It's priceless. I will merge this PR later today once I get to my PC. |
2 participants
Description
What's been done?
Kbdcomponent, adapted from HeroUI, into the LumexUI component library.Kbdcomponent.Checklist
Additional Notes
No additional notes.
Summary by CodeRabbit
New Features
Documentation
Styles
Tests