Conversation
Downloading a submission file with annotations included skipped annotations on PDFs: the existing code path wraps each annotation in the file's language comment syntax, which only makes sense for text files. PDF submission files now get their annotations added as PDF "sticky note" text annotations anchored at the annotation's location, so they are visible when the downloaded file is opened in a PDF viewer. Prawn cannot modify an existing PDF, so the notes are built in a separate document and their :Annots entries are copied onto the submission's pages, since CombinePDF drops annotations when injecting one page into another. Annotation coordinates are stored as a fraction of the page as it is displayed, so they are flipped and rotated back into PDF user space for pages that carry a :Rotate entry, as scanned submissions do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
Coverage Report for CI Build 35929381312Coverage increased (+0.01%) to 90.803%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
| doc.start_new_page | ||
| annotations_by_page.fetch(index + 1, []).each do |annotation| | ||
| doc.text_annotation(pdf_annotation_rect(page, annotation), | ||
| "#{annotation.annotation_number}. #{annotation_content(annotation)}", |
Collaborator
There was a problem hiding this comment.
To avoid possible conflicts with ordered list syntax, let's use the display format (#1), (#2), etc.
| annotations_by_page.fetch(index + 1, []).each do |annotation| | ||
| doc.text_annotation(pdf_annotation_rect(page, annotation), | ||
| "#{annotation.annotation_number}. #{annotation_content(annotation)}", | ||
| Name: :Comment, Open: false) |
Collaborator
There was a problem hiding this comment.
I see in the source code there's an option for a "last updated" time. Please include this information in the metadata. I happened to notice that Firefox seems to use this information when displaying annotations.
Use the display format "(MarkUsProject#1)" for annotation numbers in PDF sticky notes, rather than "1.", which can be mistaken for ordered list syntax. Set the :M (modification date) entry on each sticky note so PDF viewers can show when the annotation was last changed; Firefox falls back to the epoch when it is absent. An annotation's text can be edited after the annotation itself was placed, so the later of the two timestamps is used. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
3 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.
Proposed Changes
Closes #7907.
Downloading a submission file with annotations included currently skips annotations on PDFs — the existing code path wraps each annotation in the file's language comment syntax, which only makes sense for text files. The download modal says as much ("Warning: Annotations on PDFs are not currently downloaded.").
PDF submission files now get their annotations added as PDF "sticky note" (
/Subtype /Text) annotations anchored at the annotation's location, so graders' comments are visible when the downloaded file is opened in a PDF viewer. This applies both to downloading a single file and to the zipped download of all files.A few implementation notes:
Prawncannot modify an existing PDF, so (as inResult#generate_print_pdf) the sticky notes are built in a separate, otherwise empty document with one page per page of the submission file, using the undocumentedPrawn::Document#text_annotation.:Annotsentries directly, rather than by overlaying pages:CombinePDFdrops annotations when injecting one page into another (the concat is commented out in the gem), so the overlay approach used for printing loses them.:Rotateentry — scanned submissions do, sinceSplitPdfJobsets it (see feat: option to correct sideways scans #8103).The download warning has been updated to name images, which remain unsupported.
Screenshots of your changes (if applicable)
The only visible UI change is the download modal's warning, which now reads "Warning: Annotations on images are not currently downloaded." The downloaded PDFs themselves show a standard note icon at each annotation's location, which opens a popup with the annotation number and text.
Type of Change
Checklist
Before opening your pull request:
After opening your pull request:
Questions and Comments
🤖 Generated with Claude Code