Update layout structure of Boilerplate (#9565)#9566
Update layout structure of Boilerplate (#9565)#9566msynk merged 6 commits intobitfoundation:developfrom
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 significant updates to the Boilerplate project's client-side layout and styling infrastructure. The changes primarily focus on replacing legacy CSS custom properties with new SCSS variables, updating component layouts, and refactoring navigation and layout management. Key modifications include replacing 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 (13)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/SnackBar.razor.scss (1)
4-4: Consider using a SCSS variable for the spacing value.While the calculation correctly uses the SCSS variable for inset, consider extracting the fixed
1remvalue to a SCSS variable for better maintainability and consistency across components.- bottom: calc(#{$bit-env-inset-bottom} + 1rem); + bottom: calc(#{$bit-env-inset-bottom} + #{$bit-spacing-base});src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/IdentityHeader.razor.scss (1)
18-19: Consider removing the!importantflagWhile the migration from CSS custom properties to SCSS variables looks good, the use of
!importanton thetopproperty might indicate a specificity issue in the CSS cascade. This could make the styles harder to maintain or override when needed.Consider resolving the specificity conflict by:
- Restructuring the selectors to be more specific
- Reviewing where this style might be getting overridden
- If the override is necessary, document why the
!importantflag is required.language-callout { @include lt-sm { height: unset; max-width: 225px; box-shadow: none; bottom: $bit-env-inset-bottom; - top: $bit-env-inset-top !important; + top: $bit-env-inset-top; } }src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MessageBox.razor.scss (1)
24-24: Consider documenting the height calculation.The
3remoffset in the calculationcalc(#{$bit-env-height-available} - 3rem)would benefit from a brief comment explaining its purpose..stack { + // 3rem offset accounts for header and footer spacing max-height: calc(#{$bit-env-height-available} - 3rem); }src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Prompt.razor.scss (1)
24-24: Consider viewport-specific adjustments for the stack height calculation.While the SCSS variable transition is correct, the fixed
3remsubtraction might need adjustment for different viewport sizes. Consider making this value responsive or documenting why it's fixed at 3rem..stack { - max-height: calc(#{$bit-env-height-available} - 3rem); + max-height: calc(#{$bit-env-height-available} - var(--stack-offset, 3rem)); + @include lt-md { + --stack-offset: 2rem; + } }src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Styles/abstracts/_bit-css-variables.scss (3)
325-328: Ensure consistent naming in CSS custom properties.The CSS custom properties for these variables follow the established pattern, but some corresponding properties in other sections use abbreviated forms. Consider using full words consistently.
-$bit-env-width-percent: var(--bit-env-width-per); -$bit-env-height-percent: var(--bit-env-height-per); -$bit-env-width-available: var(--bit-env-width-avl); -$bit-env-height-available: var(--bit-env-height-avl); +$bit-env-width-percent: var(--bit-env-width-percent); +$bit-env-height-percent: var(--bit-env-height-percent); +$bit-env-width-available: var(--bit-env-width-available); +$bit-env-height-available: var(--bit-env-height-available); -$bit-env-window-width: var(--bit-env-win-width); -$bit-env-window-height: var(--bit-env-win-height); +$bit-env-window-width: var(--bit-env-window-width); +$bit-env-window-height: var(--bit-env-window-height);
324-341: Add descriptive comments for the new environment variables section.The new section would benefit from documentation explaining the purpose and usage of these variables.
// Extras +// Environment variables for managing layout dimensions and safe areas +// - Inset variables: Define safe area insets for different edges of the viewport $bit-env-inset-top: var(--bit-env-inset-top); $bit-env-inset-left: var(--bit-env-inset-left); $bit-env-inset-right: var(--bit-env-inset-right); $bit-env-inset-bottom: var(--bit-env-inset-bottom); //-- +// - Viewport and container dimensions: Define various width and height measurements $bit-env-width-vw: var(--bit-env-width-vw); $bit-env-height-vh: var(--bit-env-height-vh); $bit-env-width-percent: var(--bit-env-width-per); $bit-env-height-percent: var(--bit-env-height-per); $bit-env-width-available: var(--bit-env-width-avl); $bit-env-height-available: var(--bit-env-height-avl); //-- +// - Logical properties: Support for RTL/LTR layouts $bit-env-inset-inline-start: var(--bit-env-inset-inline-start); $bit-env-inset-inline-end: var(--bit-env-inset-inline-end); //-- +// - Window dimensions: Define browser window measurements $bit-env-window-width: var(--bit-env-win-width); $bit-env-window-height: var(--bit-env-win-height);
324-341: Consider using a more descriptive section name than "Extras".The new variables are specifically related to environment and layout dimensions. Consider renaming the section to better reflect its purpose.
-// Extras +/*---- Environment & Layout ----*/src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor (1)
18-18: Enhance accessibility for<BitNavPanel>
The<BitNavPanel>is togglable and bound toisNavPanelOpen; consider adding ARIA attributes such asaria-expanded, or configuring a label for screen readers. This helps users who rely on assistive technologies.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/MainLayout.razor.items.cs (1)
7-91: Consider a more data-driven approach for menu items
TheInitializeNavPanelItemsmethod is well-structured, but the multiple conditional directives and repeated item definitions can become large and difficult to maintain. A data-driven approach (e.g., loading items from a config or a builder pattern) could improve clarity.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.cs (4)
9-9:isNavPanelOpennaming
This straightforward boolean field name is descriptive enough. A property could be used if external components need to set it directly.
25-25: Prefer an interface or readonly collection
private List<BitNavItem> navPanelItems = [];works, but consider usingIReadOnlyList<BitNavItem>if you do not need to modify the collection outside this class. This prevents accidental external mutations.
77-82: Open nav panel on demand
SettingisNavPanelOpen = trueupon receivingOPEN_NAV_PANELis straightforward and clear. Consider logging or telemetry if you need usage statistics for UI events.
88-89: Synchronous vs. asynchronous calls
await bitExtraServices.AddRootCssClasses()followed byawait base.OnInitializedAsync()is logical here. If performance becomes a concern, you might investigate parallelizing or caching.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/ClientAppCoordinator.cs(0 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/DiagnosticModal.razor.scss(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/IdentityHeader.razor.scss(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.cs(4 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.scss(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MessageBox.razor.scss(2 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.scss(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Prompt.razor.scss(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/SnackBar.razor.scss(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/UserMenu.razor.scss(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Categories/CategoriesPage.razor.scss(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Products/ProductsPage.razor.scss(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Todo/TodoPage.razor.scss(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Routes.razor(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Extensions/IJSRuntimeExtensions.cs(0 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/MainLayout.razor.items.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/app.ts(0 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Styles/abstracts/_bit-css-variables.scss(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Styles/app.scss(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/compilerconfig.json(1 hunks)
💤 Files with no reviewable changes (3)
- src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/app.ts
- src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Extensions/IJSRuntimeExtensions.cs
- src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/ClientAppCoordinator.cs
🔇 Additional comments (31)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/SnackBar.razor.scss (1)
1-1: LGTM! Consistent with new architecture.
The import of SCSS variables from the abstracts folder aligns well with the project's transition from CSS custom properties to SCSS variables.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MessageBox.razor.scss (1)
1-2: LGTM! Import changes align with standardization efforts.
The addition of bit-css-variables import supports the transition to SCSS variables while maintaining good import ordering practices.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Products/ProductsPage.razor.scss (2)
10-10: LGTM: Good transition to SCSS variables
The change from CSS custom properties to SCSS variables is a good practice as it provides better compile-time checking and maintains consistency with modern SCSS practices.
13-17: Review the height calculations across breakpoints
The current height calculations across breakpoints seem counter-intuitive:
- Default: -14rem
- lt-md: -17rem
- lt-sm: -16rem
The lt-md breakpoint subtracts more height (17rem) than lt-sm (16rem), which is unusual as smaller screens typically need more space for UI elements. Please verify if this is intentional.
Let's check other similar components for consistency:
✅ Verification successful
Let's also check for any other grid containers in the codebase:
And let's specifically look at this component's full SCSS file to understand the breakpoint context better:
The height calculations are intentional and correct
After reviewing the media query breakpoints, the height calculations make sense:
- Default (> 960px): -14rem
- lt-md (≤ 959px): -17rem
- lt-sm (≤ 599px): -16rem
The larger subtraction at lt-md breakpoint (≤959px) is logical because:
- Tablets and smaller laptops (lt-md) often need more space for UI elements like headers/toolbars
- The smallest devices (lt-sm) can use a slightly smaller subtraction since mobile UIs are typically more compact
- Larger screens (default) need the least subtraction since they have more vertical space
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check height calculations in other similar component styles
# Look for any .grid-container height calculations in SCSS files
rg -U "\.grid-container\s*{[^}]*height:\s*calc\([^}]*\)" --type scss
Length of output: 221
Script:
#!/bin/bash
# Let's try a simpler search pattern to find height calculations in SCSS files
rg "height:\s*calc" --type scss -A 2 -B 2
Length of output: 72
Script:
#!/bin/bash
# Search for grid-container class definitions
rg "\.grid-container" --type scss -A 5 -B 2
Length of output: 74
Script:
#!/bin/bash
cat "src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Products/ProductsPage.razor.scss"
Length of output: 2022
Script:
#!/bin/bash
# Let's try searching without file type specification
rg "height:\s*calc" -A 2 -B 2
Length of output: 131070
Script:
#!/bin/bash
# Let's also check the media queries definitions to understand the breakpoints
cat "src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Styles/abstracts/_media-queries.scss"
Length of output: 1872
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Prompt.razor.scss (2)
1-2: LGTM! Import structure is well-organized.
The addition of _bit-css-variables.scss import aligns with the transition to SCSS variables, and the import order follows good practices.
7-7: Verify the SCSS variable definition and value.
The transition from CSS custom property to SCSS variable looks good, but let's verify the variable definition.
✅ Verification successful
SCSS variable is properly defined and accessible
The variable $bit-env-height-available is correctly defined in the _bit-css-variables.scss files and maps to the CSS variable var(--bit-env-height-avl). The change from var(--app-height) to $bit-env-height-available is consistent with the project's CSS variable naming convention and structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the SCSS variable definition and its usage across the codebase
# Check if the variable is defined in _bit-css-variables.scss
echo "Checking variable definition:"
rg -A 3 "\$bit-env-height-available" "src/**/\_bit-css-variables.scss"
# Check for any remaining usage of the old CSS variable
echo -e "\nChecking for any remaining --app-height usage:"
rg "var\(--app-height\)" "src/**/*.scss"
Length of output: 418
Script:
#!/bin/bash
# Let's first find the SCSS files and their locations
echo "Finding SCSS files:"
fd -e scss
# Then search for the variable definition in all SCSS files
echo -e "\nSearching for variable definition:"
rg "\$bit-env-height-available" -A 3
# Check for any remaining usage of the old CSS variable
echo -e "\nChecking for any remaining --app-height usage:"
rg "var\(--app-height\)"
Length of output: 33696
Script:
#!/bin/bash
# Let's check the bit-css-variables file specifically
echo "Checking bit-css-variables.scss files:"
fd "bit-css-variables.scss"
# Let's examine the content of these files
echo -e "\nChecking content of bit-css-variables files:"
fd "bit-css-variables.scss" -x cat {}
Length of output: 41831
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/compilerconfig.json (1)
33-38: LGTM! Configuration for MainLayout follows project conventions.
The new configuration block for MainLayout is consistent with other component configurations in terms of structure and settings.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/NavBar.razor (1)
13-13: Reintroduction of "Products" menu option looks good.
The code reintroduces a navigation item for "Products." Ensure that this reflects the intended business logic, especially if its visibility was previously controlled by a conditional or feature flag.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/NavBar.razor.scss (2)
10-10: Use of $bit-env-inset-bottom is consistent with SCSS-based variables.
No issues found. The SCSS variable usage is aligned with the new styling approach.
12-12: Updated max-width to $bit-env-width-available for responsive design.
Switching from custom properties to SCSS variables here is a solid move towards consistent styling.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/UserMenu.razor.scss (2)
2-2: Imported _bit-css-variables.scss.
This import aligns with the new SCSS variable strategy used throughout the application.
11-13: Switched to SCSS inset variables for user menu callout.
Transitioning to $bit-env-inset-top, $bit-env-inset-bottom, and $bit-env-inset-inline-start is consistent with the application’s updated layout scheme and simplifies global style management.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/DiagnosticModal.razor.scss (4)
2-2: Imported _bit-css-variables.scss for DiagnosticModal.
This promotes consistency in using SCSS variables for layout management.
12-15: Replaced top, left, right, bottom with $bit-env-inset-... variables.
This shift away from CSS custom properties improves maintainability and aligns with the project's SCSS-based approach.
30-30: Updated .go-top-button to use $bit-env-inset-bottom in calc().
This maintains consistent spacing across devices and aligns with the global layout variables.
38-39: Refined modal dimensions to rely on $bit-env-height-available and $bit-env-width-available.
Using SCSS variables for height and width calculations promotes a robust and responsive design.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Todo/TodoPage.razor.scss (2)
45-46: LGTM: Proper migration to SCSS environment variables
The inset variables have been correctly migrated from CSS custom properties to the new SCSS variables.
53-53: Verify height calculations across breakpoints
The height calculations follow a consistent pattern but use different offsets:
- Default: -14rem
- lt-md: -17rem
- lt-sm: -18rem
Please verify if these increasing offsets (-14rem → -17rem → -18rem) for smaller screens are intentional, as typically mobile layouts might need more available height.
Also applies to: 56-56, 60-60
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Styles/app.scss (2)
2-2: LGTM: Added centralized SCSS variables import
Properly imported the new SCSS variables file that centralizes the environment variables.
54-59: LGTM: Complete migration of modal positioning
All modal positioning and dimension properties have been correctly migrated to use the new SCSS environment variables.
Run the following to verify modal positioning variables are consistently used across all modal components:
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Categories/CategoriesPage.razor.scss (1)
10-10: Review height calculation consistency across components
While the migration to SCSS variables is correct, there's an inconsistency in responsive height calculations between components:
- CategoriesPage:
- Default: -14rem
- lt-md: -17rem
- lt-sm: -16rem
- TodoPage:
- Default: -14rem
- lt-md: -17rem
- lt-sm: -18rem
The lt-sm breakpoint uses different offsets (-16rem vs -18rem). This might lead to inconsistent layouts across pages on small screens.
Run the following to check height calculations across all page components:
Consider standardizing the height calculations across similar components for consistent layouts.
Also applies to: 13-13, 17-17
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor (2)
3-9: Consider validating and null-checking cascaded parameters in <BitAppShell>
While the new <BitAppShell> usage appears appropriate, ensure that any critical parameters (e.g., currentRouteData) are checked for null values if your app logic requires them. This helps avoid runtime errors when they are consumed downstream.
45-45: No issues with closing tag
The closing </BitAppShell> tag aligns well with the opened tag, and no code changes are necessary here.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Routes.razor (1)
4-4: Confirm that all references to RootLayout are replaced
Switching the layout to MainLayout is correct. Verify that there are no remaining references to RootLayout in the code or configuration to avoid confusion or build-time warnings.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/MainLayout.razor.items.cs (2)
1-2: Namespace and top-level structure
Using a separate .items.cs partial file for MainLayout is a good approach for organizing logic. No issues found.
3-6: Leverage partial class for layout
Declaring public partial class MainLayout and injecting IStringLocalizer<AppStrings> fosters clean code separation. Ensure that the resource keys like nameof(AppStrings.Home) exist in your .resx files.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.cs (4)
6-6: Use of IAsyncDisposable is correct
Implementing IAsyncDisposable for cleanup is good practice and is properly paired with DisposeAsync.
31-31: Injections look consistent
BitExtraServices is properly injected. Ensure AddRootCssClasses() usage is fully tested to avoid unexpected UI changes.
46-47: Explicit initialization for nav panel items
Calling InitializeNavPanelItems() in OnInitializedAsync is a good way to ensure menu items are set before rendering.
50-63: PubSub subscriptions and unsubscribers
Nice approach to manage unsubscribers for each call. This practice ensures a clean teardown, preventing memory leaks and extraneous event handling.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.scss (1)
10-10: Verify $bit-env-height-available usage
Switching to $bit-env-height-available can improve responsiveness. Confirm that this variable is properly defined in _bit-css-variables.scss and tested on various devices.
...t.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/IdentityHeader.razor.scss
Show resolved
Hide resolved
...e/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MessageBox.razor.scss
Show resolved
Hide resolved
...Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/compilerconfig.json
Show resolved
Hide resolved
1 participant
closes #9565
Summary by CodeRabbit
Based on the comprehensive summary, here are the release notes:
New Features
Styling Updates
Layout Changes
RootLayoutwithMainLayoutBitAppShellandBitNavPanelcomponentsPerformance
Chores