Conversation
- Updated `HelloReport.razor` to include a new paragraph element for displaying a greeting message. - Added an IIFE in `HelloJavascriptData.js` to manage the `HelloWorld` element and set its content. - Modified `BlazorReportsTemplate.razor` to improve indentation and introduced a `Scripts` parameter for including JavaScript files. - Registered `JavascriptContainer` in `ServiceCollectionExtensions.cs` to manage JavaScript files. - Created `JavascriptContainer.cs` to scan and store JavaScript file contents from the `wwwroot/js` directory. - Updated `ReportService.cs` to utilize `JavascriptContainer` for dynamic script inclusion in reports.
- Introduced a new `BlazorReport` class for managing report readiness. - Updated `HelloJavascriptData.js` to set report status and modify HTML content. - Renamed `JavascriptSettings` to `JavascriptInternalSettings` in `BlazorReportsOptions`. - Adjusted `BrowserFactory` and `ReportService` to utilize the new internal settings. - Improved overall structure and clarity of JavaScript interactions in the Blazor application.
…lling. Refactor report status handling and JavaScript integration Updated `HelloJavascriptData.js` to use `notReady()` and dynamic timeout for report readiness. Replaced `ready()` with `notReady()` in `BlazorReportsTemplate.razor` and added `waitForSignal` for improved asynchronous handling. Modified `Browser.cs` to utilize `waitForSignal` for waiting on JavaScript flags. Refactored `WaitForJsFlagAsync` in `BrowserPage.cs` to evaluate JavaScript directly, removing the flag name parameter and enhancing error handling with the new `EvaluateJavaScriptAsync` method.
…javascript also added the CurrentReportJsOptions so people can set javascript options per report or globally.
- Updated `HelloReport.razor` to display greeting and default message. - Added `SimpleJsTimeoutReport` to `Program.cs` with specific options. - Modified `SimpleJsTimeoutReport.razor` to include an example of JavaScript update and timeout signaling. - Introduced `SimpleJsTimeoutReportData` for timeout management. - Enhanced error handling in `ReportExtensions.cs` to include 408 status code for timeouts. - Implemented `JavascriptTimedoutProblem` in `Browser.cs` for timeout issues. - Improved JavaScript flag handling in `BrowserPage.cs` with better documentation. - Updated `BrowserService.cs` and `IBrowserService.cs` to incorporate new timeout problem in report generation. - Created `JavascriptTimedoutProblem.cs` to define the new timeout problem struct. - Reflected changes in report generation methods in `ReportService.cs`.
| /// <summary> | ||
| /// Javascript api settings | ||
| /// </summary> | ||
| public BlazorReportCurrentReportJavascriptSettings JavascriptSettings { get; set; } = new(); |
There was a problem hiding this comment.
What other options would be placed here? Thinking about moving the signal and timeout options as a regular setting instead of nesting another object
There was a problem hiding this comment.
As reading it like this, the options don't necesarrily configure Javascript but will just wait for the signal, what do you think?
There was a problem hiding this comment.
We could either rename the object to JavascriptReportSignalSettings or something along those lines...
or we could have something like a js configuration that would have JavascriptReportSignalSettings inside of it if we ever want to give users more flexibility on how to handle the js files in their project, for example
Solution 1
public class JavascriptReportOptions
{
/// <summary>
/// Whether to include the razor.js code-behind file associated with the component (e.g., MyReport.razor.js).
/// </summary>
public bool IncludeCodeBehindScript { get; set; } = true;
/// <summary>
/// Whether to include all global scripts registered in the wwwroot or shared locations for this specific report.
/// </summary>
public bool IncludeGlobalScripts { get; set; } = true;
/// <summary>
/// Settings related to how the JS signals the report is ready.
/// </summary>
public JavascriptReportSignalSettings Signal { get; set; } = new();
/// <summary>
/// Whether to defer script execution until the page is fully loaded so it wouldn't matter if they were placed at the
/// 'head' tag or below the body tag
/// </summary>
public bool DeferScripts { get; set; } = true;
/// <summary>
/// Custom script URLs to include before rendering.
/// </summary>
public List<string> CustomScriptUrls { get; set; } = new();
/// <summary>
/// Custom inline scripts to inject directly into the page.
/// </summary>
public List<string> InlineScripts { get; set; } = new();
}The JavascriptReportOptions would go inside the BlazorReportRegistrationOptions
Solution 2
The other option would be to just do
public class BlazorReportRegistrationOptions
{
/// <summary>
/// Output format for the report. Defaults to PDF.
/// </summary>
public ReportOutputFormat OutputFormat { get; set; } = ReportOutputFormat.Pdf;
/// <summary>
/// The name of the report. This is utilized to generate the route for the report.
/// </summary>
public string? ReportName { get; set; }
/// <summary>
/// Base styles path for the report.
/// </summary>
public string? BaseStylesPath { get; set; }
/// <summary>
/// Assets path for the report.
/// </summary>
public string? AssetsPath { get; set; }
/// <summary>
/// Settings for generating a PDF
/// </summary>
public BlazorReportsPageSettings PageSettings { get; set; } = new();
// --- JavaScript options defined directly here ---
/// <summary>
/// Whether to include the report's `.razor.js` code-behind file.
/// </summary>
public bool IncludeCodeBehindScript { get; set; } = true;
/// <summary>
/// Whether to include globally registered JS scripts.
/// </summary>
public bool IncludeGlobalScripts { get; set; } = true;
/// <summary>
/// Name of the global JS signal to watch for report readiness.
/// </summary>
public string? ReportIsReadySignal { get; set; }
/// <summary>
/// Whether to wait for the `.completed()` signal from JavaScript.
/// </summary>
public bool WaitForJavascriptCompletedSignal { get; set; } = false;
/// <summary>
/// Timeout to wait for JS `.completed()` before failing.
/// </summary>
public TimeSpan WaitForCompletedSignalTimeout { get; set; } = TimeSpan.FromSeconds(10);
}I think I prefer the first one pero both sound good honestly
|
|
||
| TimeSpan timeout = currentReportTimeout ?? globalTimeout; | ||
|
|
||
| var didNotHitTimeOut = await browserPage.WaitForJsFlagAsync(timeout, cancellationToken); |
There was a problem hiding this comment.
What value do you get out of this variable?
There was a problem hiding this comment.
If set to true, it means the report did not hit the timeout—e.g., if the timeout was set 2 seconds and the report finished rendering (including any JavaScript execution) in 1 second, it successfully completed within the allowed time therefore the value will be true (it did not hit the time out).
If set to false, it means the report exceeded the timeout—e.g., if it took 3 seconds to finish due to long-running JavaScript or other dependencies, the timeout would have been hit (did not hit the time out will be false).
Should I rename the variable?
I could also rename the method or reverse the method so that the variable could be
var reportHitTimeout = await browserPage.WaitForJsFlagAsync(timeout, cancellationToken); or something
src/BlazorReports/Services/BrowserServices/Problems/JavascriptTimedoutProblem.cs
Outdated
Show resolved
Hide resolved
|
|
||
| // Function to wait for signal change | ||
| window.waitForSignal = (timeoutMs = 5000, intervalMs = 50) => { | ||
| var expectedValue = 'ready' |
There was a problem hiding this comment.
Change to blazorReportsSignalReady
| /// <summary> | ||
| /// The current report javascript settings. | ||
| /// </summary> | ||
| public required BlazorReportCurrentReportJavascriptSettings CurrentReportJavascriptSettings { get; set; } |
There was a problem hiding this comment.
To mantain consistency use BlazorReportsJavascriptSettings
| /// <summary> | ||
| /// Settings for the internal javascript api | ||
| /// </summary> | ||
| public class BlazorReportCurrentReportJavascriptSettings |
There was a problem hiding this comment.
Probably let's leave it at 1s if it's not completed, to not affect the performance too much if they forget. But this will throw an error right?
| /// <summary> | ||
| /// The signal that the report is ready | ||
| /// </summary> | ||
| public string ReportIsReadySignal { get; set; } = default!; |
| /// <summary> | ||
| /// The default settings for the current report javascript settings | ||
| /// </summary> | ||
| public static BlazorReportCurrentReportJavascriptSettings Default => |
There was a problem hiding this comment.
The defaults can be added to each property
No description provided.