Add AutoGoToTop parameter to BitAppShell (#9788) #9789
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 enhance navigation handling and component lifecycle management. The Changes
Sequence Diagram(s)sequenceDiagram
participant NM as NavigationManager
participant Shell as BitAppShell
participant JS as JS Runtime / Extras
Note over NM: Navigation event occurs
NM->>Shell: Trigger LocationChanged event
Shell->>Shell: Check AutoGoToTop setting
Shell->>Shell: Invoke GoToTop(BitScrollBehavior.Instant)
Shell->>JS: Call BitExtrasGoToTop(element, behavior)
JS->>Browser: Execute scrollTo(top, behavior)
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: 0
🧹 Nitpick comments (2)
src/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.ts (1)
8-14: LGTM! Consider adding JSDoc comments.The implementation correctly handles the scroll behavior parameter and uses modern TypeScript features.
Consider adding JSDoc comments to document the parameters and their purpose:
+ /** + * Scrolls the specified element to the top. + * @param element - The HTML element to scroll. + * @param behavior - Optional scroll behavior ('auto', 'smooth', or 'instant'). + */ public static goToTop(element: HTMLElement, behavior: ScrollBehavior | undefined) {src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/AppShell/BitAppShellDemo.razor.cs (1)
50-61: LGTM! Comprehensive scroll behavior implementation.The changes introduce:
- A flexible GoToTop method that accepts BitScrollBehavior
- A well-documented BitScrollBehavior enum with appropriate scroll options
Consider adding examples in the documentation to demonstrate the usage of different scroll behaviors.
Also applies to: 154-183
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AppShell/BitAppShell.razor.cs(3 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Extensions/JsInterop/BitScrollBehavior.cs(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Extensions/JsInterop/ExtrasJsRuntimeExtensions.cs(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Scripts/Extras.ts(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/AppShell/BitAppShellDemo.razor(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/AppShell/BitAppShellDemo.razor.cs(3 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/AppFooter.razor.cs(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor(2 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (17)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AppShell/BitAppShell.razor.cs (9)
1-2: Using directive added.
This import is necessary for handling navigation events viaLocationChangedEventArgs.
8-8: Implements IDisposable for proper resource management.
AddingIDisposableto the class is a good practice to ensure subscriptions are cleaned up.
10-10: Private disposal field.
Using_disposedis a standard approach to prevent multiple disposals.
14-16: Injected services.
These injection points look correct, and the usage ofdefault!is acceptable for Blazor components.
19-22: New AutoGoToTop parameter.
The property name and documentation are consistent, providing a clear indication of its purpose.
46-51: GoToTop method.
This method cleanly delegates to_js.BitExtrasGoToTop, with an optional parameter for scroll behavior. No issues noticed.
67-76: Subscribing to LocationChanged conditionally.
This logic correctly wires up the event subscription based on theAutoGoToTopproperty. Good job remembering to unsubscribe later.
80-86: LocationChanged event handler.
InvokingGoToTop(BitScrollBehavior.Instant)upon navigation is a straightforward and effective approach.
90-106: Dispose pattern.
Properly unsubscribing from_navManager.LocationChangedand suppressing finalization is clean and avoids event leaks.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/AppFooter.razor.cs (1)
12-12: Explicit scroll behavior.
CallingAppShell.GoToTop(BitScrollBehavior.Instant)aligns with the updated method signature and provides clear scroll behavior.src/BlazorUI/Bit.BlazorUI.Extras/Extensions/JsInterop/BitScrollBehavior.cs (1)
1-22: New enum for scroll behavior.
The enum values and documentation are straightforward, offering clear scrolling options.src/BlazorUI/Bit.BlazorUI.Extras/Extensions/JsInterop/ExtrasJsRuntimeExtensions.cs (1)
10-13: LGTM! Clean and consistent implementation.The method signature and implementation correctly handle the optional scroll behavior parameter, with proper null handling and case conversion for JS interop.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor (1)
3-3: LGTM! Verify navigation behavior.The
AutoGoToTopattribute has been correctly added toBitAppShell, and the removal of theOnItemClickhandler suggests that navigation is now managed internally.Please verify that:
- Navigation still works correctly without the
OnItemClickhandler- The page automatically scrolls to top on navigation
- The scroll behavior is smooth and user-friendly
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/AppShell/BitAppShellDemo.razor (1)
11-13: LGTM! Documentation parameters added.The addition of
ComponentPublicMembersandComponentSubEnumsparameters enhances the demo's documentation by exposing the new scroll behavior functionality.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Shared/MainLayout.razor.cs (2)
14-17: LGTM! Dependencies are properly injected.The new dependencies are correctly injected using [AutoInject] and initialized with default values.
25-45: LGTM! Robust error handling and event subscriptions.The OnInitialized method now includes:
- Proper error handling with try-catch
- Navigation event subscription
- Page title change subscription with async state updates
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/AppShell/BitAppShellDemo.razor.cs (1)
7-13: LGTM! AutoGoToTop parameter is well-documented.The new parameter is properly defined with clear type, default value, and description, aligning with the PR objectives.
closes #9788
Summary by CodeRabbit