-
-
Notifications
You must be signed in to change notification settings - Fork 235
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Update layout to reduce UI shift while prerendering in Boilerplate (#10024) #10025
Update layout to reduce UI shift while prerendering in Boilerplate (#10024) #10025
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 WalkthroughThis pull request refactors several layout components. The removed Changes
Sequence Diagram(s)sequenceDiagram
participant H as Header Component
participant P as PubSub Service
participant N as NavigationManager
H->>P: Subscribe to PAGE_TITLE_CHANGED
P-->>H: (Tuple: pageTitle, pageSubtitle)
H->>H: Update properties & trigger StateHasChanged()
H->>P: Publish OpenNavPanel message
sequenceDiagram
participant M as MainLayout Component
participant R as RouteData Service
participant UI as UI Renderer
M->>R: SetRouteData()
alt Identity Page Detected
M->>M: Set isIdentityPage = true
M->>UI: Render IdentityHeader
else Non-Identity Page
M->>M: Set isIdentityPage = false
M->>UI: Render Header & NavBar
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
🧹 Nitpick comments (8)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/HeaderButtons.razor (1)
10-28
: Consider adding aria-label for better accessibility.The culture selection dropdown implementation is good, but could benefit from accessibility improvements.
<BitDropdown Items="cultures" FitWidth NoBorder Transparent + AriaLabel="@Localizer[nameof(AppStrings.SelectLanguage)]" DefaultValue="@CultureInfo.CurrentUICulture.Name" OnChange="WrapHandled((string c) => OnCultureChanged(c))">
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/HeaderButtons.razor.cs (1)
17-27
: Consider adding null check for cultures array access.While the initialization looks good, it's good practice to add a null check before accessing the array.
protected override async Task OnInitAsync() { if (CultureInfoManager.MultilingualEnabled) { + var supportedCultures = CultureInfoManager.SupportedCultures; + if (supportedCultures is not null) + { cultures = CultureInfoManager.SupportedCultures .Select(sc => new BitDropdownItem<string> { Value = sc.Culture.Name, Text = sc.DisplayName }) .ToArray(); + } } await base.OnInitAsync(); }src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor (1)
13-27
: Consider refactoring duplicated NavPanel code.The NavPanel rendering logic could be simplified to reduce code duplication while maintaining the same functionality.
-@if (isIdentityPage is false) -{ - if (isAuthenticated is true) - { - <BitNavPanel @bind-IsOpen="isNavPanelOpen" - Items="@navPanelAuthenticatedItems" - IconUrl="_content/Boilerplate.Client.Core/images/bit-logo.svg" /> - } - else if (isAuthenticated is false) - { - <BitNavPanel @bind-IsOpen="isNavPanelOpen" - Items="@navPanelUnAuthenticatedItems" - IconUrl="_content/Boilerplate.Client.Core/images/bit-logo.svg" /> - } -} +@if (isIdentityPage is false) +{ + <BitNavPanel @bind-IsOpen="isNavPanelOpen" + Items="@(isAuthenticated is true ? navPanelAuthenticatedItems : navPanelUnAuthenticatedItems)" + IconUrl="_content/Boilerplate.Client.Core/images/bit-logo.svg" /> +}src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.cs (2)
161-166
: Consider making the namespace check more robust.The current namespace check might be fragile if the namespace structure changes.
Consider using a constant or configuration value for the identity pages namespace:
+private const string IDENTITY_PAGES_NAMESPACE = "Client.Core.Components.Pages.Identity"; -if (type.Namespace?.Contains("Client.Core.Components.Pages.Identity") ?? false) +if (type.Namespace?.Contains(IDENTITY_PAGES_NAMESPACE) ?? false)
180-182
: Consider simplifying the CSS class logic.The nested ternary expressions could be simplified for better readability.
-var authClass = isIdentityPage is false ? "authenticated" - : isIdentityPage is true ? "unauthenticated" - : string.Empty; +var authClass = isIdentityPage switch +{ + false => "authenticated", + true => "unauthenticated", + _ => string.Empty +};src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header.razor.cs (1)
5-12
: Consider initializing fields with default values.The
pageTitle
andpageSubtitle
fields are nullable strings but lack initial values. While this is valid, explicitly initializing them asnull
or empty string would make the intent clearer.- private string? pageTitle; - private string? pageSubtitle; + private string? pageTitle = null; + private string? pageSubtitle = null;src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header.razor (1)
33-36
: Consider extracting URL construction to a helper method.The URL construction logic is duplicated between sign-up and sign-in buttons. Consider extracting it to a helper method for better maintainability.
+ private string GetIdentityUrl(string basePath) => + $"{basePath}?return-url={Uri.EscapeDataString(NavigationManager.GetRelativePath())}"; - Href="@($"{Urls.#Page}?return-url={Uri.EscapeDataString(NavigationManager.GetRelativePath())}")"> + Href="@GetIdentityUrl(Urls.#Page)">Also applies to: 37-42
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header.razor.scss (1)
20-24
: Consider adding transition for smoother responsive behavior.The identity button's display changes abruptly at breakpoints. Consider adding a transition for opacity or transform to make the change smoother.
.identity-button { + transition: opacity 0.2s ease-in-out; @include lt-sm { - display: none; + opacity: 0; + pointer-events: none; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AuthorizedHeader.razor.cs
(0 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header.razor
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header.razor.cs
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header.razor.scss
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/HeaderButtons.razor
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/HeaderButtons.razor.cs
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/HeaderButtons.razor.scss
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/IdentityHeader.razor
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/IdentityHeader.razor.cs
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor
(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.cs
(5 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/NavBar.razor
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/NavBar.razor.cs
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/MainLayout.razor.items.cs
(3 hunks)
💤 Files with no reviewable changes (1)
- src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AuthorizedHeader.razor.cs
✅ Files skipped from review due to trivial changes (1)
- src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/HeaderButtons.razor.scss
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (17)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/NavBar.razor.cs (1)
5-5
: LGTM! Good use of cascading parameter for auth state.The nullable boolean property with cascading parameter is well-suited for handling authentication state across components.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/NavBar.razor (2)
7-17
: LGTM! Good use of conditional rendering for authenticated navigation items.The conditional block properly encapsulates authenticated-only navigation options, which helps reduce UI shift during prerendering.
19-22
: LGTM! Appropriate platform-specific condition for About page.The About page is correctly shown only for unauthenticated users in Blazor Hybrid platform, as mentioned in the code comments.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/MainLayout.razor.items.cs (5)
9-21
: LGTM! Good refactoring to reduce code duplication.Creating reusable nav items for home and terms pages improves maintainability and reduces the chance of inconsistencies.
23-39
: LGTM! Clear separation of unauthenticated navigation items.The unauthenticated navigation items are well-organized and include appropriate icons and URLs.
41-86
: LGTM! Well-structured authenticated navigation items.The authenticated navigation items are properly organized with clear module-specific sections and appropriate conditional compilation directives.
94-103
: LGTM! Good platform-specific handling of About page.The About page is correctly added to both authenticated and unauthenticated items for Blazor Hybrid platform.
105-117
: LGTM! Comprehensive settings navigation setup.The settings navigation item is well-configured with all relevant section URLs.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/HeaderButtons.razor (1)
3-8
: LGTM! Theme toggle button implementation looks good.The button implementation is clean and follows best practices:
- Uses
IconOnly
andFixedColor
for consistent styling- Properly handles theme toggle via
WrapHandled
- Icon changes appropriately based on current theme
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/HeaderButtons.razor.cs (1)
5-14
: LGTM! Clean dependency injection and parameter declarations.The code follows best practices:
- Uses
AutoInject
for service injection- Properly declares cascading parameters with appropriate nullability
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/IdentityHeader.razor.cs (1)
31-32
: LGTM! Clear comment explaining the StateHasChanged requirement.The comment effectively explains why
StateHasChanged
is needed on location changes.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/IdentityHeader.razor (1)
35-35
: LGTM! Clean integration of HeaderButtons component.The HeaderButtons component is properly integrated into the header layout, maintaining the same functionality while improving code organization.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor (2)
30-38
: LGTM! Clear and safe conditional rendering.The header component rendering logic is well-structured with explicit null checks and clear separation between identity and non-identity pages.
53-56
: LGTM! Consistent with the layout pattern.The NavBar rendering logic follows the same pattern as other components, with explicit null checks and clear conditions.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.cs (1)
10-28
: LGTM! Well-organized property declarations.The property changes improve code organization and separation of concerns:
- Added
isIdentityPage
for better state management- Split navigation items for better control over authenticated vs unauthenticated states
- Removed redundant null initialization
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header.razor.cs (2)
15-25
: Consider handling potential race conditions in async event handler.The event handler uses async/await but doesn't handle potential race conditions that could occur if multiple title changes happen in quick succession.
Consider implementing debouncing or ensuring state updates are atomic:
unsubscribePageTitleChanged = PubSubService.Subscribe(ClientPubSubMessages.PAGE_TITLE_CHANGED, async payload => { + await InvokeAsync(() => { (pageTitle, pageSubtitle) = ((string, string))payload!; StateHasChanged(); + }); });
41-47
: LGTM! Proper cleanup implementation.The disposal method correctly unsubscribes from events and calls the base implementation.
...oilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header.razor
Outdated
Show resolved
Hide resolved
…/github.com/msynk/bitframework into 10024-templates-boilerplate-simplify-layout
…/github.com/msynk/bitframework into 10024-templates-boilerplate-simplify-layout
…/github.com/msynk/bitframework into 10024-templates-boilerplate-simplify-layout
Signed-off-by: Yaser Moradi <ysmoradi@outlook.com>
…/github.com/msynk/bitframework into 10024-templates-boilerplate-simplify-layout
…/github.com/msynk/bitframework into 10024-templates-boilerplate-simplify-layout
…/github.com/msynk/bitframework into 10024-templates-boilerplate-simplify-layout
closes #10024
Summary by CodeRabbit
New Features
Refactor