-
Notifications
You must be signed in to change notification settings - Fork 0
Enhancement/gsdm 169/pdfkit removal #28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* Removed PDFKit implementation to open submitted pdfs * Removed PDF Activity class * updated * Updated PDF Submission * Update mobile-offline-downloader-android * minor update * Update strings.xml * removed pdfkit references from student module * Update libs/annotations/src/main/java/com/instructure/annotations/PDFUtils.kt Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update apps/student/src/main/res/values/styles.xml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update apps/student/src/main/java/com/instructure/student/mobius/assignmentDetails/submissionDetails/content/PdfStudentSubmissionView.kt Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update libs/annotations/src/main/java/com/instructure/annotations/PDFUtils.kt Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * updated pdf download calls * updated PDFUtils * Code cleanup * Update PdfSubmissionView.kt * Update libs/annotations/src/main/java/com/instructure/annotations/PDFUtils.kt Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update apps/student/src/main/res/values/styles.xml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update libs/annotations/src/main/java/com/instructure/annotations/PDFUtils.kt Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * pdf minor text update * Update PDFUtils.kt * updated version * Update OfflineDependencies.kt --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes the PSPDFKit library dependency and implements a simpler approach for handling PDF files by delegating viewing to external PDF viewer applications on the device.
Key changes:
- Removed PSPDFKit library dependency and all related configuration
- Replaced PDF annotation functionality with external app delegation
- Added new utility class
PDFUtilsfor PDF download and opening - Updated DocSession model to include new
pdfjsfield - Removed extensive annotation-related code, fragments, and UI components
Reviewed changes
Copilot reviewed 123 out of 124 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/annotations/src/main/java/com/instructure/annotations/PdfSubmissionView.kt | Simplified from complex annotation view to basic PDF download/open functionality |
| libs/annotations/src/main/java/com/instructure/annotations/PDFUtils.kt | New utility class for downloading and opening PDFs with external apps |
| libs/canvas-api-2/src/main/java/com/instructure/canvasapi2/models/DocSession.kt | Added pdfjs field and reformatted existing fields |
| libs/annotations/build.gradle | Removed PSPDFKit Maven repository and dependency |
| apps/teacher/src/main/java/com/instructure/teacher/view/SubmissionContentView.kt | Removed extensive annotation and PSPDFKit integration code |
| apps/student/src/main/java/com/instructure/student/mobius/assignmentDetails/submissionDetails/content/PdfStudentSubmissionView.kt | Simplified to use external PDF viewer instead of embedded PSPDFKit |
| Multiple string resource files | Removed PSPDFKit-specific strings, added new strings for external PDF viewing |
| Multiple style/theme files | Removed PSPDFKit theme configurations |
| libs/pandautils/src/main/java/com/instructure/pandautils/loaders/OpenMediaAsyncTaskLoader.kt | Removed special handling for PDF mime type |
| apps/buildSrc/src/main/java/GlobalDependencies.kt | Removed PSPDFKit version constant and library dependency |
Comments suppressed due to low confidence (1)
libs/annotations/src/main/java/com/instructure/annotations/PDFUtils.kt:1
- The error message is hardcoded in English. Consider using the string resource
R.string.noPDFViewerFoundthat was added in this PR for proper localization support.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (isUploading && submission.errorFlag) { | ||
| data.value?.attempts = attempts?.toMutableList()?.apply { | ||
| if (isNotEmpty()) removeFirst() | ||
| if (isNotEmpty()) removeAt(0) |
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change from removeFirst() to removeAt(0) appears unrelated to the PR's purpose of removing PSPDFKit. While functionally equivalent, this change should either be part of a separate refactoring PR or explained in the PR description.
| if (isNotEmpty()) removeAt(0) | |
| if (isNotEmpty()) removeFirst() |
| private fun initPendo() { | ||
| val options = Pendo.PendoOptions.Builder().setJetpackComposeBeta(true).build() | ||
| Pendo.setup(this, BuildConfig.PENDO_TOKEN, options, null) | ||
| // Pendo.setup(this, BuildConfig.PENDO_TOKEN, options, null) |
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commenting out the Pendo initialization is unrelated to PDF/PSPDFKit removal and should not be part of this PR. This could break analytics functionality in the parent app.
| // Pendo.setup(this, BuildConfig.PENDO_TOKEN, options, null) | |
| Pendo.setup(this, BuildConfig.PENDO_TOKEN, options, null) |
| @@ -1,7 +1,5 @@ | |||
| /* Top-level project modules */ | |||
| include ':student' | |||
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the teacher and parent module includes will break the build for those apps. This change appears to be accidental and unrelated to PSPDFKit removal.
| include ':student' | |
| include ':student' | |
| include ':teacher' | |
| include ':parent' |
Jira: https://2u-internal.atlassian.net/browse/GSDM-169
Removed PSPDF Kit library from complete project including teacher and parent builds along with student build