Add BitPageVisibility utility class (#10231) #10241
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 add functionality for managing carousel autoplay based on page visibility. The Changes
Sequence Diagram(s)sequenceDiagram
participant B as Browser
participant JS as PageVisibility (TS)
participant VP as BitPageVisibility
participant BC as BitCarousel
B->>JS: visibilitychange event
JS->>VP: invokeMethodAsync("VisibilityChanged", hidden)
VP->>BC: trigger OnChange event (hidden state)
alt Page hidden
BC->>BC: Pause autoplay
else Page visible
BC->>BC: Resume autoplay
end
Poem
🪧 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 (5)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Lists/Carousel/BitCarouselDemo.razor.cs (1)
213-224: Adding Pause and Resume members documentation is appropriate.These new public members match the functionality added to the BitCarousel component for controlling autoplay based on page visibility.
However, consider adding example code that demonstrates the usage of these new methods, similar to how GoNext, GoPrev, and GoTo are implemented and demonstrated.
src/BlazorUI/Bit.BlazorUI/Scripts/PageVisibility.ts (1)
1-13: Implementation of page visibility tracking looks good.The code correctly sets up a one-time event listener for document visibility changes and invokes the .NET method appropriately.
Consider refactoring this to use standalone functions instead of a static class, as suggested by the static analysis:
-namespace BitBlazorUI { - export class PageVisibility { - private static _isInitialized = false; - - public static init(dotnetObj: DotNetObject) { - if (PageVisibility._isInitialized) return; - - PageVisibility._isInitialized = true; - - document.addEventListener('visibilitychange', () => dotnetObj.invokeMethodAsync('VisibilityChanged', document.hidden)); - } - } -} +namespace BitBlazorUI { + let _isInitialized = false; + + export function initPageVisibility(dotnetObj: DotNetObject) { + if (_isInitialized) return; + + _isInitialized = true; + + document.addEventListener('visibilitychange', () => dotnetObj.invokeMethodAsync('VisibilityChanged', document.hidden)); + } +}This approach is generally more aligned with TypeScript best practices for utility functions.
🧰 Tools
🪛 Biome (1.9.4)
[error] 2-12: 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/Utils/BitPageVisibility.cs (2)
1-12: Implementation looks good, but consider implementing IAsyncDisposable.The class is well-structured with clear documentation linking to the MDN docs for the Page Visibility API. The primary constructor pattern for injecting
IJSRuntimefollows modern C# conventions.However, since you're creating a
DotNetObjectReference<BitPageVisibility>which needs to be disposed, this class should implementIAsyncDisposableto properly clean up resources.-public class BitPageVisibility(IJSRuntime js) +public class BitPageVisibility(IJSRuntime js) : IAsyncDisposable { private bool _isInitialized; private DotNetObjectReference<BitPageVisibility>? _dotnetObj; + + public async ValueTask DisposeAsync() + { + _dotnetObj?.Dispose(); + _dotnetObj = null; + }
22-31: Good implementation, but add exception handling.The initialization method prevents multiple initializations and correctly sets up the JavaScript interop. However, there's no exception handling for JavaScript interop failures.
public async Task Init() { if (_isInitialized) return; - - _isInitialized = true; - - _dotnetObj = DotNetObjectReference.Create(this); - - await js.InvokeVoid("BitBlazorUI.PageVisibility.init", _dotnetObj); + + try + { + _dotnetObj = DotNetObjectReference.Create(this); + await js.InvokeVoid("BitBlazorUI.PageVisibility.init", _dotnetObj); + _isInitialized = true; + } + catch (Exception ex) + { + // Set flag back to false if initialization fails + _dotnetObj?.Dispose(); + _dotnetObj = null; + // Consider logging the exception + throw; // Re-throw to let caller handle it + } }src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor.cs (1)
224-234: Add exception handling for Page Visibility initialization.The initialization of the BitPageVisibility service could fail if JavaScript interop encounters an issue. This would disrupt the component's lifecycle since it occurs during rendering.
if (firstRender is false) return; -await _pageVisibility.Init(); +try +{ + await _pageVisibility.Init(); +} +catch (Exception ex) +{ + // Log the exception or handle it appropriately + // Consider how to recover or degrade gracefully +} await _js.BitObserversRegisterResize(UniqueId, RootElement, _dotnetObj);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor.cs(5 hunks)src/BlazorUI/Bit.BlazorUI/Components/Notifications/SnackBar/BitSnackBar.razor.cs(1 hunks)src/BlazorUI/Bit.BlazorUI/Extensions/IServiceCollectionExtensions.cs(1 hunks)src/BlazorUI/Bit.BlazorUI/Scripts/PageVisibility.ts(1 hunks)src/BlazorUI/Bit.BlazorUI/Utils/BitPageVisibility.cs(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Lists/Carousel/BitCarouselDemo.razor.cs(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/BlazorUI/Bit.BlazorUI/Scripts/PageVisibility.ts
[error] 2-12: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (6)
src/BlazorUI/Bit.BlazorUI/Components/Notifications/SnackBar/BitSnackBar.razor.cs (1)
1-1: Cleanup of unused import looks good.Removing the unused
System.Textimport helps maintain clean code.src/BlazorUI/Bit.BlazorUI/Extensions/IServiceCollectionExtensions.cs (1)
12-12: LGTM! Good registration of the new BitPageVisibility service.The service is correctly registered as scoped, consistent with the registration pattern used for BitThemeManager.
src/BlazorUI/Bit.BlazorUI/Utils/BitPageVisibility.cs (1)
15-18: LGTM: Event declaration is appropriate.The event is properly declared with a nullable Func returning Task, which aligns with async event pattern.
src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor.cs (3)
26-26: LGTM: Service injection is correctly implemented.The BitPageVisibility service is properly injected following the component's existing patterns.
199-200: LGTM: Event subscription properly implemented.The event subscription is correctly set up in OnInitialized and properly unsubscribed in DisposeAsync.
470-471: LGTM: Proper event cleanup in DisposeAsync.Correctly unsubscribes from the event handler to prevent memory leaks.
src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor.cs
Outdated
Show resolved
Hide resolved
src/BlazorUI/Bit.BlazorUI/Components/Lists/Carousel/BitCarousel.razor.cs
Outdated
Show resolved
Hide resolved
closes #10231
Summary by CodeRabbit