Fix chevron icon issues (#9075) #9082
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 primarily involve updates to icon usage and styling within 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: 0
🧹 Outside diff range and nitpick comments (7)
src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.scss (1)
70-72: LGTM! Consider adding a comment for clarity.The implementation is correct and efficient, using GPU-accelerated CSS transform for the horizontal flip effect.
Consider adding a brief comment to document the purpose:
+/* Horizontally flips an element (used for converting right-facing icons to left-facing) */ .bit-csl-rbi { transform: scaleX(-1); }src/BlazorUI/Bit.BlazorUI/Components/Navs/Pagination/BitPagination.razor (1)
78-78: Last button icon implementation is well-implemented!The implementation correctly follows the established pattern and handles directionality appropriately.
Consider extracting the default icon names into constants to improve maintainability:
+private const string DefaultFirstLastIcon = "ChevronRightEnd6"; +private const string DefaultPrevNextIcon = "ChevronRight"; -<i ... class="bit-icon bit-icon--@(FirstIcon ?? "ChevronRightEnd6") ..." /> +<i ... class="bit-icon bit-icon--@(FirstIcon ?? DefaultFirstLastIcon) ..." />src/BlazorUI/Bit.BlazorUI/Components/Navs/Pagination/BitPagination.scss (1)
35-37: LGTM! Consider adding a descriptive comment.The implementation of
.bit-pgn-btlwithscaleX(-1)transform is a clean solution for horizontally flipping icons. This aligns well with the PR objective of using right chevron icons with appropriate transformations.Consider adding a comment to explain the purpose of this class:
+// Horizontally flips icons (e.g., converting right chevron to left chevron) .bit-pgn-btl { transform: scaleX(-1); }src/BlazorUI/Bit.BlazorUI/Components/Navs/Pagination/BitPagination.razor.cs (2)
46-46: Consider documenting the default icon values.Since the icon properties are now nullable, consider adding XML documentation to specify the default icon values that will be used when these properties are null. This helps developers understand the component's behavior without having to check the razor template.
/// <summary> - /// The icon name of the first button. + /// The icon name of the first button. If null, defaults to ChevronRightEnd6 with horizontal flip. /// </summary> [Parameter] public string? FirstIcon { get; set; } /// <summary> - /// The icon name of the last button. + /// The icon name of the last button. If null, defaults to ChevronRightEnd6. /// </summary> [Parameter] public string? LastIcon { get; set; } /// <summary> - /// The icon name of the next button. + /// The icon name of the next button. If null, defaults to ChevronRight. /// </summary> [Parameter] public string? NextIcon { get; set; } /// <summary> - /// The icon name of the previous button. + /// The icon name of the previous button. If null, defaults to ChevronRight with horizontal flip. /// </summary> [Parameter] public string? PreviousIcon { get; set; }Also applies to: 51-51, 63-63, 73-73
46-46: Consider adding migration guide for breaking changes.The removal of default values from icon properties is a breaking change that might affect existing applications. Consider adding a migration guide in the PR description or documentation to help users update their code.
Also applies to: 51-51, 63-63, 73-73
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/Pagination/BitPaginationDemo.razor.cs (2)
49-50: Verify FirstIcon documentation completeness.The parameter's type has been updated to be nullable, but the description doesn't mention the default icon that will be used when null. Consider updating the description to clarify the fallback behavior.
Suggested description update:
- Description = "The icon name of the first button." + Description = "The icon name of the first button. When null, defaults to ChevronRightEnd6 with appropriate transformation."
56-57: Ensure consistent documentation for icon parameters.The changes to make LastIcon, NextIcon, and PreviousIcon nullable align with the new flexible icon system. However, like FirstIcon, their descriptions should be updated to document the default behavior when null.
Consider updating the descriptions to be more informative:
- Description = "The icon name of the last button." + Description = "The icon name of the last button. When null, defaults to ChevronRightEnd6." - Description = "The icon name of the next button." + Description = "The icon name of the next button. When null, defaults to ChevronRight." - Description = "The icon name of the previous button." + Description = "The icon name of the previous button. When null, defaults to ChevronRight with appropriate transformation."Also applies to: 70-71, 84-85
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor(1 hunks)src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.scss(1 hunks)src/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.razor(1 hunks)src/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.scss(1 hunks)src/BlazorUI/Bit.BlazorUI/Components/Navs/Pagination/BitPagination.razor(4 hunks)src/BlazorUI/Bit.BlazorUI/Components/Navs/Pagination/BitPagination.razor.cs(3 hunks)src/BlazorUI/Bit.BlazorUI/Components/Navs/Pagination/BitPagination.scss(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/Pagination/BitPaginationDemo.razor.cs(3 hunks)
🔇 Additional comments (9)
src/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.razor (1)
25-25: LGTM! Elegant solution using CSS transform.
The change aligns with the PR objectives by replacing the ChevronLeft icon with ChevronRight. Using CSS transform via bit-swp-lbi class is an elegant approach as it maintains DRY principles by reusing the same icon.
Let's verify the CSS class exists:
✅ Verification successful
CSS transform class is properly defined and implemented
The bit-swp-lbi class is correctly defined in the SCSS file with transform: scaleX(-1), which will horizontally flip the ChevronRight icon to create a left-pointing chevron. This confirms that the elegant solution using CSS transform is working as intended.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the bit-swp-lbi class is defined in the stylesheet
# Expected: Should find the class definition with transform: scaleX(-1)
rg -l "bit-swp-lbi" "src/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.scss"
rg "bit-swp-lbi.*transform:\s*scaleX\(-1\)" "src/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.scss"
Length of output: 260
Script:
#!/bin/bash
# Let's check the actual content of the SCSS file to see the definition of bit-swp-lbi class
cat "src/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.scss"
Length of output: 1912
src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor (1)
30-30: Implementation aligns with requirements.
The change correctly implements the ChevronRight icon with a flip transformation class for the right button, while the left button uses the same icon without transformation.
Let's verify the CSS class implementation:
✅ Verification successful
CSS class implementation is correct and functional
The bit-csl-rbi class is properly defined in the carousel SCSS file and uses transform: scaleX(-1) to horizontally flip the ChevronRight icon, which is the correct approach for creating a mirror image of the right button icon to serve as the left button icon. This implementation aligns with the review comment's objectives.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the CSS class implementation for the flip transform
# Expected: Find the bit-csl-rbi class definition in the SCSS/CSS files
# Search for the CSS class definition
rg -t scss -t css 'bit-csl-rbi'
Length of output: 60
Script:
#!/bin/bash
# Let's try different approaches to find the CSS class:
# 1. Search for the class in all files (not just scss/css)
rg "bit-csl-rbi"
# 2. Find all potential style files using fd
fd -e css -e scss -e sass -e less
# 3. Look for carousel-related style files
fd -e css -e scss -e sass -e less -g "*carousel*"
# 4. Search for transform/rotate styles that might be related
rg -t css "transform.*rotate|rotate.*transform"
Length of output: 365121
Script:
#!/bin/bash
# Let's check the carousel SCSS file to see the bit-csl-rbi class definition
cat src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.scss
Length of output: 1938
src/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.scss (1)
64-66: LGTM! Clean implementation of icon flipping.
The new .bit-swp-lbi class implements a horizontal flip transformation that elegantly solves the chevron icon replacement requirement. The implementation:
- Follows the existing naming conventions
- Uses the standard
scaleX(-1)transform for horizontal flipping - Is placed logically in the stylesheet structure
src/BlazorUI/Bit.BlazorUI/Components/Navs/Pagination/BitPagination.razor (4)
16-16: First button icon implementation looks good!
The implementation correctly handles the icon direction by applying the horizontal flip class when appropriate, ensuring consistent visual representation in both LTR and RTL modes.
27-27: Previous button icon implementation is consistent!
The implementation follows the same pattern as the First button, maintaining consistency in the codebase while correctly handling directionality.
67-67: Next button icon implementation correctly handles directionality!
The implementation appropriately inverts the flip logic for RTL mode, ensuring the icon points in the correct direction for "next" navigation in both LTR and RTL contexts.
Line range hint 16-78: Overall implementation is robust and well-structured!
The changes consistently handle icon directionality across all navigation buttons, successfully addressing the chevron icon issue (#9075). The implementation:
- Uses a consistent pattern for all navigation buttons
- Correctly handles both LTR and RTL modes
- Provides flexibility through nullable icon parameters
- Maintains proper fallback behavior
src/BlazorUI/Bit.BlazorUI/Components/Navs/Pagination/BitPagination.razor.cs (1)
46-46: Verify the default icon fallbacks in the razor template.
The icon properties have been made nullable, which is a breaking change. Ensure that the razor template (BitPagination.razor) implements appropriate fallback logic to handle null values and uses the correct chevron icons as specified in issue #9075.
Also applies to: 51-51, 63-63, 73-73
✅ Verification successful
Let me check the content of the remaining files that still contain references to the old icons.
Icon properties and fallback implementation are correctly handled
The verification shows that:
- The razor template implements proper fallback logic for all nullable icon properties using the null coalescing operator (
??) - Default icons are set to
ChevronRightandChevronRightEnd6as intended - RTL support is handled via the
bit-pgn-btlclass based on theDirproperty
The remaining references to ChevronLeft and ChevronLeftEnd6 in style and icon definition files are expected as they are part of the icon library's complete set of available icons.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the razor template implementation and icon usage
# Test 1: Check the razor template for fallback logic
echo "Checking BitPagination.razor for fallback logic..."
rg -A 5 'FirstIcon|LastIcon|NextIcon|PreviousIcon' 'src/BlazorUI/Bit.BlazorUI/Components/Navs/Pagination/BitPagination.razor'
# Test 2: Verify no remaining usage of old chevron icons
echo "Checking for any remaining usage of old chevron icons..."
rg -l 'ChevronLeft|ChevronLeftEnd6' 'src/BlazorUI'
Length of output: 1614
Script:
#!/bin/bash
# Check the content of files still containing old chevron references
echo "Checking fabric.mdl2.bit.blazoui.scss..."
rg -C 2 'ChevronLeft|ChevronLeftEnd6' 'src/BlazorUI/Bit.BlazorUI/Styles/fabric.mdl2.bit.blazoui.scss'
echo "Checking BitIconName.cs..."
rg -C 2 'ChevronLeft|ChevronLeftEnd6' 'src/BlazorUI/Bit.BlazorUI.Icons/BitIconName.cs'
echo "Checking fabric.mdl2.icons.scss..."
rg -C 2 'ChevronLeft|ChevronLeftEnd6' 'src/BlazorUI/Bit.BlazorUI.Icons/Styles/fabric.mdl2.icons.scss'
Length of output: 3107
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/Pagination/BitPaginationDemo.razor.cs (1)
49-50: Verify the impact on demo examples.
The change to nullable icon parameters with null defaults represents a significant change in the component's behavior. Please ensure that the demo page includes examples demonstrating:
- Default behavior with null icons
- Custom icon override scenarios
- RTL handling with the new icon system
Also applies to: 56-57, 70-71, 84-85
✅ Verification successful
The demo page adequately covers all icon-related scenarios
Looking at the demo page content, I can verify that:
- Default behavior with null icons is demonstrated in multiple examples (Basic, Variants, etc.)
- Custom icon override scenarios are shown in example 7 with
NextIcon="@BitIconName.Next" PreviousIcon="@BitIconName.Previous" - RTL handling is demonstrated in example 12 with proper RTL pagination examples
The demo page comprehensively covers all the necessary scenarios related to the nullable icon parameters change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the demo page includes examples for both default and custom icons
rg -l "BitPaginationDemo\.razor" | xargs rg -A 10 "<BitPagination"
Length of output: 68
Script:
#!/bin/bash
# Let's try a different approach to find the pagination demo files and their content
fd "BitPaginationDemo.razor" --type f
# After finding the file, let's look at its content
fd "BitPaginationDemo.razor" --type f --exec cat {}
Length of output: 30784
This closes #9075
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores