Skip to content

Update PDF viewer to use toolbar layout and remove screenshot prevention #919

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Update PDF viewer to use toolbar layout and remove screenshot prevention #919

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f44aaa4
fix
PruthiviRaj27 Jul 23, 2025
c3ce482
fix
PruthiviRaj27 Jul 23, 2025
56fb042
fix
PruthiviRaj27 Jul 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions course/src/main/java/in/testpress/course/ui/PdfViewerActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import `in`.testpress.course.util.PDFDownloadManager
import `in`.testpress.course.util.PdfDownloadListener
import android.os.Bundle
import android.view.View
import `in`.testpress.ui.BaseToolBarActivity
import android.view.WindowManager
import androidx.appcompat.app.AppCompatActivity

class PdfViewerActivity : AppCompatActivity(), PdfDownloadListener, DisplayPDFListener {
class PdfViewerActivity : BaseToolBarActivity(), PdfDownloadListener, DisplayPDFListener {
Copy link

@gemini-code-assist gemini-code-assist bot Jul 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Changing the base class to BaseToolBarActivity for a screen that is meant to be fullscreen and has no toolbar seems counter-intuitive. This change necessitates adding a gone toolbar to the layout file, which feels like a workaround and adds an unnecessary view.

Please consider if a more appropriate base class is available to extend, for example, one that provides common functionality without enforcing a toolbar.

If BaseToolBarActivity must be used for other essential logic it contains, it might indicate that the responsibilities in your base activities could be separated more cleanly (e.g., toolbar logic in one class, other common logic in another). This would improve long-term maintainability.

Copy link
Contributor Author

@PruthiviRaj27 PruthiviRaj27 Jul 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gemini-code-assist BaseToolBarActivity is the base activity for all other activities, providing common theme settings and privacy-related implementations.
For this specific activity, we don’t need the toolbar UI, but we still require the underlying privacy logic.
To avoid duplicating code, we continue to extend BaseToolBarActivity and simply hide the toolbar view.

Copy link

@gemini-code-assist gemini-code-assist bot Jul 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification, @PruthiviRaj27. I understand that BaseToolBarActivity serves as a common base for shared theme settings and privacy logic across your activities, and extending it here avoids code duplication for those functionalities.

While this approach works, it does introduce a slight architectural compromise by requiring a gone toolbar in the layout for an activity that doesn't visually use it. This can make the code less intuitive and potentially harder to maintain if BaseToolBarActivity accumulates more toolbar-specific logic in the future.

For long-term maintainability and clearer separation of concerns, consider refactoring your activity hierarchy. You could introduce a more fundamental BaseActivity (or similar) that handles only the common theme and privacy logic, and then have BaseToolBarActivity extend that class. This would allow PdfViewerActivity to extend the truly minimal base class, avoiding the need for a hidden toolbar and making the purpose of each base class more explicit.

private lateinit var binding: LayoutPdfViewerBinding
private lateinit var pdfDownloadManager: PDFDownloadManager

Expand All @@ -25,7 +25,6 @@ class PdfViewerActivity : AppCompatActivity(), PdfDownloadListener, DisplayPDFLi

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
disableScreenShot()
binding = LayoutPdfViewerBinding.inflate(layoutInflater)
setContentView(binding.root)
hideStatusBar()
Expand All @@ -38,13 +37,6 @@ class PdfViewerActivity : AppCompatActivity(), PdfDownloadListener, DisplayPDFLi
}
}

private fun disableScreenShot() {
window.setFlags(
WindowManager.LayoutParams.FLAG_SECURE,
WindowManager.LayoutParams.FLAG_SECURE
)
}

private fun hideStatusBar() {
window.decorView.systemUiVisibility = View.SYSTEM_UI_FLAG_FULLSCREEN
this.window.setFlags(
Expand Down
5 changes: 5 additions & 0 deletions course/src/main/res/layout/layout_pdf_viewer.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
xmlns:tools="http://schemas.android.com/tools"
android:orientation="vertical">

<!-- We are displaying the PDF content in fullscreen, so the toolbar is hidden.
However, the toolbar view is still required because this Activity extends BaseToolBarActivity. -->
<include layout="@layout/testpress_toolbar"
android:visibility="gone" />

<com.github.barteksc.pdfviewer.PDFView
android:id="@+id/pdfView"
android:layout_width="0dp"
Expand Down