Conversation
# Conflicts: # src/components/BootstrapBlazor.CherryMarkdown/BootstrapBlazor.CherryMarkdown.csproj # src/components/BootstrapBlazor.Markdown/BootstrapBlazor.Markdown.csproj # src/components/BootstrapBlazor.Region/BootstrapBlazor.Region.csproj # src/components/BootstrapBlazor.SummerNote/BootstrapBlazor.SummerNote.csproj # src/components/BootstrapBlazor.Vditor/BootstrapBlazor.Vditor.csproj # test/UnitTestEditor/UnitTestEditor.csproj # test/UnitTestHoliday/UnitTestHoliday.csproj # test/UnitTestOpcDa/UnitTestOpcDa.csproj # test/UnitTestOpcUa/UnitTestOpcUa.csproj # test/UnitTestRegion/UnitTestRegion.csproj # test/UnitTestSvgIcon/UnitTestSvgIcon.csproj # tools/BootstrapBlazor.CssBundler/BootstrapBlazor.CssBundler.csproj
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR upgrades the entire solution to version 10.0.0, extends PDF service APIs to accept an optional PdfOptions parameter in both Playwright and Select implementations, and refreshes license headers for consistency. Class diagram for updated DefaultPdfService API (Playwright and Select)classDiagram
class DefaultPdfService {
+Task<byte[]> PdfDataAsync(string url, PdfOptions? options = null)
+Task<Stream> PdfStreamAsync(string url, PdfOptions? options = null)
+Task<byte[]> PdfDataFromHtmlAsync(string html, IEnumerable<string>? links = null, IEnumerable<string>? scripts = null, PdfOptions? options = null)
+Task<Stream> PdfStreamFromHtmlAsync(string html, IEnumerable<string>? links = null, IEnumerable<string>? scripts = null, PdfOptions? options = null)
}
class PdfOptions {
}
DefaultPdfService ..> PdfOptions : uses
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}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The PdfOptions parameter is never used in GeneratePdfFromUrlAsync or in the HtmlToPdf converters, so the new options have no effect—consider wiring them through or removing them.
- You updated the copyright header in one service but not others—either standardize this change project-wide or revert it to keep headers consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The PdfOptions parameter is never used in GeneratePdfFromUrlAsync or in the HtmlToPdf converters, so the new options have no effect—consider wiring them through or removing them.
- You updated the copyright header in one service but not others—either standardize this change project-wide or revert it to keep headers consistent.
## Individual Comments
### Comment 1
<location> `src/components/BootstrapBlazor.Html2Pdf.Playwright/Services/DefaultPdfService.cs:14` </location>
<code_context>
- /// <inheritdoc/>
- /// </summary>
- public Task<byte[]> PdfDataAsync(string url)
+ public Task<byte[]> PdfDataAsync(string url, PdfOptions? options = null)
{
return GeneratePdfFromUrlAsync(url);
</code_context>
<issue_to_address>
**issue:** The new 'options' parameter is unused in PdfDataAsync.
If the unused 'options' parameter is for compatibility, please document this. Otherwise, pass 'options' to the PDF generation logic.
</issue_to_address>
### Comment 2
<location> `src/components/BootstrapBlazor.Html2Pdf.Playwright/Services/DefaultPdfService.cs:19` </location>
<code_context>
- /// <inheritdoc/>
- /// </summary>
- public async Task<Stream> PdfStreamAsync(string url)
+ public async Task<Stream> PdfStreamAsync(string url, PdfOptions? options = null)
{
var data = await GeneratePdfFromUrlAsync(url);
</code_context>
<issue_to_address>
**issue:** The 'options' parameter is unused in PdfStreamAsync.
If you plan to support options later, add a TODO or use the parameter to clarify its purpose.
</issue_to_address>
### Comment 3
<location> `src/components/BootstrapBlazor.Html2Pdf.Playwright/Services/DefaultPdfService.cs:25` </location>
<code_context>
- /// <param name="links"></param>
- /// <param name="scripts"></param>
- public Task<byte[]> PdfDataFromHtmlAsync(string html, IEnumerable<string>? links = null, IEnumerable<string>? scripts = null)
+ public Task<byte[]> PdfDataFromHtmlAsync(string html, IEnumerable<string>? links = null, IEnumerable<string>? scripts = null, PdfOptions? options = null)
{
return GeneratePdfFromHtmlAsync(html, links, scripts);
</code_context>
<issue_to_address>
**issue:** The 'options' parameter is unused in PdfDataFromHtmlAsync.
If 'options' is meant to influence PDF generation, it should be passed to GeneratePdfFromHtmlAsync.
</issue_to_address>
### Comment 4
<location> `src/components/BootstrapBlazor.Html2Pdf.Playwright/Services/DefaultPdfService.cs:30` </location>
<code_context>
- /// <param name="links"></param>
- /// <param name="scripts"></param>
- public async Task<Stream> PdfStreamFromHtmlAsync(string html, IEnumerable<string>? links = null, IEnumerable<string>? scripts = null)
+ public async Task<Stream> PdfStreamFromHtmlAsync(string html, IEnumerable<string>? links = null, IEnumerable<string>? scripts = null, PdfOptions? options = null)
{
var data = await PdfDataFromHtmlAsync(html, links, scripts);
</code_context>
<issue_to_address>
**issue:** The 'options' parameter is unused in PdfStreamFromHtmlAsync.
If 'options' is only present for interface compatibility, document this clearly. Otherwise, use or remove the parameter.
</issue_to_address>
### Comment 5
<location> `src/components/BootstrapBlazor.Html2Pdf.Select/Services/DefaultPdfService.cs:14` </location>
<code_context>
- /// <inheritdoc/>
- /// </summary>
- public Task<byte[]> PdfDataAsync(string url)
+ public Task<byte[]> PdfDataAsync(string url, PdfOptions? options = null)
{
return GeneratePdfFromUrlAsync(url);
</code_context>
<issue_to_address>
**issue:** The 'options' parameter is unused in PdfDataAsync.
If 'options' should influence PDF generation, ensure it is used in the implementation, such as by passing it to HtmlToPdf.
</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> | ||
| public Task<byte[]> PdfDataAsync(string url) | ||
| public Task<byte[]> PdfDataAsync(string url, PdfOptions? options = null) |
There was a problem hiding this comment.
issue: The new 'options' parameter is unused in PdfDataAsync.
If the unused 'options' parameter is for compatibility, please document this. Otherwise, pass 'options' to the PDF generation logic.
| /// <inheritdoc/> | ||
| /// </summary> | ||
| public async Task<Stream> PdfStreamAsync(string url) | ||
| public async Task<Stream> PdfStreamAsync(string url, PdfOptions? options = null) |
There was a problem hiding this comment.
issue: The 'options' parameter is unused in PdfStreamAsync.
If you plan to support options later, add a TODO or use the parameter to clarify its purpose.
| /// <param name="links"></param> | ||
| /// <param name="scripts"></param> | ||
| public Task<byte[]> PdfDataFromHtmlAsync(string html, IEnumerable<string>? links = null, IEnumerable<string>? scripts = null) | ||
| public Task<byte[]> PdfDataFromHtmlAsync(string html, IEnumerable<string>? links = null, IEnumerable<string>? scripts = null, PdfOptions? options = null) |
There was a problem hiding this comment.
issue: The 'options' parameter is unused in PdfDataFromHtmlAsync.
If 'options' is meant to influence PDF generation, it should be passed to GeneratePdfFromHtmlAsync.
| /// <param name="links"></param> | ||
| /// <param name="scripts"></param> | ||
| public async Task<Stream> PdfStreamFromHtmlAsync(string html, IEnumerable<string>? links = null, IEnumerable<string>? scripts = null) | ||
| public async Task<Stream> PdfStreamFromHtmlAsync(string html, IEnumerable<string>? links = null, IEnumerable<string>? scripts = null, PdfOptions? options = null) |
There was a problem hiding this comment.
issue: The 'options' parameter is unused in PdfStreamFromHtmlAsync.
If 'options' is only present for interface compatibility, document this clearly. Otherwise, use or remove the parameter.
| /// <inheritdoc/> | ||
| /// </summary> | ||
| public Task<byte[]> PdfDataAsync(string url) | ||
| public Task<byte[]> PdfDataAsync(string url, PdfOptions? options = null) |
There was a problem hiding this comment.
issue: The 'options' parameter is unused in PdfDataAsync.
If 'options' should influence PDF generation, ensure it is used in the implementation, such as by passing it to HtmlToPdf.
There was a problem hiding this comment.
Pull Request Overview
This PR bumps the target framework from .NET 9.0 to .NET 10.0 across the BootstrapBlazor ecosystem. The PR title indicates "bump vesion 10.0.0" with a typo ("vesion" should be "version").
Key changes:
- Updates the target framework to
net10.0across all projects - Consolidates version management by introducing version variables (
NET6Version,NET7Version, etc.) in Directory.Build.props - Removes hardcoded version numbers from individual project files in favor of centralized version management
- Updates package references to use wildcard or variable-based versioning
Reviewed Changes
Copilot reviewed 92 out of 92 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/BootstrapBlazor.CssBundler/BootstrapBlazor.CssBundler.csproj | Updates version to 10.0.0, adds net10.0 as target framework, consolidates PropertyGroups |
| test/Directory.Build.props | Updates TargetFramework from net9.0 to net10.0, removes global Using directives |
| test/UnitTest*.csproj (multiple) | Removes explicit framework configurations and adds specific Using directives where needed |
| src/Directory.Build.props | Updates Version to 10.0.0, introduces version variables for different .NET versions |
| src/Frameworks.props | Adds net10.0 to TargetFrameworks, removes Visual Studio version conditions |
| src/components/*.csproj (multiple) | Removes explicit Version properties, standardizes package references to use version variables |
| src/extensions/*.csproj (multiple) | Updates package version references to use centralized version variables |
| src/middleware/*.csproj | Removes explicit Version property |
| BootstrapBlazor.Extensions.slnx | Reorganizes Directory.Build.props into a dedicated /props/ folder |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </PropertyGroup> | ||
|
|
||
| <PropertyGroup> | ||
| <TargetFrameworks>net10.0</TargetFrameworks> |
There was a problem hiding this comment.
Both TargetFramework (line 6) and TargetFrameworks (line 12) are defined, which is incorrect. Only one should be used. Since TargetFramework (singular) is already set to 'net10.0' on line 6, the TargetFrameworks (plural) property on line 12 should be removed or the singular version should be removed if multi-targeting is intended.
|
|
||
| <PropertyGroup> | ||
| <BBVersion>9.0.0</BBVersion> | ||
| <BBVersion>9.*</BBVersion> |
There was a problem hiding this comment.
The BBVersion is set to '9.' but the overall Version has been updated to '10.0.0'. This creates an inconsistency where projects reference BootstrapBlazor version 9.x while the solution is being updated to version 10.0.0. This should likely be '10.' or '10.0.0' to match the version bump.
| <BBVersion>9.*</BBVersion> | |
| <BBVersion>10.*</BBVersion> |
| @@ -1,4 +1,4 @@ | |||
| // Copyright (c) Argo Zhang (argo@163.com). All rights reserved. | |||
| // Copyright (c) BootstrapBlazor & Argo Zhang (argo@live.ca). All rights reserved. | |||
There was a problem hiding this comment.
The email address has been changed from 'argo@163.com' to 'argo@live.ca', which is inconsistent with other files in this PR (e.g., BootstrapBlazor.Html2Pdf.Select/Services/DefaultPdfService.cs). Copyright headers should remain consistent across the codebase unless there's a deliberate policy change.
|
|
||
| <ItemGroup> | ||
| <Using Include="BootstrapBlazor.Components" /> | ||
| <PackageReference Include="BootstrapBlazor" Version="$(BBVersion)" /> |
There was a problem hiding this comment.
[nitpick] The PackageReference and Using item groups have been swapped. While this doesn't affect functionality, it creates inconsistency. The original order (Using includes before PackageReference) should be maintained for better readability and consistency with the codebase conventions.
| @@ -13,7 +9,6 @@ | |||
| <ItemGroup> | |||
| <PackageReference Include="Baidu.AI" Version="4.15.16" /> | |||
| <PackageReference Include="BootstrapBlazor" Version="$(BBVersion)" /> | |||
There was a problem hiding this comment.
The Newtonsoft.Json package reference has been removed. If this package was a dependency of Baidu.AI and is now pulled in transitively, this is fine. However, if the code directly uses Newtonsoft.Json types, this removal could cause build errors. Please verify that Newtonsoft.Json is not directly used in this project.
| <PackageReference Include="BootstrapBlazor" Version="$(BBVersion)" /> | |
| <PackageReference Include="BootstrapBlazor" Version="$(BBVersion)" /> | |
| <PackageReference Include="Newtonsoft.Json" Version="13.0.3" /> |
Link issues
fixes #672
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Bump library version to 10.0.0 and extend HTML-to-PDF services to support optional PDF configuration
Enhancements:
Build:
Chores: