chore(Vote): remove gitee vote #7489
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideRefactors theme initialization in BaseLayout and extracts the Gitee voting toast into a dedicated GlobalToast component with its own JS interop and throttling logic, while simplifying BaseLayout’s lifecycle and disposal responsibilities. Sequence diagram for GlobalToast initialization and toast displaysequenceDiagram
actor User
participant Browser
participant GlobalToast
participant GlobalToastJS as GlobalToast_js_module
participant ToastService
User->>Browser: Load page
Browser->>GlobalToast: Render component
GlobalToast->>GlobalToast: InvokeInitAsync()
GlobalToast->>GlobalToastJS: init(Interop, ShowToast, debug)
Note over GlobalToastJS: Initialization in JS
GlobalToastJS->>GlobalToastJS: Read localStorage bb-g-toast
alt value exists and < 24h
GlobalToastJS-->>GlobalToastJS: Return without toast
else value missing or expired
GlobalToastJS->>GlobalToastJS: setTimeout(10s)
Browser-->>GlobalToastJS: Timer fires
GlobalToastJS->>GlobalToast: invokeMethodAsync(ShowToast)
GlobalToast->>GlobalToast: ShowToast()
GlobalToast->>GlobalToast: Check CultureInfo.CurrentUICulture
alt culture is en-US
GlobalToast-->>GlobalToastJS: Do nothing
else non en-US
GlobalToast->>ToastService: Show(ToastOption with RenderVote)
ToastService-->>User: Display toast UI
end
end
User->>Browser: Click inside toast #bb-g-toast
Browser->>GlobalToastJS: EventHandler click handler
GlobalToastJS->>Browser: Hide toast
GlobalToastJS->>GlobalToastJS: localStorage set bb-g-toast = now
Class diagram for updated BaseLayout and new GlobalToast componentclassDiagram
class BaseLayout {
+WebsiteOption WebsiteOption
+ToastService Toast
+CommitDispatchService CommitDispatchService
+RebootDispatchService RebootDispatchService
-string~?~ Title
-string~?~ CancelText
-bool _init
+void OnInitialized()
+Task OnInitializedAsync()
-Task NotifyCommit(DispatchEntry~GiteePostBody~ payload)
-Task NotifyReboot(DispatchEntry~bool~ payload)
-void Dispose(bool disposing)
+void Dispose()
}
class GlobalToast {
+ToastService Toast
+Task InvokeInitAsync()
+Task ShowToast()
}
class WebSiteModuleComponentBase {
+object Interop
+Task InvokeVoidAsync(string identifier, object arg1, string arg2)
+Task InvokeInitAsync()
}
class ToastService {
+Task Show(ToastOption option)
}
class ToastOption {
+ToastCategory Category
+string Title
+bool IsAutoHide
+RenderFragment ChildContent
+bool PreventDuplicates
}
class JSModuleAutoLoader {
}
BaseLayout ..> ToastService : uses
GlobalToast ..> ToastService : uses
GlobalToast --|> WebSiteModuleComponentBase
GlobalToast ..> JSModuleAutoLoader : attribute
GlobalToast ..> ToastOption : creates
Flow diagram for GlobalToast JavaScript throttling and click handlingflowchart TD
A[init invoked
with GlobalToast_js_module] --> B{localStorage
has bb-g-toast?}
B -->|yes| C[Compute time differ]
B -->|no| E[Proceed to schedule toast]
C --> D{"differ < 86400000 (24 hours)?"}
D -->|yes| F[Return without toast]
D -->|no| E
E --> G[Set timeout 10s]
G --> H[Invoke .NET
ShowToast method]
H --> I[GlobalToast.ShowToast
checks culture]
I --> J[ToastService.Show
renders toast]
J --> K[User clicks element
with id bb-g-toast]
K --> L[Hide parent .toast element]
L --> M[localStorage
set bb-g-toast = now]
M --> F
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
BaseLayout, you now callJSRuntime.LoadModuleinOnInitializedAsyncwithout holding onto or disposing the module; consider either using the existingJSModuleAutoLoaderpattern or keeping a reference and disposing it to avoid leaking JS object references. GlobalToast.InvokeInitAsynccalls the JSinitfunction without passing adebugflag, but the JS implementation relies ondebug !== trueto schedule the toast; if you want to preserve the previous behavior of disabling the toast in development, pass through the environment flag (e.g.,WebsiteOption.Value.IsDevelopment).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `BaseLayout`, you now call `JSRuntime.LoadModule` in `OnInitializedAsync` without holding onto or disposing the module; consider either using the existing `JSModuleAutoLoader` pattern or keeping a reference and disposing it to avoid leaking JS object references.
- `GlobalToast.InvokeInitAsync` calls the JS `init` function without passing a `debug` flag, but the JS implementation relies on `debug !== true` to schedule the toast; if you want to preserve the previous behavior of disabling the toast in development, pass through the environment flag (e.g., `WebsiteOption.Value.IsDevelopment`).
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor.Server/Components/Layout/BaseLayout.razor.cs:74-83` </location>
<code_context>
/// </summary>
/// <returns></returns>
- protected override async Task OnAfterRenderAsync(bool firstRender)
+ protected override async Task OnInitializedAsync()
{
- await base.OnAfterRenderAsync(firstRender);
+ await base.OnInitializedAsync();
- if (firstRender)
- {
- _module = await JSRuntime.LoadModule($"{WebsiteOption.Value.JSModuleRootPath}Layout/BaseLayout.razor.js");
- _interop = DotNetObjectReference.Create(this);
- await _module.InvokeVoidAsync("doTask", _interop, WebsiteOption.Value.IsDevelopment);
- _init = true;
- StateHasChanged();
- }
</code_context>
<issue_to_address>
**issue (bug_risk):** Using JS interop in OnInitializedAsync may fail because JS is not guaranteed to be available before first render.
Previously the module load ran in OnAfterRenderAsync with a firstRender guard, which matches Blazor’s guidance that JS interop is only reliable after the first render. Moving JSRuntime.LoadModule and module.InvokeVoidAsync("initTheme") into OnInitializedAsync risks runtime errors (especially on Blazor Server) because JS may not be ready. Please move this initialization back to OnAfterRender[Async] or another post-first-render lifecycle method while keeping the simplified initTheme call.
</issue_to_address>
### Comment 2
<location> `src/BootstrapBlazor.Server/Components/Components/GlobalToast.razor.cs:25` </location>
<code_context>
+ /// <inheritdoc/>
+ /// </summary>
+ /// <returns></returns>
+ protected override Task InvokeInitAsync() => InvokeVoidAsync("init", Interop, nameof(ShowToast));
+
+ /// <summary>
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The JS init function defines a `debug` parameter that is never set, so the debug behavior cannot be controlled.
`GlobalToast.razor.js` exports `init(invoke, method, debug)` and uses `if (debug !== true)` to decide whether to schedule the toast. `InvokeInitAsync` only passes `Interop` and the method name, so `debug` is always `undefined` and the toast is always scheduled. Either pass an `IsDevelopment`/similar flag as the third argument, or remove the `debug` parameter and related branching in JS to avoid dead configuration.
Suggested implementation:
```csharp
public partial class GlobalToast
{
[Inject]
[NotNull]
private ToastService? Toast { get; set; }
[Inject]
private IWebHostEnvironment? HostEnvironment { get; set; }
/// <summary>
```
```csharp
/// <inheritdoc/>
protected override Task InvokeInitAsync() =>
InvokeVoidAsync("init", Interop, nameof(ShowToast), HostEnvironment?.IsDevelopment() == true);
```
1. Ensure the file has `using Microsoft.AspNetCore.Hosting;` or `using Microsoft.Extensions.Hosting;` at the top, depending on which hosting environment type your project already uses. If the project uses `IHostEnvironment` instead of `IWebHostEnvironment`, change the injected type accordingly and keep using `.IsDevelopment()`.
2. No changes are required in `GlobalToast.razor.js` if its exported `init` signature remains `init(invoke, method, debug)` and it already branches on `debug` using `if (debug !== true) { ... }`. With these changes, `debug` will be `true` in development and `undefined`/`false` otherwise.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| /// <inheritdoc/> | ||
| /// </summary> | ||
| /// <returns></returns> | ||
| protected override Task InvokeInitAsync() => InvokeVoidAsync("init", Interop, nameof(ShowToast)); |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): The JS init function defines a debug parameter that is never set, so the debug behavior cannot be controlled.
GlobalToast.razor.js exports init(invoke, method, debug) and uses if (debug !== true) to decide whether to schedule the toast. InvokeInitAsync only passes Interop and the method name, so debug is always undefined and the toast is always scheduled. Either pass an IsDevelopment/similar flag as the third argument, or remove the debug parameter and related branching in JS to avoid dead configuration.
Suggested implementation:
public partial class GlobalToast
{
[Inject]
[NotNull]
private ToastService? Toast { get; set; }
[Inject]
private IWebHostEnvironment? HostEnvironment { get; set; }
/// <summary> /// <inheritdoc/>
protected override Task InvokeInitAsync() =>
InvokeVoidAsync("init", Interop, nameof(ShowToast), HostEnvironment?.IsDevelopment() == true);- Ensure the file has
using Microsoft.AspNetCore.Hosting;orusing Microsoft.Extensions.Hosting;at the top, depending on which hosting environment type your project already uses. If the project usesIHostEnvironmentinstead ofIWebHostEnvironment, change the injected type accordingly and keep using.IsDevelopment(). - No changes are required in
GlobalToast.razor.jsif its exportedinitsignature remainsinit(invoke, method, debug)and it already branches ondebugusingif (debug !== true) { ... }. With these changes,debugwill betruein development andundefined/falseotherwise.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7489 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 749 749
Lines 32943 32943
Branches 4577 4577
=========================================
Hits 32943 32943
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request removes the Gitee vote functionality from the BaseLayout component and refactors it into a new standalone GlobalToast component. However, the refactoring is incomplete and introduces several critical issues.
Changes:
- Removed vote-related JavaScript code (doTask, dispose functions) from BaseLayout.razor.js and exported initTheme function
- Simplified BaseLayout.razor.cs by removing vote toast logic, changing from IAsyncDisposable to IDisposable, and moving initialization from OnAfterRenderAsync to OnInitializedAsync
- Created new GlobalToast component (razor, cs, and js files) with the extracted vote functionality
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BootstrapBlazor.Server/Components/Layout/BaseLayout.razor.js | Removed vote-related functions and EventHandler import; exported initTheme function |
| src/BootstrapBlazor.Server/Components/Layout/BaseLayout.razor.cs | Removed vote toast functionality, simplified dispose pattern, changed lifecycle methods, and removed JSModule management |
| src/BootstrapBlazor.Server/Components/Layout/BaseLayout.razor | Removed RenderVote code block |
| src/BootstrapBlazor.Server/Components/Components/GlobalToast.razor.js | New file implementing toast initialization and event handling for vote functionality |
| src/BootstrapBlazor.Server/Components/Components/GlobalToast.razor.cs | New component class with toast display logic |
| src/BootstrapBlazor.Server/Components/Components/GlobalToast.razor | New component template with vote UI markup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+128
to
+140
| private void Dispose(bool disposing) | ||
| { | ||
| if (disposing) | ||
| { | ||
| CommitDispatchService.UnSubscribe(NotifyCommit); | ||
| RebootDispatchService.UnSubscribe(NotifyReboot); | ||
|
|
||
| if (_module != null) | ||
| { | ||
| await _module.InvokeVoidAsync("dispose"); | ||
| await _module.DisposeAsync(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// 释放资源 | ||
| /// </summary> | ||
| public async ValueTask DisposeAsync() | ||
| public void Dispose() |
There was a problem hiding this comment.
The change from IAsyncDisposable to IDisposable is problematic because the previous implementation needed to dispose of JSModule asynchronously. While the current code doesn't store the module reference, if async disposal is needed in the future (which is likely given that JSModule disposal is async), this change will require reverting back to IAsyncDisposable. Consider keeping IAsyncDisposable for consistency with JavaScript interop best practices.
Comment on lines
+1
to
+14
| @inherits WebSiteModuleComponentBase | ||
|
|
||
| @code { | ||
| RenderFragment RenderVote => | ||
| @<div> | ||
| <div style="font-size: 48px; text-align: center;">🏆</div> | ||
| <p class="text-center mt-2">正在参加 <b>Gitee 2025 最受欢迎的开源软件</b> 投票活动，快来给我投票吧！</p> | ||
| <div class="my-3 text-center">您的每一票都是对开源社区的支持，感谢您的参与！</div> | ||
| <div style="display: flex; justify-content: space-around;" id="bb-g-toast"> | ||
| <a href="https://gitee.com/activity/2025opensource?ident=I6MYBB" target="_blank" style="font-weight: bold; padding: 6px 12px; border-radius: var(--bs-border-radius); background: linear-gradient(135deg, #667eea, #764ba2); color: #fff;">🚀 必须投一票</a> | ||
| <a href="https://gitee.com/activity/2025opensource?ident=I6MYBB" target="_blank" class="text-muted" style="padding: 6px 12px;" title="老六你居然不投票">我知道了</a> | ||
| </div> | ||
| </div>; | ||
| } |
There was a problem hiding this comment.
The GlobalToast component has been created but is not instantiated anywhere in the application. The vote toast functionality was removed from BaseLayout, but the new GlobalToast component needs to be added to a layout or component for the functionality to work. Consider adding <GlobalToast /> to BaseLayout.razor or another appropriate location.
Comment on lines
+78
to
+79
| var module = await JSRuntime.LoadModule($"{WebsiteOption.Value.JSModuleRootPath}Layout/BaseLayout.razor.js"); | ||
| await module.InvokeVoidAsync("initTheme"); |
There was a problem hiding this comment.
The JavaScript module loaded in OnInitializedAsync is not being disposed. The module reference should be stored in a field and disposed in the Dispose method to prevent memory leaks. Consider storing the module as a field and calling DisposeAsync on it during disposal.
| namespace BootstrapBlazor.Server.Components.Components; | ||
|
|
||
| /// <summary> | ||
| /// 正站通知组件 |
There was a problem hiding this comment.
The comment contains a typo. It should be "全站通知组件" (full site notification component) instead of "正站通知组件".
Suggested change
| /// 正站通知组件 | |
| /// 全站通知组件 |
Comment on lines
+17
to
+25
| [Inject] | ||
| [NotNull] | ||
| private ToastService? Toast { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// <inheritdoc/> | ||
| /// </summary> | ||
| /// <returns></returns> | ||
| protected override Task InvokeInitAsync() => InvokeVoidAsync("init", Interop, nameof(ShowToast)); |
There was a problem hiding this comment.
The GlobalToast component needs access to WebsiteOption to pass the IsDevelopment flag to the JavaScript init function (as done in the original BaseLayout implementation). Since WebSiteModuleComponentBase has WebsiteOption as a private field, GlobalToast should inject IOptions<WebsiteOptions> and pass WebsiteOption.Value.IsDevelopment as the third parameter to InvokeInitAsync.
Comment on lines
+74
to
+80
| protected override async Task OnInitializedAsync() | ||
| { | ||
| await base.OnAfterRenderAsync(firstRender); | ||
| await base.OnInitializedAsync(); | ||
|
|
||
| if (firstRender) | ||
| { | ||
| _module = await JSRuntime.LoadModule($"{WebsiteOption.Value.JSModuleRootPath}Layout/BaseLayout.razor.js"); | ||
| _interop = DotNetObjectReference.Create(this); | ||
| await _module.InvokeVoidAsync("doTask", _interop, WebsiteOption.Value.IsDevelopment); | ||
| _init = true; | ||
| StateHasChanged(); | ||
| } | ||
| var module = await JSRuntime.LoadModule($"{WebsiteOption.Value.JSModuleRootPath}Layout/BaseLayout.razor.js"); | ||
| await module.InvokeVoidAsync("initTheme"); | ||
| _init = true; |
There was a problem hiding this comment.
Moving the module initialization from OnAfterRenderAsync to OnInitializedAsync may cause issues because the JavaScript may not be ready to execute at this lifecycle stage. The DOM is not guaranteed to be rendered during OnInitializedAsync, which could lead to JavaScript errors if the initTheme function interacts with DOM elements. Consider keeping this in OnAfterRenderAsync with a firstRender check.
| } | ||
| var module = await JSRuntime.LoadModule($"{WebsiteOption.Value.JSModuleRootPath}Layout/BaseLayout.razor.js"); | ||
| await module.InvokeVoidAsync("initTheme"); | ||
| _init = true; |
There was a problem hiding this comment.
The StateHasChanged call was removed when _init is set to true. Without StateHasChanged, the UI may not re-render to display the content inside the @if (_init) block in the razor file. This could cause the Header and main content not to appear until the next render cycle is triggered.
Suggested change
| _init = true; | |
| _init = true; | |
| await InvokeAsync(StateHasChanged); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #7488
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Introduce a global toast component for site-wide notifications and simplify BaseLayout initialization and disposal by moving Gitee voting logic out of the layout.
New Features:
Enhancements: