Skip to content

fix(popup): portal annotation popup toolbar and reply so it stays upright on rotate#749

Merged
kduncanhsu merged 3 commits into
box:masterfrom
kduncanhsu:fix/popup-toolbar-and-reply-rotate
May 14, 2026
Merged

fix(popup): portal annotation popup toolbar and reply so it stays upright on rotate#749
kduncanhsu merged 3 commits into
box:masterfrom
kduncanhsu:fix/popup-toolbar-and-reply-rotate

Conversation

@kduncanhsu
Copy link
Copy Markdown
Contributor

@kduncanhsu kduncanhsu commented May 12, 2026

Summary

  • Portal PopupDrawingToolbar and PopupReply to the popupPortalEl container (outside the rotating .ba-Layer) so they remain upright when the page is rotated, matching the existing PopupV2 approach
  • Fix PopupDrawingToolbar Popper.js options so the toolbar correctly follows its annotation on scroll and doesn't stick to the viewport edge
  • Prevent DeselectListener from deselecting annotations when clicking portaled popup elements

Details

When the page is rotated, annotation layers receive transform: rotate(Xdeg), causing all children to rotate. PopupV2 already solved this via portaling, but PopupDrawingToolbar and PopupReply still rotated with their layers.

Portaling changes:

  • Pass popupPortalEl through DrawingManager → DrawingAnnotationsContainer → DrawingAnnotations for both DocumentAnnotator and ImageAnnotator
  • Wrap PopupDrawingToolbar and PopupReply in ReactDOM.createPortal() targeting popupPortalEl
  • Return null if popupPortalEl is unavailable (no fallback to inline rendering)

Stacking context fix:

  • Add position: relative; z-index: 10; pointer-events: none to .ba-PopupPortal so portaled popups render above the drawing layer's full-page click overlay
  • Set pointer-events: auto on .ba-Popup children so they receive clicks

Popper.js behavioral fixes:

  • Remove scroll: false from eventListeners modifier — now that the toolbar is outside the scroll container, Popper must actively reposition on scroll
  • Remove altAxis: true from preventOverflow — this was clamping the toolbar to the viewport edge vertically instead of letting it scroll away with the annotation

Deselect fix:

  • Add .ba-Popup to the DeselectListener closest-check so clicks on portaled popups don't trigger annotation deselection

Test plan

  • Unit tests pass (160 tests across 9 suites)
  • Rotate a PDF 90/180/270°, enter drawing mode, draw a shape — toolbar stays upright and positioned near the drawing
  • Click "Add Comment" — reply popup stays upright
  • Scroll while toolbar is visible — toolbar follows the annotation and disappears off-screen with it
  • Test the same on images (not just PDFs)
  • Click outside a popup — annotation correctly deselects
  • Click popup buttons (undo/redo/delete/Add Comment) — all register correctly

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Duncan Hsu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@kduncanhsu kduncanhsu force-pushed the fix/popup-toolbar-and-reply-rotate branch 2 times, most recently from b3d3cb2 to 39f1a7e Compare May 12, 2026 23:28
@kduncanhsu kduncanhsu marked this pull request as ready for review May 12, 2026 23:46
@kduncanhsu kduncanhsu requested a review from a team as a code owner May 12, 2026 23:46
Comment thread src/common/BaseAnnotator.scss
@kduncanhsu kduncanhsu force-pushed the fix/popup-toolbar-and-reply-rotate branch from c67e8cc to 7391fed Compare May 13, 2026 23:53
@kduncanhsu kduncanhsu force-pushed the fix/popup-toolbar-and-reply-rotate branch from 7391fed to d0559fb Compare May 14, 2026 17:45
@kduncanhsu kduncanhsu merged commit 06fe595 into box:master May 14, 2026
6 of 7 checks passed
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.

3 participants