-
-
Notifications
You must be signed in to change notification settings - Fork 231
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
Add Classes/Styles parameter to BitButtonGroup (#9514) #9546
base: develop
Are you sure you want to change the base?
Add Classes/Styles parameter to BitButtonGroup (#9514) #9546
Conversation
WalkthroughThe pull request introduces enhanced customization capabilities for 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (23)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor.samples.cs (1)
112-127
: Example8 C# Code: Good approach to handle item-specific click events.
The usage of delegate assignments (OnClick = _ => ...
) is clear. If the logic grows, consider factoring it into separate methods for readability.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor.samples.cs (8)
63-63
: Use a more descriptive variable name (optional).The name
example3CsharpCode
is acceptable, but for improved clarity and maintainability, consider making the variable name more descriptive of its content or purpose.
77-77
: Ensure naming consistency.
example4RazorCode
maintains the numbering approach, but check if there is a standard naming scheme across all examples (e.g., consistent numbering or descriptive suffixes).
123-123
: Avoid confusion by removing outdated references.Before adding a new example, verify that references to removed or renamed sections (e.g., older examples) do not linger in doc comments or related text.
153-153
: Add relevant documentation or inline comments.Documenting the significance of example 6 helps readers understand the toggling scenario more intuitively.
199-199
: Keep example numbering consistent.Validating example naming ensures coherence for readers toggling between examples in the documentation.
216-226
: Focus on clarity in event demonstration.The new event-based code effectively demonstrates item clicks. Consider adding a brief comment so users understand how the event is fired and where
clickedCustom
is defined.
390-395
: Enhance in-file documentation for class/style assignments.Users new to CSS-in-Blazor might benefit from added inline comments about applying custom classes or styles to child elements.
404-415
: LeverageStyles
&Classes
parameters carefully.This snippet powerfully demonstrates how to override styles on a per-button basis. Consider clarifying how these styles cascade or potentially conflict with global styles.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/BitButtonGroupDemo.razor.cs (1)
22-29
: Document howClasses
affects nested elements.The new
Classes
parameter is a great addition. For clarity, include a short explanation of how nested child elements would inherit or override these class styles.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor (7)
36-36
: Example numbering consistency.Using
example3
for icons is consistent, but verify each example’s content matches the associated C# snippet (e.g.,_BitButtonGroupCustomDemo.razor.samples.cs
).
113-113
: Maintain consistent spacing for readability.Increasing white space or using sub-headers can help visually separate the fill/outline/text examples from each other.
143-143
: Provide context for toggled usage.A short note explaining toggled behavior helps new users quickly understand the difference between toggled and non-toggled examples.
185-185
: Vertical stacking note.Consider clarifying how the vertical arrangement might interplay with parent container sizing/responsiveness.
258-383
: Robust color coverage.The comprehensive color demonstration is valuable. If additional colors or brand guidelines exist, ensure they are included for completeness.
385-385
: Rename “Style & Class” if needed.If official docs use “Styles & Classes” or “Styling & Classes,” keep naming consistent to avoid user confusion.
404-415
: Show advanced usage with dynamic style toggling.Consider adding an example that conditionally sets classes or styles at runtime (e.g., hover effects or conditional theming).
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor.samples.cs (5)
44-60
: Icon usage clarity.This example highlights icon usage with
BitButtonGroupOption
. A quick code comment referencingBitIconName
usage in other examples may help novices.
101-118
: ReversedIcon usage.Combine ReversedIcon with more descriptive text or icons if needed for better user experience, especially in right-to-left languages.
139-150
: Vertical arrangement tip.Mention layout or container constraints for vertical alignment, especially to avoid overflow if the parent has fixed height.
152-165
: Event usage best practice.Current example correctly increments/resets/decrements a counter. Encourage usage of minimal logic in UI events and delegation to a service or state manager if the logic grows.
Line range hint
375-431
: Advanced styling snippet.Including multiple custom classes (e.g.,
.custom-class
,.custom-item
,.custom-btn
) is excellent. Adding a brief mention of potential usage for dynamic theming would benefit advanced users.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor (1)
248-272
: Great use of event handling in the new “Events” example.You are demonstrating both
OnItemClick
and per-buttonOnClick
nicely. However, consider whether you might want to unify or clarify the usage instructions for each approach, as some consumers might find competing event paradigms confusing. A short inline comment or summary within the example could help explain when to use each event.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroup.razor
(2 hunks)src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroup.razor.cs
(6 hunks)src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroupClassStyles.cs
(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/BitButtonGroupDemo.razor.cs
(3 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/BitButtonGroupDemo.razor.scss
(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor
(8 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor.samples.cs
(12 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor
(7 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor.samples.cs
(4 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor
(8 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor.samples.cs
(4 hunks)
🔇 Additional comments (76)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor.samples.cs (13)
40-47
: No issues found with the new example3CsharpCode
.
This code snippet for iconItems
demonstrates standard usage of icon properties in button items.
48-48
: Example4 Razor: Looks good.
This snippet demonstrates icon-only vs. icon-with-text usage. Implementation appears consistent with the intended API.
56-56
: Example4 C# Code: No concerns.
These items follow the same structure as other examples, ensuring consistent usage of icon and text fields.
71-71
: Example5 Razor: ReversedIcon usage is clear and consistent.
Calls for reversing icons in button items are properly demonstrated in the markup.
75-75
: Example5 C# Code: Items are configured properly with ReversedIcon = true
.
No functional or naming issues found.
83-83
: Example6 Razor: The toggled feature is well presented.
Showcases how toggled items can be used. Suggest verifying the visual states in the final UI.
87-87
: Example6 C# Code: Toggle properties set on items correctly.
No issues identified with the toggled items' logic.
95-95
: Example7 Razor: Vertical layout usage.
Code segments illustrate the vertical orientation setting effectively.
99-99
: Example7 C# Code: Reuses basicItems
for vertical layout.
Implementation is straightforward and aligned with the previous code structure.
105-111
: Example8 Razor: Demonstrates item click events properly.
Using an inline lambda to capture the clicked item is clear and concise. Be mindful of any performance trade-offs with inline lambdas in complex scenarios.
148-217
: Example10 Razor: Demonstrates multiple colors and variants.
Showcases the broad color/variant matrix. Code is repetitive but effectively communicates the styling possibilities.
218-222
: Example10 C# Code: Straightforward definition of basicItems
.
No issues with the item structure or naming.
Line range hint 224-268
: Example11 Razor & C# Code: Custom styles and classes look well-implemented.
This provides a good demonstration of the newly introduced Classes
and Styles
parameters. Consider adding a tooltip or note clarifying the CSS layering rules if multiple classes/styles overlap.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroupClassStyles.cs (7)
1-2
: Namespace usage is clear and straightforward.
No issues found.
3-4
: Class declaration appears well-named.
The class name reflects its purpose to host CSS classes and styles.
5-9
: Property Root
serves a clear purpose.
The summary helps developers apply custom styling to the group’s container.
10-14
: Property Button
is intuitive.
It provides targeted control when styling internal buttons.
15-19
: Property Icon
naming.
The naming scheme is consistent and clarifies usage for icon styling.
20-24
: Property Text
documentation is concise.
No issues found.
25-29
: Property ToggleButton
finalizes coverage.
All expected styling points have been accounted for.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroup.razor (3)
25-25
: Enhanced style retrieval for items.
Switching to GetItemStyle(item)
is a good step toward modularizing style logic.
40-40
: Conditional icon style.
Using @Styles?.Icon
provides flexible icon-level styling without forcing a global approach.
46-46
: Conditional text style usage.
Applying @Styles?.Text
maintains consistency with the new styling architecture.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButtonGroup/BitButtonGroup.razor.cs (7)
24-27
: Introduction of Classes
parameter.
Allows granular class definitions and improves overall customization.
71-75
: Introduction of Styles
parameter.
Offers parallel flexibility for CSS styles.
116-117
: Registering Classes.Root
.
This direct approach ensures the root styling is integrated into the ClassBuilder.
159-163
: Registering Styles.Root
.
Analogous to Classes
, this extends consistent style handling at the root level.
231-235
: ToggleButton-specific classes.
Lines 231-235 correctly inject Classes.ToggleButton
into toggled items, improving clarity.
244-248
: Applying Classes.Button
to items.
This keeps consistent theming across regular (non-toggled) buttons.
252-274
: Unified style aggregator in GetItemStyle
.
Collecting user and framework-defined styles in a single list is both readable and extensible.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor (28)
36-36
: Icon example box naming.
Clear to new users that it focuses on icons in button groups.
57-57
: IconOnly example box for minimal designs.
Demonstrates discrete usage nicely.
95-95
: ReversedIcon usage.
Minimal, targeted demonstration is helpful for clarifying reversed icon behavior.
116-116
: Toggled example.
Showcasing toggled states is crucial for explaining advanced interactions.
137-137
: Vertical layout example.
A straightforward approach to show vertical stacking.
160-177
: Events example.
Illustrates both component-level and item-level event handling. Very educational for new users.
205-205
: Color example box logic.
Explicitly labeled "Color" to highlight the usage of color props.
207-207
: Clarifying text about specialized colors.
Nicely conveys the purpose of each color variant.
210-214
: Primary color usage.
Straightforward demonstration of multiple variants within the same color.
217-221
: Secondary color usage.
Consistent approach for a separate color set.
223-228
: Tertiary color usage.
Follows the established pattern for different color categories.
230-235
: Info color segment.
Keeping examples uniform fosters easier comparisons.
237-242
: Success color block.
Another helpful demonstration of supported colors.
244-249
: Warning color block.
Retains the same demonstration pattern, which is consistent.
251-256
: SevereWarning color block.
Already consistent with naming, no issues found.
258-263
: Error color block.
Clearly grouped for user clarity.
265-286
: Background colors section.
Visually contextualizing these examples with a colored container is a nice touch.
288-293
: PrimaryForeground section.
No issues; straightforward demonstration approach.
295-300
: SecondaryForeground section.
Again, consistent with the pattern for color usage.
302-307
: TertiaryForeground section.
All well aligned with the distinct color categories.
309-314
: PrimaryBorder demo.
Demonstrates border-specific usage well.
316-321
: SecondaryBorder usage.
In line with the other color demonstration blocks.
323-327
: TertiaryBorder usage.
Various border color demos unify the color system.
332-332
: Title changed to “Style & Class.”
Highlights the demo’s emphasis on custom styling.
334-334
: Intro text clarifies customization scope.
A good explanation of override possibilities.
337-339
: Component-level style & class usage.
Quick demonstration to show border radius and custom margin usage is helpful.
343-345
: Item-level style & class usage.
Showcasing per-item customizations addresses flexible UI scenarios.
347-355
: Combining Styles
and Classes
props.
A practical demonstration of new param usage.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor.samples.cs (8)
102-102
: Confirm the content aligns with the matching Razor code.
Whenever a C# code snippet is paired with its Razor counterpart, ensure they remain synchronized and reflect the same example scenario.
138-138
: Check for consistent toggling logic.
When demonstrating reversed icons, ensure the usage across examples is consistent and does not conflict with toggled examples.
180-180
: Validate toggled states in example 6.
If unified toggling logic is presented in this code, verify that user interactions reflect the intended states, especially if they differ from earlier examples.
203-203
: Ensure cross-reference correctness.
Double-check that references to example7CsharpCode
in documentation or other narratives accurately match this code block.
227-227
: No change detected – skipping comment.
228-256
: Show usage in real scenarios.
This C# snippet elegantly illustrates how to update clickCounter
. Consider adding a quick note explaining best practices for updating state within event handlers (e.g., StateHasChanged()
).
284-365
: Ensure color examples remain consistent with official guidelines.
The updated code provides thorough color samples for the BitButtonGroup
. Confirm that these color enumerations match design specifications and that they are all documented in the relevant style guide.
366-366
: No change detected – skipping comment.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/BitButtonGroupDemo.razor.cs (2)
94-101
: Validate possible style conflicts.
When injecting custom styles, confirm they don’t conflict with existing framework classes or user-defined overrides.
363-404
: Excellent use of BitButtonGroupClassStyles
.
These new parameters provide powerful flexibility. Ensure the documentation outlines each property’s usage clearly, so consumers understand how to apply or override them.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor (2)
63-63
: Align example ID and content.
Id="example4"
is an IconOnly example. Double-check references in your documentation or summary to ensure they match the final usage.
208-229
: Events example is well-structured.
This block concisely shows OnItemClick
and item-specific clicks. Suggest briefly noting that both events can coexist and the difference in usage.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor.samples.cs (4)
120-137
: Toggle example clarity.
Document how toggling differs from normal usage and whether it maintains state across re-renders or navigation.
167-168
: No critical changes required.
clickCounter
and clickedOption
are straightforward. Looks good.
170-199
: Size variants coverage.
All size variants (Small, Medium, Large) look well represented. Confirm that any official design guidelines specifying minimum or maximum sizes are adhered to if applicable.
Line range hint 201-374
: Extensive color example.
Similar to the other file, this covers a wide range of color enumerations for BitButtonGroupOption
. Keep this updated if new colors are introduced globally.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor (1)
572-582
: Excellent illustration of the new Styles
and Classes
dictionaries.
The Styles
and Classes
usage is straightforward. Just ensure you’re mindful of potential naming conflicts if more complex styling dictionaries are merged. If you foresee multiple style definitions colliding, consider prefixing or segmenting them to avoid collisions.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/BitButtonGroupDemo.razor.scss (1)
29-34
: Verify color contrast for accessibility.
While aliceblue
on crimson
may look visually appealing, confirm that it meets your accessibility (WCAG) requirements regarding contrast. If the target user base requires improved readability, consider adjusting either the foreground or background color.
private readonly string example4RazorCode = @" | ||
<BitButtonGroup Variant=""BitVariant.Fill"" TItem=""BitButtonGroupOption"" IconOnly> | ||
<BitButtonGroupOption Text=""Add"" IconName=""@BitIconName.Add"" /> | ||
<BitButtonGroupOption Text=""Edit"" IconName=""@BitIconName.Edit"" /> | ||
<BitButtonGroupOption Text=""Delete"" IconName=""@BitIconName.Delete"" /> | ||
</BitButtonGroup> | ||
|
||
<BitButtonGroup Variant=""BitVariant.Outline"" TItem=""BitButtonGroupOption"" IconOnly> | ||
<BitButtonGroupOption Text=""Add"" IconName=""@BitIconName.Add"" /> | ||
<BitButtonGroupOption Text=""Edit"" IconName=""@BitIconName.Edit"" /> | ||
<BitButtonGroupOption Text=""Delete"" IconName=""@BitIconName.Delete"" /> | ||
</BitButtonGroup> | ||
|
||
<BitButtonGroup Variant=""BitVariant.Text"" TItem=""BitButtonGroupOption"" IconOnly> | ||
<BitButtonGroupOption Text=""Add"" IconName=""@BitIconName.Add"" /> | ||
<BitButtonGroupOption Text=""Edit"" IconName=""@BitIconName.Edit"" /> | ||
<BitButtonGroupOption Text=""Delete"" IconName=""@BitIconName.Delete"" /> | ||
</BitButtonGroup> | ||
|
||
|
||
|
||
<BitButtonGroup Variant=""BitVariant.Fill"" TItem=""BitButtonGroupOption""> | ||
<BitButtonGroupOption Text=""Add"" IconName=""@BitIconName.Add"" /> | ||
<BitButtonGroupOption IconName=""@BitIconName.Edit"" /> | ||
<BitButtonGroupOption Text=""Delete"" IconName=""@BitIconName.Delete"" /> | ||
</BitButtonGroup> | ||
|
||
<BitButtonGroup Variant=""BitVariant.Outline"" TItem=""BitButtonGroupOption""> | ||
<BitButtonGroupOption Text=""Add"" IconName=""@BitIconName.Add"" /> | ||
<BitButtonGroupOption IconName=""@BitIconName.Edit"" /> | ||
<BitButtonGroupOption Text=""Delete"" IconName=""@BitIconName.Delete"" /> | ||
</BitButtonGroup> | ||
|
||
<BitButtonGroup Variant=""BitVariant.Text"" TItem=""BitButtonGroupOption""> | ||
<BitButtonGroupOption Text=""Add"" IconName=""@BitIconName.Add"" /> | ||
<BitButtonGroupOption IconName=""@BitIconName.Edit"" /> | ||
<BitButtonGroupOption Text=""Delete"" IconName=""@BitIconName.Delete"" /> | ||
</BitButtonGroup>"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IconOnly property best practice.
When toggling IconOnly
, confirm accessibility strategies for screen-reader users, such as providing aria-label
or title
attributes.
Possible fix:
-<BitButtonGroup Variant="BitVariant.Fill" TItem="BitButtonGroupOption" IconOnly>
+<BitButtonGroup Variant="BitVariant.Fill" TItem="BitButtonGroupOption" IconOnly aria-label="Actions">
Committable suggestion skipped: line range outside the PR's diff.
This closes #9514
Summary by CodeRabbit
New Features
BitButtonGroup
with new parameters for classes and styles.Bug Fixes
Documentation