Improve BitButtonGroup (#9463) #9517
Conversation
WalkthroughThe changes introduce significant enhancements to the Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/BitButtonGroupDemo.razor.cs (1)
The default values referencing
BitButtonGroupItemare incorrect and need to be updatedThe review comment is correct. Both
BitButtonGroupItemandBitButtonGroupOptionhave identical property names, but the default values in the demo are incorrectly referencing onlyBitButtonGroupItem. This is problematic because:
- The name selectors should work with both types
BitButtonGroupOptionis actually a component (ComponentBase) whileBitButtonGroupItemis just a class- Properties in
BitButtonGroupOptionare marked with[Parameter]attribute, making them proper Blazor parametersThe default values should be updated to reference the correct type based on the generic parameter
TItem.🔗 Analysis chain
Line range hint
387-493: Verify name selector default values.The default values for the new name selectors reference
BitButtonGroupItem, but they should work for bothBitButtonGroupItemandBitButtonGroupOption.
Let me try to find the correct paths for these files first.
Now that we have the correct paths, let's examine the properties in both classes:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that both BitButtonGroupItem and BitButtonGroupOption have the same property names # Search for property declarations in both classes echo "Searching for property declarations in BitButtonGroupItem:" ast-grep --pattern 'public string? $PROP { get; set; }' src/BlazorUI/BitButtonGroupItem.cs echo "Searching for property declarations in BitButtonGroupOption:" ast-grep --pattern 'public string? $PROP { get; set; }' src/BlazorUI/BitButtonGroupOption.csLength of output: 611
Script:
#!/bin/bash # Find the correct paths for BitButtonGroupItem and BitButtonGroupOption fd -e cs "BitButtonGroupItem|BitButtonGroupOption" # Once we find the files, let's check their content echo "Searching for all property declarations in both classes:" ast-grep --pattern 'class BitButtonGroupItem { $$$ public $_ $PROP { get; set; } $$$ }' ast-grep --pattern 'class BitButtonGroupOption { $$$ public $_ $PROP { get; set; } $$$ }'Length of output: 1055
Script:
#!/bin/bash echo "Properties in BitButtonGroupItem:" cat "src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroupItem.cs" echo -e "\nProperties in BitButtonGroupOption:" cat "src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroupOption.cs"Length of output: 5798
🧹 Nitpick comments (23)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor (1)
326-327: Suggest clarifying that the IconOnly attribute applies to all or individual items.
Currently, IconOnly at the group level might confuse new users regarding whether the text property is overridden or simply hidden. Consider clarifying in documentation.src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroup.scss (2)
73-75: Consider a more descriptive class nameWhile the functionality is good, the class name
bit-btg-rvi(presumably "row-reverse") could be more descriptive. Consider using something likebit-btg-reverseorbit-btg-rtlif this is intended for RTL support.-.bit-btg-rvi { +.bit-btg-reverse { flex-direction: row-reverse; }
Line range hint
111-111: Fix typo in transparent valueThere's a typo in the background color value for disabled state.
-.bit-btg-itm-clr-bg-dis: transprent; +.bit-btg-itm-clr-bg-dis: transparent;src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroup.razor.cs (4)
7-7: Consider renaming for readability.
_private TItem? toggleItem is a descriptive name, but renaming it to something like _selectedToggleItem might improve clarity.
202-223: Leverage consistent naming for CSS utility.
GetItemClass returns an aggregated list of classes. Confirm naming consistency across code and CSS.
252-275: Dynamic titles.
GetItemTitle conditionally returns a toggled or non-toggled title. Consider fallback if neither is provided.
590-613: OffTitle fallback.
Same as OnTitle. Ensure a fallback if OffTitle is not defined, especially if IconOnly is set.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/Operation.cs (1)
20-31: Provide default values for toggle properties.
It might help to define sensible defaults (like empty strings) for OnIcon, OffIcon, etc., preventing null references or fallback logic confusion in demos.src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroupItem.cs (3)
36-40: OffTitle and tooltips.
If OffTitle is omitted, consider a fallback to general Title or an empty string for clarity.
51-55: OnTitle toggled tooltip.
Same principle: if OnTitle is identical to OffTitle, ensure it’s intentional.
80-84: New Title property.
Consider aligning naming conventions with Bit naming patterns (Title => Tooltip, etc.) if more consistent.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor.cs (1)
41-46: Consider standardizing toggle state propertiesIn
toggledCustoms:
- First and third items use OnName/OffName
- Second item uses OnTitle/OffTitle
- Mixed usage of properties might lead to maintenance challenges
Consider standardizing to either Name or Title consistently across all toggle items:
-new() { OnTitle = "Resume", OffTitle = "Play", OnIcon = BitIconName.PlayResume, OffIcon = BitIconName.Play }, +new() { OnName = "Resume", OffName = "Play", OnIcon = BitIconName.PlayResume, OffIcon = BitIconName.Play },src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor.cs (2)
25-30: Maintain consistency with BitButtonGroupCustomDemo implementationThe
onlyIconItemsimplementation mirrors the same pattern asonlyIconCustomswith inconsistent naming. Consider aligning the implementation approach across both demo files.
39-44: Standardize property usage between Text and TitleSimilar to the custom demo,
toggledItemsshows mixed usage of Text/Title properties:
- First item uses OnText/OffText
- Second item uses OnTitle/OffTitle
Consider standardizing the property usage across all items for better maintainability.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroupOption.cs (1)
86-90: Consider documenting property precedenceThe Title property might conflict with OnTitle/OffTitle in toggle mode. Consider documenting the precedence rules in the XML comments.
/// <summary> /// Title to render in the option +/// Note: This property is ignored when OnTitle/OffTitle are set in toggle mode /// </summary> [Parameter] public string? Title { get; set; }src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor.samples.cs (2)
165-170: Consider adding validation for toggle state properties.While the toggle implementation is good, consider adding validation to ensure that either both or neither of the On/Off properties are set for each toggle pair (text, icon, title) to prevent inconsistent states.
private List<BitButtonGroupItem> toggledItems = [ new() { OnText = "Back (2X)", OffText = "Back", OnIconName = BitIconName.RewindTwoX, OffIconName = BitIconName.Rewind }, new() { OnTitle = "Resume", OffTitle = "Play", OnIconName = BitIconName.PlayResume, OffIconName = BitIconName.Play }, new() { OnText = "Forward (2X)", OffText = "Forward", OnIconName = BitIconName.FastForwardTwoX, OffIconName = BitIconName.FastForward, ReversedIcon = true } + // Add validation in BitButtonGroupItem class: + // public void ValidateToggleProperties() + // { + // if ((OnText != null) != (OffText != null)) throw new InvalidOperationException("Both OnText and OffText must be set together"); + // if ((OnIconName != null) != (OffIconName != null)) throw new InvalidOperationException("Both OnIconName and OffIconName must be set together"); + // if ((OnTitle != null) != (OffTitle != null)) throw new InvalidOperationException("Both OnTitle and OffTitle must be set together"); + // } ];
259-274: Consider adding error handling for state changes.The event handling implementation should include error handling for state changes to ensure robust behavior.
protected override void OnInitialized() { - eventsItems[0].OnClick = _ => { clickCounter++; StateHasChanged(); }; - eventsItems[1].OnClick = _ => { clickCounter = 0; StateHasChanged(); }; - eventsItems[2].OnClick = _ => { clickCounter--; StateHasChanged(); }; + eventsItems[0].OnClick = _ => { + try { + clickCounter++; + StateHasChanged(); + } + catch (Exception ex) { + // Handle or log the error + Console.WriteLine($"Error incrementing counter: {ex.Message}"); + } + }; + // Similar try-catch blocks for other event handlers }src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor.samples.cs (2)
139-179: Consider grouping related toggle properties together.While the properties are well-documented, consider reorganizing them to group related properties:
- On/Off Text pairs
- On/Off Icon pairs
- On/Off Title pairs
This would improve code readability and maintenance.
188-194: Consider moving ReversedIcon property.The ReversedIcon property could be moved closer to the IconName property since they are functionally related to icon handling.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor (3)
190-238: LGTM! Comprehensive example with clear demonstrations.The example effectively shows both ways to achieve icon-only buttons. Consider standardizing the spacing between variants for consistency.
270-310: Consider adding toggle state demonstration.While the example shows all toggle-related properties, it would be more helpful to demonstrate the actual state changes when toggling. Consider adding a simple counter or state indicator to show the toggle functionality in action.
Line range hint
405-432: Consider enhancing RTL support documentation.While the RTL implementation is correct, consider:
- Adding comments about text alignment considerations
- Documenting any RTL-specific styling requirements
- Mentioning browser compatibility notes for RTL support
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/BitButtonGroupDemo.razor.cs (1)
139-179: Consider improving toggle-related parameter descriptions.While the toggle-related parameters are well structured, their descriptions could be more detailed to better guide developers:
- Consider mentioning that these parameters are only applicable when
Toggledis true- Consider adding examples of common use cases
Example improved descriptions:
- Description = "The icon of the item when it is not checked in toggle mode.", + Description = "The icon of the item when it is not checked in toggle mode. Only applicable when the ButtonGroup's Toggled parameter is true.",Also applies to: 188-193, 214-220
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroup.razor(2 hunks)src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroup.razor.cs(7 hunks)src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroup.scss(19 hunks)src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroupItem.cs(2 hunks)src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroupNameSelectors.cs(2 hunks)src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroupOption.cs(2 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/BitButtonGroupDemo.razor.cs(9 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/Operation.cs(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor(5 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor.cs(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor.samples.cs(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor(5 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor.cs(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor.samples.cs(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor(5 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor.cs(0 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor.samples.cs(1 hunks)
💤 Files with no reviewable changes (1)
- src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor.cs
🔇 Additional comments (49)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor (8)
320-330: Impressive new IconOnly feature!
Allowing the entire button group to hide text is a great addition for space-saving UI designs.
351-359: Nicely demonstrated alternative approach for icon-only items.
Making individual icons textless is awesome for a partial icon-only setup.
382-413: ReversedIcon usage is clear.
These examples offer a clean demonstration of placing icons after the text. Great for bilingual or design-specific layouts.
415-446: Toggled functionality shows good feedback for user selections.
Bringing toggled states into a group is beneficial. The example is well-structured and easy to follow.
Line range hint 448-476: Vertical orientation is well demonstrated.
The usage is straightforward, with the “Vertical” attribute making the layout intuitive for stacked buttons.
Line range hint 477-521: Size variations are comprehensive.
Covering small, medium, and large ensures full coverage of typical design specs.
Line range hint 522-546: Style & Class examples are very thorough.
Allowing overrides at both the group level and item level is flexible. The example is self-explanatory.
Line range hint 547-572: Event handling examples are clear.
The usage of OnItemClick at the group level vs. OnClick at the item level is distinctly laid out, reducing confusion for new users.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor.samples.cs (7)
1-3: New partial class for sample code is well organized.
Providing sample snippets in a separate partial file promotes clarity and separation of concerns.
5-10: All examples are user-friendly and straightforward.
Well done including a minimal example that clarifies the default usage of BitButtonGroup and BitButtonGroupOption.
236-274: IconOnly sample code lines complement the new feature.
These snippet expansions align neatly with the usage in the Razor file.
295-311: Toggled sample code lines are consistent with the feature usage.
The toggling references to OnText/OffText and OnIconName/OffIconName are self-explanatory.
357-395: Style and class override examples are beneficial for theming.
Showing custom CSS, classes, and inline styling in examples is helpful for real-world scenarios.
397-411: Event handling snippet is comprehensive.
Demonstrating both group-level OnItemClick and item-level OnClick fosters clarity for different approaches to event handling.
415-432: RTL snippet showcases localized UI needs.
Including language-specific text with right-to-left alignment showcases a complete example for global usage.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroup.scss (3)
37-37: LGTM: Improved content alignment
The addition of justify-content: center ensures consistent alignment of button content, which is particularly beneficial for icon-only buttons mentioned in the PR summary.
139-155: LGTM: Well-structured checked state styling
The new .bit-btg-chk class is well-implemented with proper hover and active states, following the established pattern of other button variants. This addition properly supports the toggle functionality mentioned in the PR summary.
163-165: LGTM: Comprehensive dark theme support
The dark theme variables have been consistently implemented across all color variants, properly utilizing the existing design system color variables. This provides a solid foundation for dark theme support.
Also applies to: 174-176, 185-187, 196-198, 207-209, 218-220, 229-231, 240-242, 252-254, 263-265, 274-276, 285-287, 296-298, 307-309, 318-320, 329-331, 340-342
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroup.razor.cs (11)
30-33: Validate the icon-only scenario.
When IconOnly is set to true, ensure there's no unexpected layout shift or accessibility issue by providing relevant ARIA labels or tooltips.
66-70: Confirm toggled mode usage.
Toggled mode affects button state management. Verify all usage paths to ensure that toggling doesn't introduce conflicting interactions with external state.
156-156: Spread operator usage is helpful.
Using [.. Items] simplifies copying enumerables, but be mindful of performance if Items grows large.
189-199: Ensure toggling logic is aligned with user expectations.
When Toggled is true, the code toggles null if the same item is re-clicked, or sets the new item if it's different. Double-check if this behavior is desired in all scenarios.
225-250: Handle label fallback.
IconOnly is respected, but if no text is found, ensure there's a fallback for accessibility.
277-299: Valid icon retrieval.
Ensure that referencing OnIconName or OffIconName doesn't conflict with icon sets.
374-397: OffIcon must exist.
If no off icon is provided, defaulting to a fallback avoids broken UI.
494-517: Check toggled text retrieval.
GetOnText uses item properties or NameSelectors. Ensure no null reference if these properties are absent.
518-541: Likewise for OffText.
Similar to GetOnText, ensure a consistent fallback flow for OffText.
566-589: Review toggled title property.
Double-check if you require distinct alt/title attributes for accessibility if an item is toggled.
614-636: Confirm reversed icon rendering logic.
If ReversedIcon is true, confirm the UI’s left-to-right or right-to-left alignment.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/Operation.cs (1)
11-12: Check reversed icon usage.
Ensure the ReversedIcon property is clearly demonstrated in examples to avoid confusion about how icons are flipped.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroup.razor (2)
24-26: Title attribute usage.
Adding title enhances accessibility. Consider localizing or providing alternative text for dynamic items.
37-47: Conditional text rendering.
Using text.HasValue() ensures only non-empty text is rendered. Confirm if spacing or layout remains consistent when text is absent.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroupItem.cs (5)
26-30: OffIconName clarity.
OffIconName is for an unselected button state. Double-check that this property name is consistent with domain terminology.
31-35: OffText usage.
Similarly for OffText. Confirm all toggled logic references OffText properly if no text is assigned.
41-45: OnIconName toggled state.
Ensure that product designs or UI guidelines define a coherent set of toggled icons for a consistent user experience.
46-50: OnText toggled label.
If OnText is empty or identical to OffText, it might confuse end-users.
61-65: Positioning reversal.
ReversedIcon can flip visual layout. Ensure it’s thoroughly tested in different language directions (e.g., RTL).
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor.cs (2)
34-39: LGTM! Consistent implementation of reversed icons
The reversedIconCustoms list demonstrates a consistent pattern with all buttons having names, icons, and reversed positioning.
27-32: Consider consistent naming pattern for icon-only buttons
In onlyIconCustoms, the middle button lacks a name while others have it. This inconsistency might affect accessibility and user experience.
Let's verify if there's proper accessibility handling for icon-only buttons:
✅ Verification successful
Based on the code analysis, I can now generate the final response:
Icon-only buttons are properly handled for accessibility
The inconsistent naming pattern in onlyIconCustoms is not a concern because:
- The component handles icon-only buttons through the
IconOnlyproperty - Title attributes are properly set via
GetItemTitlefor all buttons - When text is absent, the title serves as the accessible name
- The component maintains proper accessibility by using
aria-disabledattributes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for aria-label or title attributes handling in BitButtonGroup component
rg -l "aria-label|title" --type cs --glob "**/BitButtonGroup/**"
Length of output: 313
Script:
#!/bin/bash
# Check how aria-label and title are handled in BitButtonGroup components
rg "aria-label|title" --type cs --glob "**/BitButtonGroup/**" -C 2
Length of output: 3015
Script:
#!/bin/bash
# Check the GetItemTitle implementation and Name property usage
rg "GetItemTitle|Name\s+{" --type cs --glob "**/BitButtonGroup/**" -C 3
Length of output: 179
Script:
#!/bin/bash
# Check the GetItemTitle implementation and Name property
rg "GetItemTitle|Name" --type cs --glob "**/BitButtonGroup/**" -C 3
Length of output: 50091
Script:
#!/bin/bash
# Check how text is handled for icon-only buttons
rg "GetItemText|text" --type cs --glob "**/BitButtonGroup/**" -C 2
Length of output: 11585
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroupOption.cs (1)
66-70: LGTM! Well-documented ReversedIcon property
The ReversedIcon property is well-documented and its purpose is clear.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroupNameSelectors.cs (3)
25-54: LGTM! Well-structured toggle state properties.
The new toggle-related properties are well-organized, consistently documented, and follow the established pattern using BitNameSelectorPair. The property pairs (On/Off) for IconName, Text, and Title provide comprehensive control over button states.
60-64: LGTM! Clear property for icon positioning.
The ReversedIcon property is appropriately typed as boolean and follows the same documentation pattern.
80-83: LGTM! Consistent Title property implementation.
The Title property follows the established pattern and complements the existing properties.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor.samples.cs (1)
134-146: LGTM! Clear demonstration of IconOnly functionality.
The example effectively demonstrates both ways to achieve icon-only buttons: using the IconOnly property and leaving the text field empty.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor (1)
222-241: LGTM! Clear demonstration of icon position customization.
The ReversedIcon example effectively demonstrates the flexibility in icon positioning.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor.samples.cs (2)
15-20: LGTM! Well-structured parameter definition.
The IconOnly parameter is clearly documented with appropriate type, default value, and description.
69-75: LGTM! Consistent parameter definition.
The Toggled parameter maintains consistency with other parameter definitions and provides clear documentation.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/BitButtonGroupDemo.razor.cs (2)
15-20: LGTM: New BitButtonGroup parameters are well documented.
The new IconOnly and Toggled parameters are properly documented with clear descriptions and appropriate default values.
Also applies to: 69-74
259-299: Ensure consistency between BitButtonGroupItem and BitButtonGroupOption.
The toggle-related parameters are correctly mirrored between BitButtonGroupItem and BitButtonGroupOption, maintaining API consistency.
Also applies to: 308-313, 334-340
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroup.razor.cs
Show resolved
Hide resolved
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroupOption.cs
Show resolved
Hide resolved
| <ComponentExampleBox Title="Toggled" RazorCode="@example7RazorCode" CsharpCode="@example7CsharpCode" Id="example7"> | ||
| <ExamplePreview> | ||
| <div>The Toggled in BitButtonGroup allows you to control the active or inactive state of each button, providing clear visual feedback to users about which buttons are selected or currently in use.</div> | ||
| <br /><br /> | ||
| <div class="example-content"> | ||
| <div>Fill (default)</div> | ||
| <BitButtonGroup Variant="BitVariant.Fill" Items="toggledItems" Toggled /> | ||
| </div> | ||
| <br /><br /> | ||
| <div class="example-content"> | ||
| <div>Outline</div> | ||
| <BitButtonGroup Variant="BitVariant.Outline" Items="toggledItems" Toggled /> | ||
| </div> | ||
| <br /><br /> | ||
| <div class="example-content"> | ||
| <div>Text</div> | ||
| <BitButtonGroup Variant="BitVariant.Text" Items="toggledItems" Toggled /> | ||
| </div> | ||
| </ExamplePreview> | ||
| </ComponentExampleBox> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding keyboard navigation support for toggle buttons.
The toggle functionality should include keyboard navigation support for better accessibility.
-<BitButtonGroup Variant="BitVariant.Fill" Items="toggledItems" Toggled />
+<BitButtonGroup Variant="BitVariant.Fill" Items="toggledItems" Toggled Role="group" TabIndex="0" />Committable suggestion skipped: line range outside the PR's diff.
...BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor
Show resolved
Hide resolved
This closes #9463
Summary by CodeRabbit
New Features
BitButtonGroupcomponent.Documentation
Style