Fix export column widths in new Financial sample app #333
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR centralizes column width settings for Excel and PDF exports into a shared utility and cleans up repetitive inline width assignments across multiple pages. Additionally, it adds a display title for the Payment Method column in Overview and Analytics.
- Introduced
CommonUtils.SetExportableColumnWidthsto apply consistent column widths. - Refactored
OnGridBeforeExcelExportandOnGridBeforePdfExporthandlers to use the new utility. - Added a
Title="Payment Method"attribute to thePaymentMethodIdcolumn in Overview and Analytics pages.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
Services/CommonUtils.cs |
New utility class for export column width settings. |
Components/Pages/Transactions.razor |
Replaced manual width assignments with utility call. |
Components/Pages/Overview.razor |
Ditto, plus added Title for Payment Method column. |
Components/Pages/Analytics.razor |
Same refactoring and Title addition. |
Comments suppressed due to low confidence (3)
sample-applications/blazor-financial-dashboard/BlazorFinancialDashboard/Services/CommonUtils.cs:8
- The variable name 'ExportableColumnWiths' contains a typo and may be misleading. Consider renaming it to 'ExportableColumnWidths' for clarity.
private static readonly Dictionary<string, string> ExportableColumnWiths = new()
sample-applications/blazor-financial-dashboard/BlazorFinancialDashboard/Services/CommonUtils.cs:6
- [nitpick] Public utility class and methods lack XML documentation. Adding
<summary>comments would improve maintainability and API clarity.
public static class CommonUtils
sample-applications/blazor-financial-dashboard/BlazorFinancialDashboard/Services/CommonUtils.cs:20
- New utility methods aren’t covered by any unit tests. Adding tests for both Excel and PDF overloads would help prevent regressions in column width logic.
public static void SetExportableColumnWidths(List<GridExcelExportColumn> currentColumns)
| public static void SetExportableColumnWidths(List<GridExcelExportColumn> currentColumns) | ||
| { | ||
| foreach (GridExcelExportColumn column in currentColumns) |
There was a problem hiding this comment.
[nitpick] There are two nearly identical overloads of SetExportableColumnWidths for Excel and PDF columns. Consider abstracting the shared logic (e.g., via a generic method or interface) to reduce duplication.
| public static void SetExportableColumnWidths(List<GridExcelExportColumn> currentColumns) | |
| { | |
| foreach (GridExcelExportColumn column in currentColumns) | |
| public static void SetExportableColumnWidths<T>(List<T> currentColumns) where T : IExportableColumn | |
| { | |
| foreach (T column in currentColumns) |
Copilot uses AI. Check for mistakes.
No description provided.