Improve MainLayout in Boilerplate (#10976) #10978
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 update refactors the layout system in the Boilerplate project template. It extracts a new Changes
Sequence Diagram(s)sequenceDiagram
participant MainLayout
participant AppShell
participant BitNavPanel
participant PubSub
participant User
MainLayout->>AppShell: Render with navPanelItems, isIdentityPage, ChildContent
AppShell->>BitNavPanel: Conditionally render based on isIdentityPage
User->>AppShell: Interact (e.g., open/close nav panel, sign-in, update app)
AppShell->>PubSub: Subscribe to OPEN_NAV_PANEL and CLOSE_NAV_PANEL
PubSub-->>AppShell: Notify to open/close nav panel
AppShell->>AppShell: Update nav panel state and UI
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
✨ Finishing Touches🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 4
🧹 Nitpick comments (9)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/ClientPubSubMessages.cs (1)
16-18: Add XML-doc & keep alphabetical order for new constant
CLOSE_NAV_PANELis a welcome addition, but:
- Constants are currently alphabetically grouped; inserting after
OPEN_NAV_PANELbreaks that ordering.- Other message constants have an XML-doc where additional context is useful for tooling (
OPEN_NAV_PANELlacks it as well, but we shouldn’t amplify the pattern).- public const string OPEN_NAV_PANEL = nameof(OPEN_NAV_PANEL); - public const string CLOSE_NAV_PANEL = nameof(CLOSE_NAV_PANEL); + /// <summary>Publish to request opening the navigation panel.</summary> + public const string OPEN_NAV_PANEL = nameof(OPEN_NAV_PANEL); + /// <summary>Publish to request closing the navigation panel.</summary> + public const string CLOSE_NAV_PANEL = nameof(CLOSE_NAV_PANEL);src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header/IdentityHeader.razor.cs (1)
31-34: Avoid needlessasynckeyword
HandleGoHomeLinkis declaredasync Taskbut performs noawait. Dropasyncand returnvoid, or addawait Task.CompletedTask;to silence analyzers.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Common/ProductImage.razor (1)
1-4: Guard against emptyWidthproducing invalid styleWhen
Widthis null/empty the inline style becomeswidth:;, which is invalid CSS and may break layout.
Consider emitting thewidthrule only when a value is supplied:<div style="@(!string.IsNullOrWhiteSpace(Width) ? $"width:{Width};overflow:hidden;position:relative;" : "overflow:hidden;position:relative;")" class="@Class">src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor (1)
8-13: Deep cascade chain is hard to maintainFive nested
<CascadingValue>wrappers add noise and risk mismatched names. Now thatAppShellencapsulates most layout concerns, consider moving theseCascadingValues insideAppShell(or a dedicated provider component) to flattenMainLayoutand improve readability.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Common/ProductImage.razor.cs (1)
5-10: Add minimal validation or XML docsParameters like
Widthexpect CSS units; adding summary comments or simple validation (e.g., ensure units present) would make the component more self-documenting and safer for consumers.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppShell.razor (2)
9-15: Remove redundantIsNavPanelOpenparameter
@bind-IsOpen="isNavPanelOpen"operates on the private field, while the publicIsNavPanelOpenparameter declared in the code-behind is never used.
Delete the unused parameter or switch the binding to it to avoid two sources of truth.
65-67: Add alt text for accessibility
BitImagerenders decorative content without anyAltattribute.
Provide meaningful alt text (or explicit empty alt when decorative) to satisfy WCAG 2.1 AA.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppShell.razor.cs (1)
10-12: Delete or wire upIsNavPanelOpenparameterThis parameter is declared but never used, which is misleading.
Either bind the panel to it or remove it to keep the public surface minimal.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.cs (1)
29-33: Update XML doc or remove dead referenceThe summary still refers to
Parameters.IsOnline, which no longer exists.
Trim or update the comment to match the current code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (15)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Common/ProductImage.razor(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Common/ProductImage.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppAiChatPanel.razor(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppAiChatPanel.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppShell.razor(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppShell.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppShell.razor.scss(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header/AppMenu.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header/IdentityHeader.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.cs(6 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/AccountSection.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/ProfileSection.razor.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Parameters.cs(0 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/ClientPubSubMessages.cs(1 hunks)
💤 Files with no reviewable changes (1)
- src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Parameters.cs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (7)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppAiChatPanel.razor (1)
42-43: LGTM – whitespace correction onlyThe
AutoScrollattribute trim is purely cosmetic and correct.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppAiChatPanel.razor.cs (1)
13-14: Verify cascading parameter binding after name removalDropping the explicit
Name = …relies on default matching by type or property name.
If anotherAppThemeType?is cascaded higher in the tree, the binding may become ambiguous.
Please double-check that there is exactly one such cascade source; otherwise re-introduce an explicit name.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/ProfileSection.razor.cs (1)
10-11: Potential ambiguity inCurrentUsercascade after explicit name removalSame concern as for
AppAiChatPanel: ensure only a singleUserDtocascade exists; otherwise keep the name to avoid accidental mismatch (especially in unit tests & prerender).src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Settings/Account/AccountSection.razor.cs (1)
7-8: Confirm single‐source cascade forUserDto
CurrentUsernow binds by type/name. If both anonymous and typed cascades exist (e.g., one for auth, one for impersonation), Blazor will pick the closest, which might not be intended. Please audit upstream providers.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header/IdentityHeader.razor.cs (1)
5-8: Verify cascade ambiguity after droppingNameattributeRemoving
Name = Parameters.CurrentThememakes the parameter match by type only. If multipleAppThemeTypevalues are provided higher up (e.g., theme override for dialogs), resolution will be ambiguous at runtime. Double-check that only one unnamedAppThemeTypeis cascaded through the component tree.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor (1)
20-21: ConfirmModalParametersconstant rename
BitModalContainernow expectsModalParameters. Make sure the backing field inMainLayout.razor.csis public or at least accessible; otherwise binding will fail at runtime.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Header/AppMenu.razor.cs (1)
11-13: Verify cascade source after removingNameattributesThe parameters will now match solely by type.
Please double-check that every ancestor cascadingUserDto/AppThemeTypehas also dropped the customNameor these values will arrive asnullat runtime.
...te/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppAiChatPanel.razor
Outdated
Show resolved
Hide resolved
...ate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppShell.razor.scss
Show resolved
Hide resolved
...plate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppShell.razor.cs
Show resolved
Hide resolved
...plate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/AppShell.razor.cs
Show resolved
Hide resolved
Signed-off-by: Yaser Moradi <ysmoradi@outlook.com>
Signed-off-by: Yaser Moradi <ysmoradi@outlook.com>
Signed-off-by: Yaser Moradi <ysmoradi@outlook.com>
2 participants
closes #10976
Summary by CodeRabbit
New Features
Refactor
Style