-
-
Notifications
You must be signed in to change notification settings - Fork 235
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 IconUrl parameter to BitButton component (#9863) #9864
Add IconUrl parameter to BitButton component (#9863) #9864
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 update the BitButton component to support image icons using a new property, Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant B as BitButton
participant R as Renderer
U->>B: Render Button (with IconUrl value)
B->>R: Check if in loading state
alt Button is loading
R-->>B: Render spinner indication
else Button is not loading
B->>R: Evaluate rendering conditions
alt IconName provided
R-->>B: Render icon using IconName
else
B->>R: Check if IconUrl is provided
alt IconUrl exists
R-->>B: Render <img> with IconUrl
end
end
end
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
🔭 Outside diff range comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButton/BitButton.razor (1)
97-100
:⚠️ Potential issueAdd IconUrl support to anchor tag rendering.
The anchor tag rendering block only supports
IconName
but notIconUrl
. For consistency, add the sameIconUrl
support here.Apply this diff to add
IconUrl
support:@if (IconName.HasValue()) { <i style="@Styles?.Icon" class="bit-btn-icn bit-icon bit-icon--@IconName @Classes?.Icon" /> } + else if(IconUrl.HasValue()) + { + <img src="@IconUrl" style="@Styles?.Icon" class="bit-btn-icnu @Classes?.Icon" /> + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Web/wwwroot/images/bit-logo.svg
is excluded by!**/*.svg
📒 Files selected for processing (6)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButton/BitButton.razor
(1 hunks)src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButton/BitButton.razor.cs
(1 hunks)src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButton/BitButton.scss
(4 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
(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/BitButtonDemo.razor.samples.cs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (5)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButton/BitButton.razor (1)
29-32
: LGTM! Clean implementation of the image icon support.The implementation correctly handles the
IconUrl
parameter, using the existing style and class infrastructure while maintaining mutual exclusivity withIconName
.src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButton/BitButton.razor.cs (1)
118-121
: LGTM! Well-documented parameter addition.The
IconUrl
parameter is correctly implemented with proper nullability and clear XML documentation.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/BitButtonDemo.razor.samples.cs (1)
53-59
: Verify the availability of the demo image.The examples use "/images/bit-logo.svg" which might not be available in the project.
Run the following script to check if the image exists:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if the demo image exists in the public/static directories # Search for the image file fd 'bit-logo.svg' --type fLength of output: 452
Demo image verified
The image file exists in the expected location (src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/wwwroot/images/bit-logo.svg
) and in related paths, confirming that the/images/bit-logo.svg
reference is valid.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/BitButtonDemo.razor (1)
91-102
: LGTM! Verify the image URL.The examples effectively demonstrate the new
IconUrl
parameter with different button variants and positions. The implementation follows the existing demo patterns and provides comprehensive coverage of the feature.Run the following script to verify the image URL:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify that the image file exists in the public directory. # Test: Check if the image file exists. Expect: File exists. fd --type file "bit-logo.svg" --exec file {}Length of output: 226
I'm noticing that the previous verification command failed because the external
file
command wasn’t available in the environment. Let's try a different approach that avoids invoking thefile
command. Please run the following shell script to search for the image file using the more universally availablefind
command:If this script returns one or more paths (especially something like
public/images/bit-logo.svg
or a similar location), it will confirm that the image URL/images/bit-logo.svg
referenced in the component points to an existing file.Once you run this script, please manually verify the output to ensure the image file is located where expected.
🏁 Script executed:
#!/bin/bash # Verify that the image file bit-logo.svg exists anywhere in the repository. find . -type f -name "bit-logo.svg"Length of output: 471
Image URL Verified
The file
bit-logo.svg
was found at locations corresponding to the public directories (e.g., withinBit.BlazorUI.Demo.Client.Core/wwwroot/images/
), confirming that the URL/images/bit-logo.svg
correctly points to an existing image file.src/BlazorUI/Bit.BlazorUI/Components/Buttons/BitButton/BitButton.scss (1)
66-69
: LGTM! Well-structured icon sizing implementation.The changes introduce a robust system for URL-based icon sizing:
- New
.bit-btn-icnu
class ensures consistent width and height for icons.- Size variables scale appropriately with button sizes (small: 1.5, medium: 2, large: 2.5).
- Implementation follows existing patterns and maintains consistency.
Also applies to: 326-326, 338-338, 350-350
closes #9863
Summary by CodeRabbit
New Features
Style
Documentation