Rocket.Chat.Electron icon indicating copy to clipboard operation
Rocket.Chat.Electron copied to clipboard

feat(pdf-viewer): resolve issue #2911 – added close and back navigation buttons logic

Open vipulgupta28 opened this issue 1 month ago • 2 comments

Closes #2911

Enhancement: Improved PDF viewer navigation and control

Introduced two key features to improve user experience:

A close button that exits the PDF viewer completely.

A back navigation button that lets users move one page backward within the document.

Summary by CodeRabbit

  • New Features
    • Added a close (cross) button to the document viewer for quick dismissal.
  • Bug Fixes
    • Back button now attempts to exit fullscreen before closing the viewer.
    • Closing the viewer clears displayed content and resets the viewer to avoid lingering documents.
  • Style
    • Header layout updated so the title and close control align neatly and respond better in fullscreen.

vipulgupta28 avatar Nov 12 '25 16:11 vipulgupta28

[!WARNING]

Rate limit exceeded

@vipulgupta28 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 39 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 8d187ccf5801ad7e12ee4abe0bce2cf0ccc83773 and d86a3bae0d1c6333788a8ece3addd2b1d9fac422.

📒 Files selected for processing (1)
  • src/ui/components/ServersView/DocumentViewer.tsx (2 hunks)

Walkthrough

DocumentViewer.tsx adds a src: string property to the global HTMLWebViewElement, changes Back to attempt exiting webview fullscreen before closing, adds a Close (cross) button that exits fullscreen, resets the webview src and documentUrl to about:blank, and restructures the header into a flex layout.

Changes

Cohort / File(s) Summary
DocumentViewer Component
src/ui/components/ServersView/DocumentViewer.tsx
Added src: string to global HTMLWebViewElement. Back button now executes JavaScript in the webview to exit fullscreen and then closes the viewer if not fullscreen. Added close (cross) button which exits fullscreen, sets webview src to about:blank, clears internal documentUrl state, and closes the viewer. Header moved into a flex container and title positioned alongside the cross button.

Sequence Diagram

sequenceDiagram
    participant User
    participant BackBtn as "Back Button"
    participant CloseBtn as "Close Button"
    participant WebView
    participant Viewer as "DocumentViewer"

    User->>BackBtn: click
    alt webview in fullscreen
        BackBtn->>WebView: executeJavaScript("exit fullscreen")
        WebView-->>BackBtn: fullscreen exited
        BackBtn->>Viewer: closeDocumentViewer()
        Viewer-->>User: viewer closed
    else not fullscreen
        BackBtn->>Viewer: closeDocumentViewer()
        Viewer-->>User: viewer closed
    end

    User->>CloseBtn: click
    CloseBtn->>WebView: executeJavaScript("exit fullscreen")
    WebView-->>CloseBtn: (if applicable) fullscreen exited
    CloseBtn->>WebView: set src = "about:blank"
    CloseBtn->>Viewer: set documentUrl = "about:blank"
    CloseBtn->>Viewer: closeDocumentViewer()
    Viewer-->>User: viewer closed

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Inspect executeJavaScript usage and Promise handling / error paths.
  • Verify webview src reset to about:blank properly stops presentation mode and releases resources.
  • Check header layout for accessibility and responsive behavior.

Poem

🐰 I hopped in, small and spry,
Popped fullscreen with one quick try,
A cross to clear the page so bright,
About:blank brings morning light,
Header aligned — all snug and right.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR adds close and back buttons but doesn't fully address the core requirements: preventing presentation mode menu bar loss, disabling auto-presentation mode for subsequent documents, or providing exit mechanisms. Add logic to prevent presentation mode from hiding the menu bar, prevent auto-entering presentation mode on document reload, and ensure proper fullscreen/presentation mode state management.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main changes: adding close and back navigation button logic to the PDF viewer, directly addressing issue #2911.
Out of Scope Changes check ✅ Passed The changes are narrowly scoped to adding UI buttons (close and back navigation) for the document viewer without addressing underlying presentation mode state management issues.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Nov 12 '25 16:11 coderabbitai[bot]

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Nov 12 '25 16:11 CLAassistant