VPR-157 feat(a11y): accessible Word, PDF, and Excel exports#165
Open
VPR-157 feat(a11y): accessible Word, PDF, and Excel exports#165
Conversation
Word: register Title and Heading 2 styles, set core file properties (Title/Subject/Creator/Language), and write image alt text following the displayed-name rule from PLAN-Word-PDF-WCAG.md. PDF: enable PDFUA_1 conformance with metadata, and tag titles (SemanticHeader1), group headers (SemanticHeader2), photos (SemanticImage with alt text), and the page-number footer (SemanticIgnore artifact). Photo-grid tables stay untagged because they're layout, not data.
Adds AddPdfPageNumberFooter to BaseReportService — a SemanticIgnore- wrapped footer (filter line + page numbers) so the repeated content is treated as a page artifact instead of being announced on every page. ClinicalEffortService is converted as the reference implementation: it chains WithAccessibility for PDF/UA and metadata, marks the institution + date row as SemanticIgnore, the report title as SemanticHeader1, and each job-group section as SemanticHeader2. The other 8 effort services (ClinicalSchedule, DeptSummary, EvaluationReport, MeritMultiYear, MeritReport, MeritSummary, SchoolSummary, TeachingActivity) need the same treatment in a follow-up.
Excel exports also need workbook core properties and structured tables so the Office Accessibility Checker passes and screen readers announce column headers correctly. Companion to PdfAccessibilityHelper / WordAccessibilityHelper, applied across Effort report Excel exports in subsequent commits.
…lity PDF: header marked as semantic H1, institution/date row and footer flagged as repetition artifacts, content table tagged. PDF/UA metadata applied via WithAccessibility. Excel: workbook core properties set; data range promoted to a structured Excel Table so screen readers announce row data in column-header terms.
PDF: institution/date row marked as artifact, title as H1, content table tagged, footer routed through the shared AddPdfPageNumberFooter helper, PDF/UA metadata applied via WithAccessibility. Excel: workbook core properties set. Skipped structured-table promotion because each department block emits totals + faculty-count + averages rows of differing shapes — promoting them as one table would mislead screen readers about row uniformity.
…ility PDF: institution/date row hidden as artifact, title as H1, dept name as H2, content table tagged, footer routed through AddPdfPageNumberFooter, WithAccessibility metadata applied. Excel: workbook core properties set; the instructor rows (uniform shape) promoted to a structured Excel Table so AT announces effort columns by header. Department-totals and faculty-count rows intentionally remain outside the table.
…lity Covers both Grouped (per-department) and Individual (per-instructor) PDFs: institution/date row hidden as artifact, title as H1, dept (and instructor on Individual) as H2, content table tagged, footer routed through AddPdfPageNumberFooter. WithAccessibility metadata applied. Excel: workbook core properties set on both variants. The Individual sheets get their course rows promoted to a structured Excel Table; the Grouped variant has interleaved instructor totals so a single table would misrepresent row uniformity — core properties only there.
PDF: institution/date row marked as artifact, title as H1, dept as H2, job group as H3, content table tagged. Footer routed through the shared helper. WithAccessibility metadata applied. Excel: workbook core properties set. Skipped table promotion — every sheet is just totals + faculty-count + averages rows; no uniform body data to wrap as a table.
Covers MeritDetail and MeritAverage reports. Both PDFs: institution/date row marked artifact, title as H1, dept as H2, job-group as H3 (Average report), content tables tagged, footers routed through AddPdfPageNumberFooter, WithAccessibility metadata applied. Both Excels: workbook core properties set. Skipped table promotion — detail rows are interleaved with instructor-totals/dept-totals rows of differing shape.
…ility Covers EvalSummary and EvalDetail PDFs and Excels: institution/date row hidden as artifact, title as H1, dept as H2, content tables tagged, footers routed through AddPdfPageNumberFooter. WithAccessibility metadata applied. Excel workbook core properties set on both variants. Skipped table promotion — both variants interleave instructor-average and department-average summary rows with course detail rows.
Both pages of the PDF (Merit Activity, Evaluations) get institution/date artifacts hidden, the report title as H1 (Merit page) / H2 (Eval page), content tables tagged, and footers marked as artifacts. WithAccessibility metadata applied at document level. Excel: workbook core properties set. Skipped table promotion — the merit and eval sections share one worksheet with section headers + per-year header rows + interleaved totals; no uniform body data to wrap.
…xports Both services already had PDF accessibility tagging — this catches the matching Excel exports up to the same standard: - Workbook core properties set (Title / Subject / Author / Created) - Data ranges promoted to structured Excel Tables so screen readers can announce columns by header. Per-job-group tables for Clinical Effort, one each for Emergency Contact Overview and Report.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughThis pull request introduces comprehensive accessibility support for PDF, Excel, and Word document exports across multiple reporting services. Three new utility helpers are added to standardize accessibility metadata and semantic markup, along with extensive test coverage validating their functionality. Report generation services are systematically updated to use these helpers while centralizing QuestPDF license configuration in application startup. Changes
Sequence Diagram(s)sequenceDiagram
participant Service as Report Service
participant Helper as Accessibility Helper
participant Doc as Document<br/>(PDF/Excel/Word)
Service->>Service: Generate document content<br/>(headers, tables, data)
Service->>Helper: Call SetCoreProperties/<br/>WithAccessibility
Helper->>Doc: Apply metadata<br/>(title, subject, author, language)
Helper->>Doc: Apply semantic markup<br/>(PDFUA, styles, conformance)
Helper->>Doc: Configure structure<br/>(headers, tables, table names)
Helper-->>Service: Return modified document
Service->>Doc: Return to user
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 59 minutes and 3 seconds.Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #165 +/- ##
==========================================
+ Coverage 43.24% 43.52% +0.28%
==========================================
Files 862 866 +4
Lines 50405 50646 +241
Branches 4706 4742 +36
==========================================
+ Hits 21796 22043 +247
+ Misses 28086 28078 -8
- Partials 523 525 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/Areas/Effort/Services/ClinicalScheduleService.cs (1)
422-435:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the trailing blank Excel column in single-term exports.
When
showTotalis false,colis already one past the last populated header. Using it astotalColsmakesPromoteToAccessibleTablecapture an empty extra column.Suggested fix
- int totalCols = col; + int totalCols = showTotal ? col : col - 1;Also applies to: 464-468
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/Areas/Effort/Services/ClinicalScheduleService.cs` around lines 422 - 435, In ClinicalScheduleService, the header logic uses `col` (which is one past the last filled column when `showTotal` is false) as `totalCols`, causing PromoteToAccessibleTable to include a trailing empty column; change the `totalCols` computation to reflect the last populated column (e.g., use `totalCols = showTotal ? col : col - 1` or `totalCols = col - (showTotal ? 0 : 1)`) and apply the same fix to the other occurrence around the block using `row`/`col` at lines 464-468 so PromoteToAccessibleTable receives the correct column range.
🧹 Nitpick comments (2)
web/Areas/Effort/Services/EvaluationReportService.cs (1)
681-725: ⚡ Quick winPromote the summary grid to an accessible table before the department total.
This sheet now gets core properties, but the instructor rows are still plain cells. That means screen readers still won't get header associations for the main data block even though the rows are uniform up to
Department Average.Suggested fix
// Column headers + int headerRow = row; ws.Cell(row, 1).Value = "Instructor"; ws.Cell(row, 2).Value = "Average"; ws.Range($"{row}:{row}").Style.Font.Bold = true; ws.SheetView.FreezeRows(row); row++; @@ foreach (var instructor in dept.Instructors) { ws.Cell(row, 1).Value = ExcelHelper.SanitizeStringCell(instructor.Instructor); ws.Cell(row, 2).Value = instructor.WeightedAverage; ws.Cell(row, 2).Style.NumberFormat.Format = "0.00"; row++; } + + var lastDataRow = row - 1; + if (lastDataRow > headerRow) + { + ExcelAccessibilityHelper.PromoteToAccessibleTable( + ws.Range(headerRow, 1, lastDataRow, 2), + $"EvalSummary_{dept.Department}"); + } // Department average ws.Cell(row, 1).Value = "Department Average";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/Areas/Effort/Services/EvaluationReportService.cs` around lines 681 - 725, The instructor list is written as plain cells in GenerateEvalSummaryExcel so screen readers can't associate headers; convert the header + instructor rows into an actual Excel table before writing the "Department Average" row: after you write the column headers (the code that sets ws.Cell(row,1) "Instructor" / ws.Cell(row,2) "Average") and after the loop that writes dept.Instructors, call ws.Tables.Add(...) (or the ClosedXML equivalent) using the range from the header row to the last instructor row (use the local row variable to compute the end row), give the table a unique name (e.g., based on dept.Department), ensure the header row is marked as headers and autofilter is enabled, and then continue to write the Department Average and call ShadeExcelRow; keep existing helpers AddExcelHeader/AddExcelFilterLine, ExcelHelper.SanitizeStringCell, and ShadeExcelRow unchanged.web/Areas/Students/Services/PhotoExportService.cs (1)
186-251: ⚖️ Poor tradeoffConsider extracting common image-drawing logic.
The inline drawing construction (wpDocProps, picNonVisualProps, SetImageAltText, inline assembly) is repeated in both the large-layout chunked path and the standard-layout path (lines 304-370). A private helper like
CreatePhotoDrawing(MainDocumentPart, byte[], uint imageId, long width, long height, string altText)could reduce duplication.However, given the OpenXML fluent construction style and the pragma already suppressing S3220, this is optional and can be deferred.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/Areas/Students/Services/PhotoExportService.cs` around lines 186 - 251, The repeated OpenXML image-drawing construction in PhotoExportService should be extracted into a private helper to remove duplication: add a method like CreatePhotoDrawing(MainDocumentPart mainPart, byte[] photoBytes, ref uint imageId, long width, long height, string altText) that (1) adds an ImagePart and feeds photoBytes, (2) increments imageId and creates DocProperties and NonVisualDrawingProperties, (3) calls WordAccessibilityHelper.SetImageAltText, (4) constructs and returns a DocumentFormat.OpenXml.Wordprocessing.Drawing (or the Inline to wrap into a Drawing). Replace the inline construction blocks in the standard-layout and large-layout paths with calls to this helper and append the returned drawing into the photoRun; ensure imageId is shared (pass by ref) and altText uses student.GroupExportName.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/Areas/Effort/Services/BaseReportService.cs`:
- Around line 226-243: The footer currently calls footer.SemanticIgnore() which
hides the entire footer (including filter text added via AddPdfFilterLine) from
accessibility tools; change AddPdfPageNumberFooter so that
AddPdfFilterLine(col.Item(), filters) is rendered outside the SemanticIgnore
scope and only the page counter column (the col.Item().AlignCenter().Text block
that calls CurrentPageNumber/TotalPages) is wrapped with SemanticIgnore(),
ensuring filter labels/values remain in the semantic tree while the page counter
remains ignored by assistive tech.
In `@web/Program.cs`:
- Around line 46-48: The test PdfAccessibilityHelperTests fails because
Document.Create() is called without initializing QuestPDF.Settings.License; move
the license initialization into a shared helper (e.g., a static method like
EnsureQuestPdfLicense()) that sets QuestPDF.Settings.License =
LicenseType.Community and call that helper from Program.cs startup and from the
PdfAccessibilityHelperTests (or add a one-line call to the helper in the test
class setup) so any code path (including tests) that creates QuestPDF documents
runs after the license is set.
---
Outside diff comments:
In `@web/Areas/Effort/Services/ClinicalScheduleService.cs`:
- Around line 422-435: In ClinicalScheduleService, the header logic uses `col`
(which is one past the last filled column when `showTotal` is false) as
`totalCols`, causing PromoteToAccessibleTable to include a trailing empty
column; change the `totalCols` computation to reflect the last populated column
(e.g., use `totalCols = showTotal ? col : col - 1` or `totalCols = col -
(showTotal ? 0 : 1)`) and apply the same fix to the other occurrence around the
block using `row`/`col` at lines 464-468 so PromoteToAccessibleTable receives
the correct column range.
---
Nitpick comments:
In `@web/Areas/Effort/Services/EvaluationReportService.cs`:
- Around line 681-725: The instructor list is written as plain cells in
GenerateEvalSummaryExcel so screen readers can't associate headers; convert the
header + instructor rows into an actual Excel table before writing the
"Department Average" row: after you write the column headers (the code that sets
ws.Cell(row,1) "Instructor" / ws.Cell(row,2) "Average") and after the loop that
writes dept.Instructors, call ws.Tables.Add(...) (or the ClosedXML equivalent)
using the range from the header row to the last instructor row (use the local
row variable to compute the end row), give the table a unique name (e.g., based
on dept.Department), ensure the header row is marked as headers and autofilter
is enabled, and then continue to write the Department Average and call
ShadeExcelRow; keep existing helpers AddExcelHeader/AddExcelFilterLine,
ExcelHelper.SanitizeStringCell, and ShadeExcelRow unchanged.
In `@web/Areas/Students/Services/PhotoExportService.cs`:
- Around line 186-251: The repeated OpenXML image-drawing construction in
PhotoExportService should be extracted into a private helper to remove
duplication: add a method like CreatePhotoDrawing(MainDocumentPart mainPart,
byte[] photoBytes, ref uint imageId, long width, long height, string altText)
that (1) adds an ImagePart and feeds photoBytes, (2) increments imageId and
creates DocProperties and NonVisualDrawingProperties, (3) calls
WordAccessibilityHelper.SetImageAltText, (4) constructs and returns a
DocumentFormat.OpenXml.Wordprocessing.Drawing (or the Inline to wrap into a
Drawing). Replace the inline construction blocks in the standard-layout and
large-layout paths with calls to this helper and append the returned drawing
into the photoRun; ensure imageId is shared (pass by ref) and altText uses
student.GroupExportName.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d36d72a0-5f85-4fcb-9a8f-5ae8c231687f
📒 Files selected for processing (19)
test/Classes/Utilities/ExcelAccessibilityHelperTests.cstest/Classes/Utilities/PdfAccessibilityHelperTests.cstest/Classes/Utilities/WordAccessibilityHelperTests.csweb/Areas/Effort/Services/BaseReportService.csweb/Areas/Effort/Services/ClinicalEffortService.csweb/Areas/Effort/Services/ClinicalScheduleService.csweb/Areas/Effort/Services/DeptSummaryService.csweb/Areas/Effort/Services/EvaluationReportService.csweb/Areas/Effort/Services/MeritMultiYearService.csweb/Areas/Effort/Services/MeritReportService.csweb/Areas/Effort/Services/MeritSummaryService.csweb/Areas/Effort/Services/SchoolSummaryService.csweb/Areas/Effort/Services/TeachingActivityService.csweb/Areas/Students/Services/EmergencyContactExportService.csweb/Areas/Students/Services/PhotoExportService.csweb/Classes/Utilities/ExcelAccessibilityHelper.csweb/Classes/Utilities/PdfAccessibilityHelper.csweb/Classes/Utilities/WordAccessibilityHelper.csweb/Program.cs
- AddPdfPageNumberFooter: only the "Page N of M" counter is wrapped in SemanticIgnore. The filter summary was also being hidden, which lost context for reports (e.g. evaluation reports) where filters appear nowhere outside the footer. - ClinicalScheduleService Excel: totalCols was off-by-one when showTotal is false, causing PromoteToAccessibleTable to capture an empty trailing column. - EvaluationReportService GenerateEvalSummaryExcel: instructor rows are now promoted to a structured Excel Table; matches the same treatment the eval-detail and other uniform-row exports already get.
Bundle ReportChanges will increase total bundle size by 2.92kB (0.14%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: viper-frontend-esmAssets Changed:
|
…aces - Fix EnsureAccessibilityStyles_IsIdempotent CI failure: OpenXML 3.1.1 rejects re-binding a Styles tree that's already attached. Read the existing tree once and only assign stylesPart.Styles when fresh. - Add interface XML docs on the 11 export contracts touched by this PR, describing layout and where structured Excel Tables are used.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Adds WCAG 2.1 / PDF/UA-1 tagging and Excel a11y to every export pipeline currently shipping in VIPER:
New helpers (
web/Classes/Utilities/)WordAccessibilityHelper— heading styles, image alt text, table-header rows, document language, core file properties.PdfAccessibilityHelper—WithAccessibility(...)extension wrappingDocumentMetadata+DocumentSettings.PDFUA_Conformance = PDFUA_1.ExcelAccessibilityHelper— workbook core properties,PromoteToAccessibleTablefor structured Excel Tables.Program.csso it's set once at startup.Word export tagged
PhotoExportService): heading styles, alt text on every photo, document language, core properties.PDF exports tagged (institution/date row →
SemanticIgnore, title →SemanticHeader1, dept/sub-headers →SemanticHeader2/SemanticHeader3, content tables →SemanticTable, footers → artifact via sharedAddPdfPageNumberFooterhelper,WithAccessibilityfor metadata + PDF/UA conformance):Excel exports tagged (workbook core properties on all; structured
PromoteToAccessibleTablewhere data rows are uniform):Tests — 32 unit tests across
WordAccessibilityHelperTests,PdfAccessibilityHelperTests,ExcelAccessibilityHelperTests. Includes regression guards for the review issues addressed in the helpers commit (Title style not marked default, Title vs H1 font sizes differ, alt text leaves Title null when caller doesn't pass one).Summary by CodeRabbit