Skip to content

Conversation

cameronaaron
Copy link

No description provided.

cameronaaron and others added 3 commits June 27, 2025 21:35
- Implemented vertical scrolling mode to display all PDF pages in a continuous scroll view.
- Added styles for vertical layout and page number indicators in CSS.
- Enhanced JavaScript logic for rendering all pages and tracking current page based on scroll position.
- Introduced a toggle button in the toolbar for switching between vertical scroll mode and traditional page mode.
- Updated Android integration to support vertical scroll mode and preserve state during mode switching.
- Added new vector drawable for the vertical scroll toggle icon.
@cameronaaron cameronaaron marked this pull request as ready for review June 29, 2025 20:58
@Copilot Copilot AI review requested due to automatic review settings June 29, 2025 20:58
Copy link

@Copilot Copilot AI left a 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 introduces a vertical scrolling layout in the web viewer and a "keep screen on" (wake lock) toggle in the Android PdfViewer.

  • Adds JavaScript logic and CSS styles for vertical scroll mode, with page rendering and scroll‐based page tracking.
  • Integrates menu actions and Java<->JavaScript interfaces for toggling vertical scroll and wake lock, including state persistence.

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
viewer/js/index.js Implements vertical scroll mode, rendering, throttling, and JS APIs
viewer/css/pdf_viewer.css Styles for the vertical scroll container and pages; hides horizontal overflow
app/src/main/res/values/strings.xml New string resources for "Vertical scroll" and "Keep screen on"
app/src/main/res/menu/pdf_viewer.xml Menu entries for toggling vertical scroll and wake lock
app/src/main/res/drawable/ic_view_agenda_24dp.xml Icon for vertical scroll toggle
app/src/main/res/drawable/ic_screen_lock_portrait_24dp.xml Icon for keep screen on toggle
app/src/main/java/app/grapheneos/pdfviewer/PdfViewer.java Manages wake lock, JS interfaces, menu handling, and state
app/src/main/AndroidManifest.xml Declares WAKE_LOCK permission
Comments suppressed due to low confidence (2)

app/src/main/res/menu/pdf_viewer.xml:50

  • This toggle menu item should be checkable and reflect the current vertical scroll state. Add 'android:checkable="true"' in XML and call setChecked(isVerticalScrollMode) in onPrepareOptionsMenu.
        app:showAsAction="ifRoom" />

viewer/js/index.js:168

  • New behavior for vertical scroll rendering is added here. Consider adding unit or integration tests to cover renderAllPages, scroll listener setup, and page change notifications.
async function renderAllPages(zoom = false) {

import android.util.Log;
import android.view.Gravity;
import android.view.Menu;
import android.view.MenuInflater;
import android.view.MenuItem;
import android.view.View;
import android.view.WindowManager;
Copy link
Preview

Copilot AI Jun 29, 2025

Choose a reason for hiding this comment

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

The import 'android.view.WindowManager' is unused. Consider removing it to clean up imports.

Suggested change
import android.view.WindowManager;

Copilot uses AI. Check for mistakes.

padding: 20px;
width: 100%;
min-height: 100vh;
`;
Copy link
Preview

Copilot AI Jun 29, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider moving these style rules into the CSS file under '#all-pages-container' to avoid duplication and simplify maintenance.

Copilot uses AI. Check for mistakes.

@cameronaaron
Copy link
Author

copilot literally nobody asked for your opinion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant