Improve Float feature of BitButton (#9358) #9359
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes in this pull request enhance the Changes
Assessment against linked issues
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: 2
🧹 Outside diff range and nitpick comments (10)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButton/BitButton.scss (1)
101-114: Refactor floating button classes for better maintainabilityThe floating button classes have duplicated z-index values and lack documentation.
Consider these improvements:
+ // Full-width button variant .bit-btn-flw { width: 100%; - z-index: $zindex-base; + @extend %bit-btn-float-base; } + // Base styles for floating buttons + %bit-btn-float-base { + z-index: $zindex-base; + } + // Absolute positioned floating action button .bit-btn-fab { position: absolute; - z-index: $zindex-base; + @extend %bit-btn-float-base; } + // Fixed positioned floating button .bit-btn-ffx { position: fixed; - z-index: $zindex-base; + @extend %bit-btn-float-base; }src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButton/BitButton.razor.cs (2)
81-85: Enhance FloatOffset parameter documentationWhile the parameter implementation is correct, the documentation could be more helpful by specifying:
- Expected format (e.g., CSS units like px, rem, etc.)
- Examples of valid values
- Default value if any
Example enhancement:
/// <summary> -/// Specifies the offset of the floating button. +/// Specifies the offset of the floating button using CSS units (e.g., "16px", "1rem"). +/// When not specified, default offset from the CSS class is used. /// </summary>
Line range hint
260-274: Consider extracting default position to a constantThe position handling is comprehensive, but the default position (
"bit-btn-brg") could be extracted to a constant for better maintainability.+private const string DEFAULT_FLOAT_POSITION_CLASS = "bit-btn-brg"; + ClassBuilder.Register(() => (Float || FloatAbsolute) ? FloatPosition switch { BitPosition.TopRight => "bit-btn-trg", BitPosition.TopCenter => "bit-btn-tcr", BitPosition.TopLeft => "bit-btn-tlf", BitPosition.CenterLeft => "bit-btn-clf", BitPosition.BottomLeft => "bit-btn-blf", BitPosition.BottomCenter => "bit-btn-bcr", BitPosition.BottomRight => "bit-btn-brg", BitPosition.CenterRight => "bit-btn-crg", BitPosition.Center => "bit-btn-ctr", - _ => "bit-btn-brg" + _ => DEFAULT_FLOAT_POSITION_CLASS } : string.Empty);src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/BitButtonDemo.razor.cs (3)
90-95: Consider enhancing FloatOffset documentation with examplesWhile the description is clear, it would be helpful to provide examples of valid offset values (e.g., "10px", "1rem", etc.) to guide developers.
Name = "FloatOffset", Type = "string?", DefaultValue = "null", - Description = "Specifies the offset of the floating button.", + Description = "Specifies the offset of the floating button. Examples: \"10px\", \"1rem\", \"2em\".",
102-102: Enhance FloatPosition parameter documentationThe current description could be more informative about how this parameter interacts with Float and FloatAbsolute features.
- Description = "Specifies the position of the floating button.", + Description = "Specifies the position of the floating button when Float or FloatAbsolute is enabled. Controls the anchor point relative to either the viewport or container.",
698-707: Improve LINQ chain readabilityThe LINQ expression for creating dropdown items could be more readable with proper indentation.
- private readonly List<BitDropdownItem<BitPosition>> floatPositionList = Enum.GetValues<BitPosition>() - .Cast<BitPosition>() - .Select(enumValue => new BitDropdownItem<BitPosition> - { - Value = enumValue, - Text = enumValue.ToString() - }) - .ToList(); + private readonly List<BitDropdownItem<BitPosition>> floatPositionList = Enum.GetValues<BitPosition>() + .Cast<BitPosition>() + .Select(enumValue => new BitDropdownItem<BitPosition> + { + Value = enumValue, + Text = enumValue.ToString() + }) + .ToList();src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/BitButtonDemo.razor.samples.cs (2)
169-172: Consider enhancing the FloatAbsolute demo containerWhile the implementation is correct, consider these improvements to make the demo more intuitive:
- Add visual indicators for the container boundaries
- Include examples of different container sizes
- Demonstrate interaction with scrollable content
<div style="position: relative; border: 1px gray solid"> + <div style="position: absolute; top: 0; left: 0; padding: 4px; background: rgba(0,0,0,0.1)">Container with relative positioning</div> <BitButton FloatAbsolute FloatPosition="@floatPosition" FloatOffset="@floatOffset" IconName="@BitIconName.Edit" IconOnly /> <div style="height: 300px; overflow: auto">
181-188: Consider enhancing the float position dropdown itemsWhile the implementation is functional, consider adding descriptions to help users understand each position's effect.
private readonly List<BitDropdownItem<BitPosition>> floatPositionList = Enum.GetValues<BitPosition>() .Cast<BitPosition>() .Select(enumValue => new BitDropdownItem<BitPosition> { Value = enumValue, - Text = enumValue.ToString() + Text = enumValue.ToString(), + Description = GetPositionDescription(enumValue) }) .ToList(); +private static string GetPositionDescription(BitPosition position) => position switch +{ + BitPosition.TopLeft => "Aligns to the top-left corner", + BitPosition.TopRight => "Aligns to the top-right corner", + BitPosition.BottomLeft => "Aligns to the bottom-left corner", + BitPosition.BottomRight => "Aligns to the bottom-right corner", + _ => string.Empty +};src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/BitButtonDemo.razor (2)
217-219: Consider adding input validation for FloatOffset.The UI controls look good, but the FloatOffset TextField might benefit from input validation to ensure valid numeric values.
-<BitTextField Label="FloatOffset" @bind-Value="floatOffset" Immediate /> +<BitTextField Label="FloatOffset" @bind-Value="floatOffset" Immediate + Type="BitTextFieldType.Number" + Min="0" />
223-229: Consider adding ARIA attributes for better accessibility.The FloatAbsolute example effectively demonstrates the feature with scrollable content. Consider enhancing accessibility for screen readers.
-<BitButton FloatAbsolute FloatPosition="floatPosition" FloatOffset="@floatOffset" IconName="@BitIconName.Edit" IconOnly /> +<BitButton FloatAbsolute FloatPosition="floatPosition" FloatOffset="@floatOffset" + IconName="@BitIconName.Edit" IconOnly + aria-label="Edit" role="button" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButton/BitButton.razor.cs(3 hunks)src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButton/BitButton.scss(3 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Bit.BlazorUI.Demo.Client.Core.csproj(2 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/BitButtonDemo.razor(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/BitButtonDemo.razor.cs(2 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/BitButtonDemo.razor.samples.cs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Bit.BlazorUI.Demo.Client.Core.csproj
🔇 Additional comments (11)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButton/BitButton.scss (2)
22-22: LGTM: Well-structured float offset variable
The new --bit-btn-float-offset variable follows naming conventions and uses the spacing function for consistency.
92-95: LGTM: No-text button variant
The .bit-btn-ntx class appropriately handles padding and icon margin reset for buttons without text.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButton/BitButton.razor.cs (3)
69-79: Documentation improvements align with PR objectives
The updated documentation clearly distinguishes between viewport-relative (Float) and container-relative (FloatAbsolute) positioning, making it easier for developers to choose the appropriate option.
253-258: LGTM: Clean class registration implementation
The class registration logic correctly handles both floating modes with appropriate CSS class names following the established naming pattern.
278-279: LGTM: Clean float offset style implementation
The float offset style registration correctly uses CSS custom properties and handles null values appropriately.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/BitButtonDemo.razor.cs (1)
81-81: Documentation improvements look good!
The updated descriptions clearly differentiate between viewport-relative and container-relative positioning for the Float features.
Also applies to: 88-88
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/BitButtonDemo.razor.samples.cs (3)
164-165: LGTM: Float controls implementation
The implementation provides intuitive controls for demonstrating the Float feature's positioning and offset capabilities.
167-167: LGTM: Float feature demonstration
The floating button implementation effectively showcases the Float feature with dynamic position and offset controls.
178-179: LGTM: Float parameters implementation
The state variables for float parameters are correctly implemented with appropriate types.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/BitButtonDemo.razor (2)
212-212: LGTM! Title change is consistent with component terminology.
The rename from "Floating" to "Float" better aligns with the component's property names.
221-221: LGTM! Good demonstration of FloatOffset feature.
The example effectively shows how to use both Float and FloatOffset properties together.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButton/BitButton.scss
Outdated
Show resolved
Hide resolved
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButton/BitButton.scss
Outdated
Show resolved
Hide resolved
closes #9358
Summary by CodeRabbit
Release Notes
New Features
FloatOffsetparameter for enhanced floating button positioning.Bug Fixes
Documentation
Chores