-
Notifications
You must be signed in to change notification settings - Fork 308
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
doc(Auto): update Auto mode #4785
Conversation
# Conflicts: # src/BootstrapBlazor.Server/Extensions/ServiceCollectionExtensions.cs # src/BootstrapBlazor.Shared/BootstrapBlazor.Shared.csproj # src/BootstrapBlazor.Shared/Extensions/ServicesCollectionExtensions.cs
Reviewer's Guide by SourceryThis PR introduces a new BootstrapBlazorAuto project with both client and server components, while refactoring the middleware configuration and service registration. The changes focus on improving the architecture by consolidating services, updating culture handling, and streamlining the middleware pipeline. Class diagram for updated service registrationclassDiagram
class ServicesCollectionExtensions {
+AddBootstrapBlazorServices()
+AddOptionsMonitor()
+AddCascadingAuthenticationState()
+AddBootstrapBlazorMeiliSearch()
}
class IServiceCollection {
<<interface>>
}
ServicesCollectionExtensions --> IServiceCollection
class WeatherForecastService
class PackageVersionService
class CodeSnippetService
class DashboardService
class IDataService~T~
class TableDemoDataService~T~
class ILookupService
class DemoLookupService
class MockDataTableDynamicService
class MenuService
class FanControllerDataService
class AuthenticationStateProvider
class MockAuthenticationStateProvider
IServiceCollection <|-- WeatherForecastService
IServiceCollection <|-- PackageVersionService
IServiceCollection <|-- CodeSnippetService
IServiceCollection <|-- DashboardService
IServiceCollection <|-- IDataService
IServiceCollection <|-- ILookupService
IServiceCollection <|-- MockDataTableDynamicService
IServiceCollection <|-- MenuService
IServiceCollection <|-- FanControllerDataService
IServiceCollection <|-- AuthenticationStateProvider
AuthenticationStateProvider <|-- MockAuthenticationStateProvider
Class diagram for CultureChooser componentclassDiagram
class CultureChooser {
-IJSRuntime JSRuntime
-string SelectedCulture
-string Label
+SetCulture(SelectedItem item)
+GetDisplayName(CultureInfo culture)
}
class IJSRuntime
class SelectedItem
class CultureInfo
CultureChooser --> IJSRuntime
CultureChooser --> SelectedItem
CultureChooser --> CultureInfo
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @ArgoZhang - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Consider setting a reasonable maximum size limit for SignalR messages (link)
Overall Comments:
- Consider using the minified version of Bootstrap CSS (bootstrap.min.css) instead of the full version to reduce the download size for clients.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🔴 Security: 1 blocking issue
- 🟢 Testing: all looks good
- 🟡 Complexity: 2 issues found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/BootstrapBlazorAuto/BootstrapBlazorAuto/Services/ClearUploadFilesService.cs
Outdated
Show resolved
Hide resolved
src/BootstrapBlazorAuto/BootstrapBlazorAuto/Services/ClearUploadFilesService.cs
Outdated
Show resolved
Hide resolved
src/BootstrapBlazorAuto/BootstrapBlazorAuto/Extensions/ServiceCollectionExtensions.cs
Outdated
Show resolved
Hide resolved
src/BootstrapBlazorAuto/BootstrapBlazorAuto/Controllers/Api/GiteeController.cs
Outdated
Show resolved
Hide resolved
src/BootstrapBlazorAuto/BootstrapBlazorAuto/Services/ClearUploadFilesService.cs
Outdated
Show resolved
Hide resolved
src/BootstrapBlazorAuto/BootstrapBlazorAuto/wwwroot/lib/bootstrap/dist/js/bootstrap.bundle.js
Outdated
Show resolved
Hide resolved
src/BootstrapBlazorAuto/BootstrapBlazorAuto/wwwroot/lib/bootstrap/dist/js/bootstrap.bundle.js
Outdated
Show resolved
Hide resolved
src/BootstrapBlazorAuto/BootstrapBlazorAuto/wwwroot/lib/bootstrap/dist/js/bootstrap.bundle.js
Outdated
Show resolved
Hide resolved
src/BootstrapBlazorAuto/BootstrapBlazorAuto/wwwroot/lib/bootstrap/dist/js/bootstrap.bundle.js
Outdated
Show resolved
Hide resolved
src/BootstrapBlazorAuto/BootstrapBlazorAuto/wwwroot/lib/bootstrap/dist/js/bootstrap.bundle.js
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4785 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 620 620
Lines 27424 27424
Branches 3925 3925
=========================================
Hits 27424 27424 ☔ View full report in Codecov by Sentry. |
@sourcery-ai review |
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.
Hey @ArgoZhang - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider keeping the more specific namespaces (e.g. BootstrapBlazor.Server.Controllers) to avoid potential conflicts
- Some previously configurable values are now hardcoded (e.g. HubOptions.MaximumReceiveMessageSize). Consider keeping these configurable through options
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
update Auto mode
Summary of the changes (Less than 80 chars)
简单描述你更改了什么, 不超过80个字符;如果有关联 Issue 请在下方填写相关编号
Description
fixes #4784
Regression?
[If yes, specify the version the behavior has regressed from]
[是否影响老版本]
Risk
[Justify the selection above]
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Refactor the service registration process and update the culture management logic in the CultureChooser component. Introduce a new project structure for BootstrapBlazorAuto with interactive server and web assembly components. Simplify configuration by removing unused options and update documentation for Auto mode.
New Features:
Enhancements:
Build:
Documentation: