BitCarousel component improvements (#9705) #9711
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 pull request introduces modifications to the Bit.BlazorUI carousel and swiper components, focusing on enhancing flexibility and code organization. Changes include renaming variables, updating method signatures, and adding new customization options like custom navigation icons. The modifications span multiple files across the Blazor UI project, affecting both the component implementations and their associated TypeScript and CSS files. 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: 3
🧹 Nitpick comments (2)
src/BlazorUI/Bit.BlazorUI/Scripts/Observers.ts (1)
5-14: Consider making the callback method name configurable.The hardcoded "OnResize" method name reduces flexibility. Consider accepting the method name as a parameter to support different callback names for different use cases.
- public static registerResize(id: string, element: HTMLElement, obj: DotNetObject) { + public static registerResize(id: string, element: HTMLElement, obj: DotNetObject, methodName: string = "OnResize") { const observer = new ResizeObserver(entries => { const entry = entries[0]; if (!entry) return; - obj.invokeMethodAsync("OnResize", entry.contentRect); + obj.invokeMethodAsync(methodName, entry.contentRect); });src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor.cs (1)
279-284: Ensure timer state is properly managed.The timer state management could be improved to handle edge cases:
- Timer operations should be guarded against null
- Consider using a single await for the delay
-if (AutoPlay) _autoPlayTimer.Stop(); -await Task.Delay(50); -if (AutoPlay) _autoPlayTimer.Start(); +if (AutoPlay && _autoPlayTimer is not null) +{ + _autoPlayTimer.Stop(); + await Task.Delay(50); + _autoPlayTimer.Start(); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor(2 hunks)src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor.cs(13 hunks)src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.scss(1 hunks)src/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.razor.cs(6 hunks)src/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.ts(1 hunks)src/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/ObserversJsRuntimeExtensions.cs(1 hunks)src/BlazorUI/Bit.BlazorUI/Scripts/Observers.ts(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Lists/Carousel/BitCarouselDemo.razor(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Lists/Carousel/BitCarouselDemo.razor
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (11)
src/BlazorUI/Bit.BlazorUI/Scripts/Observers.ts (1)
Line range hint
17-23: Verify object disposal.The
obj.dispose()call might cause issues if the same DotNetObject is used for multiple observers. Consider removing it and letting the caller handle disposal.🧰 Tools
🪛 Biome (1.9.4)
[error] 2-26: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
src/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/ObserversJsRuntimeExtensions.cs (1)
8-14: LGTM! Good use of DynamicallyAccessedMembers attribute.The attribute ensures proper trimming behavior when using AOT compilation.
src/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.ts (1)
22-22: LGTM! Consistent event handler naming.The change to "OnPointerLeave" aligns with the naming convention used for other event handlers like "OnResize".
src/BlazorUI/Bit.BlazorUI/Components/Lists/Swiper/BitSwiper.razor.cs (4)
96-101: LGTM! Method renamed for consistency.The method has been renamed from
OnRootResizeto_OnResizeto follow the naming convention for internal methods.
103-107: LGTM! Pointer leave handling refactored.The pointer leave handling has been refactored to separate concerns, with
_OnPointerLeavedelegating toHandlePointerLeave.
230-249: LGTM! Well-structured pointer leave handling.The
HandlePointerLeavemethod contains a well-organized implementation with clear steps:
- Early return if pointer is not down
- Reset pointer state and cursor style
- Calculate swipe metrics
- Apply transition based on swipe speed
- Calculate and apply final position
298-305: LGTM! Proper disposal handling.The disposal logic correctly handles the cleanup of the resize observer, with appropriate error handling for JSDisconnectedException.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.NavItems.cs (1)
73-74: LGTM! Improved component descriptions.Added clear and concise descriptions for both components:
- Carousel: "Slideshow"
- Swiper: "Touch slider"
src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor.cs (2)
50-58: LGTM! Added icon customization parameters.New parameters allow customization of navigation button icons:
GoLeftIcon: Custom icon for the left navigation buttonGoRightIcon: Custom icon for the right navigation button
224-227: LGTM! Improved code readability.Added braces to if conditions in infinite scrolling logic, making the code more maintainable and less prone to errors.
Also applies to: 240-243
src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.scss (1)
27-29: LGTM! Improved selector specificity.Changed to direct child selector (
>) for better performance and more precise targeting.
src/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/ObserversJsRuntimeExtensions.cs
Outdated
Show resolved
Hide resolved
src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor
Outdated
Show resolved
Hide resolved
src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor
Outdated
Show resolved
Hide resolved
closes #9705
Summary by CodeRabbit
New Features
Bug Fixes
Refactor