-
-
Notifications
You must be signed in to change notification settings - Fork 226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Boilerplate for latest BlazorUI changes (#9318) #9320
Update Boilerplate for latest BlazorUI changes (#9318) #9320
Conversation
WalkthroughThe pull request introduces significant updates to several components in the Boilerplate project, primarily focusing on enhancing the navigation and pagination features. The 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 (
|
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: 0
🧹 Outside diff range and nitpick comments (4)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/NavBar.razor (1)
10-14
: LGTM! Admin section navigation options are consistently implemented.The admin navigation options follow a consistent pattern and are properly organized. Consider adding a comment explaining the purpose of each admin section for better maintainability.
@*#if (sample == "Admin")*@ +@* Admin section provides navigation to core administrative features *@ <BitNavBarOption Text="@Localizer[nameof(AppStrings.Dashboard)]" IconName="@BitIconName.BarChartVerticalFill" Url="@Urls.DashboardPage" /> <BitNavBarOption Text="@Localizer[nameof(AppStrings.Products)]" IconName="@BitIconName.Product" Url="@Urls.ProductsPage" /> <BitNavBarOption Text="@Localizer[nameof(AppStrings.Categories)]" IconName="@BitIconName.BuildQueue" Url="@Urls.CategoriesPage" />
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/NavPanel.razor (1)
Line range hint
66-72
: LGTM! Consider adding ARIA attributes for better accessibility.The addition of
Color="BitColor.Error"
appropriately highlights the sign-out as a destructive action, consistent with modern UI/UX practices and the changes in UserMenu.razor.Consider adding
aria-label
for better screen reader support:<BitActionButton IconOnly="isToggled" Color="BitColor.Error" FullWidth="!isToggled" OnClick="ShowSignOutConfirm" IconName="@BitIconName.SignOut" + aria-label="@Localizer[nameof(AppStrings.SignOut)]" Title="@Localizer[nameof(AppStrings.SignOut)]">
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/UserMenu.razor (1)
39-41
: Consider using a URL builder or constants for route construction.While the implementation is functionally correct, the URL construction using string concatenation (
$"{Urls.SettingsPage}/{Urls.SettingsSections.Profile}"
) could be fragile if route patterns change.Consider creating a dedicated method in the
Urls
class:-Href="@($"{Urls.SettingsPage}/{Urls.SettingsSections.Profile}")" +Href="@Urls.GetProfileSettingsUrl()"src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Extensions/IClientCoreServiceCollectionExtensions.cs (1)
Line range hint
47-53
: Consider aligning with existing service registration patternThe current registration of
AuthenticationStateProvider
deviates from the pattern used for other sessioned services. Consider refactoring to use the more conciseAddSessioned<TService, TImplementation>
pattern:-services.AddSessioned<AuthenticationStateProvider>(sp => -{ - var authenticationStateProvider = ActivatorUtilities.CreateInstance<AuthenticationManager>(sp); - authenticationStateProvider.OnInit(); - return authenticationStateProvider; -}); -services.AddSessioned(sp => (AuthenticationManager)sp.GetRequiredService<AuthenticationStateProvider>()); +services.AddSessioned<AuthenticationStateProvider, AuthenticationManager>();This change would:
- Maintain consistency with other service registrations
- Leverage the built-in dependency injection lifecycle management
- Potentially move the
OnInit
call to a more appropriate lifecycle event
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
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/NavPanel.razor
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/UserMenu.razor
(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Categories/CategoriesPage.razor
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Products/ProductsPage.razor
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Extensions/IClientCoreServiceCollectionExtensions.cs
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fa.resx
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.nl.resx
(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.resx
(1 hunks)
🔇 Additional comments (18)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/NavBar.razor (4)
5-5
: LGTM! BitNavBar implementation is clean and properly structured.
The BitNavBar component is correctly configured with the appropriate generic type and styling.
Also applies to: 16-16
6-6
: LGTM! Home navigation option is well-implemented.
The BitNavBarOption correctly uses localization and proper icon naming conventions.
15-15
: LGTM! Terms navigation option is properly implemented.
The Terms navigation option follows the established pattern and is correctly positioned outside conditional blocks for consistent visibility.
7-9
: Verify the conditional rendering mechanism for the Todo section.
The conditional rendering uses special comments (@*#if (sample == "Todo")*@
) which appears to be part of a template processing system. Please ensure this mechanism is properly documented and tested.
✅ Verification successful
Let me gather more context about this template processing mechanism.
Let me check for .NET template-specific files and documentation to understand this better.
Let me check the README files to understand the template structure and configuration.
The conditional comments are part of the .NET template system and are working as intended
The @*#if (sample == "Todo")*@
syntax is a standard .NET template conditional comment used in Razor files. This is part of the Bit Platform's template system, as evidenced by:
- Multiple files using the same pattern consistently (NavBar.razor, _Imports.razor)
- The project being part of a template structure (under src/Templates/Boilerplate/)
- Documentation reference to templates at bitplatform.dev/templates
- Similar conditional imports in other parts of the codebase for different features (e.g., Admin sample)
The conditional comments are processed during template instantiation and are not part of the runtime Razor syntax, which explains their non-standard appearance.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for documentation or similar usage of this conditional comment pattern
rg -l '#if \(sample ==' --type-add 'template:*.{razor,cshtml}' -t template
# Look for the template processing implementation
rg -l 'sample.*=.*"Todo"' --type=cs
Length of output: 957
Script:
#!/bin/bash
# Check the content of _Imports.razor to understand the pattern usage
cat "src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/_Imports.razor"
# Look for any template configuration or documentation
rg -l "template.json|.template.config" --type-add 'config:*.{json,config}' -t config
# If found, examine the template configuration
fd template.json --exec cat {}
Length of output: 384
Script:
#!/bin/bash
# Look for .template.config directory which is standard for .NET templates
fd -t d .template.config
# Check for any README or documentation files in the template directory
fd -t f "README|CONTRIBUTING|DOCUMENTATION" -e md -e txt src/Templates/Boilerplate/Bit.Boilerplate/
# Look for the template manifest
fd ".template.config" -t d -x ls -la {}
Length of output: 314
Script:
#!/bin/bash
# Check the main README content
cat "src/Templates/Boilerplate/Bit.Boilerplate/README.md"
# Check if there are any .NET template configuration files in standard locations
fd -e json -e config -p ".template.config"
# Look for any template-related content in the solution
rg -A 5 -B 5 "template" "src/Templates/Boilerplate/Bit.Boilerplate/README.md"
Length of output: 1117
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Categories/CategoriesPage.razor (2)
69-70
: LGTM! Well-structured localization implementation.
The paginator implementation properly utilizes the Localizer for both the summary format and page number display, maintaining consistency with the application's localization pattern.
Also applies to: 75-77
71-74
:
Use Localizer for navigation button titles.
The navigation button titles are currently hardcoded in English, which is inconsistent with the application's localization pattern. These should use the Localizer to maintain consistency with the rest of the component.
Apply this diff to fix the localization:
- GoToFirstButtonTitle="Go to first page"
- GoToPrevButtonTitle="Go to previous page"
- GoToNextButtonTitle="Go to next page"
- GoToLastButtonTitle="Go to last page">
+ GoToFirstButtonTitle="@Localizer[nameof(AppStrings.GoToFirstPage)]"
+ GoToPrevButtonTitle="@Localizer[nameof(AppStrings.GoToPrevPage)]"
+ GoToNextButtonTitle="@Localizer[nameof(AppStrings.GoToNextPage)]"
+ GoToLastButtonTitle="@Localizer[nameof(AppStrings.GoToLastPage)]">
Let's verify if these localization keys exist:
✅ Verification successful
Based on the resource files found earlier, I can see that the localization keys for pagination navigation buttons already exist in the AppStrings.resx files:
PaginatorFirstButtonTitle
: "Go to the first page"PaginatorPrevButtonTitle
: "Go to the previous page"PaginatorNextButtonTitle
: "Go to the next page"PaginatorLastButtonTitle
: "Go to the last page"
The review comment is correct. Use the existing localization keys for navigation buttons.
The suggested fix should be updated to use the correct keys:
- GoToFirstButtonTitle="Go to first page"
- GoToPrevButtonTitle="Go to previous page"
- GoToNextButtonTitle="Go to next page"
- GoToLastButtonTitle="Go to last page">
+ GoToFirstButtonTitle="@Localizer[nameof(AppStrings.PaginatorFirstButtonTitle)]"
+ GoToPrevButtonTitle="@Localizer[nameof(AppStrings.PaginatorPrevButtonTitle)]"
+ GoToNextButtonTitle="@Localizer[nameof(AppStrings.PaginatorNextButtonTitle)]"
+ GoToLastButtonTitle="@Localizer[nameof(AppStrings.PaginatorLastButtonTitle)]">
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the localization keys for navigation buttons exist in the resource files
# Test: Search for the localization keys in the resource files
rg "GoToFirstPage|GoToPrevPage|GoToNextPage|GoToLastPage" --type resx
Length of output: 100
Script:
#!/bin/bash
# Let's try to find resource files with a different approach
fd "\.resx$|AppStrings\." --type f
# Then let's check the content of these files
fd "\.resx$|AppStrings\." --type f --exec cat {}
Length of output: 106616
Script:
#!/bin/bash
# Let's check if there are any existing localization keys for pagination in AppStrings.cs
fd "AppStrings\.cs$" --type f --exec cat {}
Length of output: 47
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Authorized/Products/ProductsPage.razor (2)
81-90
: LGTM! Well-structured pagination integration
The BitDataGridPaginator is well-integrated with the BitDataGrid component and maintains a clean, accessible structure. The custom TextTemplate provides clear pagination information to users.
81-90
: 🛠️ Refactor suggestion
Localize navigation button titles for consistency
The navigation button titles are currently hardcoded in English, while other text elements use the Localizer. This should be updated for consistency and proper localization support.
Apply this diff to fix the localization:
<BitDataGridPaginator Value="pagination"
SummaryFormat="v => string.Format(Localizer[nameof(AppStrings.PaginatorSummary)], v.TotalItemCount)"
- GoToFirstButtonTitle="Go to first page"
- GoToPrevButtonTitle="Go to previous page"
- GoToNextButtonTitle="Go to next page"
- GoToLastButtonTitle="Go to last page">
+ GoToFirstButtonTitle="@Localizer[nameof(AppStrings.GoToFirstPage)]"
+ GoToPrevButtonTitle="@Localizer[nameof(AppStrings.GoToPrevPage)]"
+ GoToNextButtonTitle="@Localizer[nameof(AppStrings.GoToNextPage)]"
+ GoToLastButtonTitle="@Localizer[nameof(AppStrings.GoToLastPage)]">
Let's verify if these localization keys exist:
✅ Verification successful
Use existing localization keys for pagination buttons
The hardcoded button titles should use the existing localization keys found in AppStrings.resx. Apply this diff:
<BitDataGridPaginator Value="pagination"
SummaryFormat="v => string.Format(Localizer[nameof(AppStrings.PaginatorSummary)], v.TotalItemCount)"
- GoToFirstButtonTitle="Go to first page"
- GoToPrevButtonTitle="Go to previous page"
- GoToNextButtonTitle="Go to next page"
- GoToLastButtonTitle="Go to last page">
+ GoToFirstButtonTitle="@Localizer[nameof(AppStrings.PaginatorFirstButtonTitle)]"
+ GoToPrevButtonTitle="@Localizer[nameof(AppStrings.PaginatorPrevButtonTitle)]"
+ GoToNextButtonTitle="@Localizer[nameof(AppStrings.PaginatorNextButtonTitle)]"
+ GoToLastButtonTitle="@Localizer[nameof(AppStrings.PaginatorLastButtonTitle)]">
The resource files (AppStrings.resx) already contain the correct localization keys with appropriate translations:
- PaginatorFirstButtonTitle: "Go to the first page"
- PaginatorPrevButtonTitle: "Go to the previous page"
- PaginatorNextButtonTitle: "Go to the next page"
- PaginatorLastButtonTitle: "Go to the last page"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the localization keys are defined in the resource files
rg -l "GoToFirstPage|GoToPrevPage|GoToNextPage|GoToLastPage" --type resx
Length of output: 103
Script:
#!/bin/bash
# Let's first find the resource files that might contain these strings
fd "AppStrings" --type f
# Then let's look for any existing pagination-related strings in these files
rg -i "page|paginator" $(fd "AppStrings" --type f)
Length of output: 9475
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/UserMenu.razor (1)
62-65
: LGTM! Good UX improvement.
The addition of Color="BitColor.Error"
improves visual hierarchy by distinctly highlighting the sign-out action. This change also maintains consistency with similar updates in NavPanel.razor.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Extensions/IClientCoreServiceCollectionExtensions.cs (2)
Line range hint 47-53
: Verify the initialization timing of AuthenticationManager
The changes introduce explicit initialization of AuthenticationManager
through OnInit()
. While this ensures proper initialization, consider these aspects:
- The initialization happens during service registration, which might be too early in the application lifecycle.
- The separate registration of
AuthenticationManager
as a resolved service might lead to multiple instances.
Let's verify the usage and lifecycle of AuthenticationManager:
Line range hint 47-53
: Verify integration with updated UI components
Given that this PR updates BitNavBar and other UI components, ensure that the authentication state changes are properly reflected in the navigation components.
Let's verify the integration:
✅ Verification successful
Based on the search results, I can see that the BitNavBar component is a UI navigation component that:
- Supports multiple modes of operation (Manual, IconOnly)
- Handles authentication-based navigation through BitNavBarOption components
- Is already integrated with authentication state in the boilerplate template (NavBar.razor shows authenticated routes)
Let me verify if there are any authentication-specific handlers in the navigation component:
Let me check the RootLayout.razor.cs to understand how authentication state is handled with navigation:
Based on the code analysis, I can now provide a final response:
Authentication integration with navigation components is properly handled
The authentication state changes are already properly integrated with the navigation system through:
- RootLayout handles authentication state changes via
AuthenticationStateChanged
event and updates UI accordingly - The Routes.razor uses
AuthorizeRouteView
to handle authenticated/unauthenticated states - The BitNavBar component in NavBar.razor is wrapped in proper layout structure that responds to authentication state
The changes to authentication service registration will work seamlessly with the existing navigation components as they rely on the standard AuthenticationStateProvider contract rather than specific implementation details.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for BitNavBar usage to ensure it properly handles authentication state
rg "BitNavBar" -A 5
# Search for authentication state usage in navigation components
ast-grep --pattern 'AuthenticationStateProvider.GetAuthenticationStateAsync($$$)'
Length of output: 107502
Script:
#!/bin/bash
# Search for authentication-related code in navigation components
rg -l "AuthenticationStateProvider|AuthenticationState" src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/
# Check if navigation is tied to authentication state
rg "OnNavigateAsync|NavigationManager" src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/
Length of output: 1598
Script:
#!/bin/bash
# Check RootLayout's authentication and navigation handling
cat src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/RootLayout.razor.cs
# Check if there are any authentication-specific route handlers
rg -A 5 "AuthorizeRouteView|AuthorizeView" src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/
Length of output: 9169
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fa.resx (2)
1037-1056
: LGTM! The pagination localization strings are well structured.
The new Persian translations for the BitDataGridPaginator component are complete and follow consistent formatting. The translations cover all necessary UI elements:
- Summary format for total items
- Navigation button titles (First, Previous, Next, Last)
- Page number display text
1037-1056
: Verify consistency across language resource files.
Let's ensure these pagination strings are consistently added across all language files.
✅ Verification successful
Pagination strings are consistently present across all language resource files
The verification confirms that all pagination-related strings (PaginatorSummary
, PaginatorFirstButtonTitle
, PaginatorPrevButtonTitle
, PaginatorNextButtonTitle
, PaginatorLastButtonTitle
, Page
, Of
) are present in all three language resource files:
- AppStrings.resx (default)
- AppStrings.nl.resx (Dutch)
- AppStrings.fa.resx (Persian)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if pagination strings exist in all language resource files
# Expected: All language files should contain the same pagination string keys
# Find all AppStrings resource files
echo "Checking resource files for pagination strings..."
for key in "PaginatorSummary" "PaginatorFirstButtonTitle" "PaginatorPrevButtonTitle" "PaginatorNextButtonTitle" "PaginatorLastButtonTitle" "Page" "Of"; do
echo "Checking key: $key"
rg -l "<data name=\"$key\" xml:space=\"preserve\">" src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/
done
Length of output: 4280
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.resx (4)
1037-1039
: LGTM: Paginator summary string is properly formatted.
The paginator summary string uses the correct placeholder format {0}
for displaying the number of items.
1040-1051
: LGTM: Navigation button titles are clear and consistent.
The paginator navigation button titles are well-defined and follow a consistent pattern:
- First page
- Previous page
- Next page
- Last page
Each title clearly describes the action it performs.
1052-1056
: LGTM: Page number display strings are properly defined.
The strings for displaying current page number and total pages are appropriately defined:
- "Page" label
- "of" separator
These strings will enable proper localization of the page number display format: "Page X of Y"
1037-1056
: Verify localization completeness across all language files.
Please ensure that these new pagination strings are added to all language-specific resource files (e.g., AppStrings.fa.resx, AppStrings.nl.resx) to maintain consistent localization support.
✅ Verification successful
All pagination strings are properly localized
The verification shows that all the new pagination strings (PaginatorSummary
, PaginatorFirstButtonTitle
, PaginatorPrevButtonTitle
, PaginatorNextButtonTitle
, PaginatorLastButtonTitle
, Page
, Of
) are already present and properly translated in both language-specific resource files:
- AppStrings.nl.resx (Dutch)
- AppStrings.fa.resx (Persian)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the new pagination strings exist in all language-specific resource files
# Find all language-specific resource files
fd "AppStrings\..*\.resx$" | while read -r file; do
echo "Checking $file for pagination strings..."
# Check for the presence of pagination-related strings
grep -A 1 "PaginatorSummary\|PaginatorFirstButtonTitle\|PaginatorPrevButtonTitle\|PaginatorNextButtonTitle\|PaginatorLastButtonTitle\|Page\|Of" "$file"
done
Length of output: 5470
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.nl.resx (1)
1037-1056
: LGTM! The paginator translations are complete and accurate.
The new Dutch translations for the paginator component are well-structured and include all necessary strings:
- Summary format for displaying total items
- Navigation button titles (First, Previous, Next, Last pages)
- Page number labels
The translations maintain consistency with the application's localization patterns and properly support the BitDataGridPaginator component updates mentioned in the PR objectives.
closes #9318
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Localization