Skip to content

fix(library): keep floating editor surfaces visible in fullscreen - #839

Closed
FelixTJDietrich wants to merge 1 commit into
mainfrom
fix/fullscreen-portals
Closed

fix(library): keep floating editor surfaces visible in fullscreen#839
FelixTJDietrich wants to merge 1 commit into
mainfrom
fix/fullscreen-portals

Conversation

@FelixTJDietrich

@FelixTJDietrich FelixTJDietrich commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Route editor-owned floating surfaces through an editor-local portal root when element-level fullscreen excludes the document body. This keeps popovers, selects, tooltips, color controls, and palette drag previews visible and interactive in subtree fullscreen without a global shared root or host-specific z-index workarounds.

Release note

Popovers, selects, tooltips, color pickers, and drag previews remain visible and interactive when an embedded editor enters fullscreen.

Implementation notes

  • Each editor declaratively owns one lifecycle-bound, layout-neutral portal root.
  • Surfaces continue to portal to document.body whenever body remains in the fullscreen subtree, preserving normal embedding and host dialog layering.
  • Subtree fullscreen uses the editor-local root only when body is excluded by the browser's fullscreen top layer.
  • The fullscreen subscription re-portals already-open surfaces when the Fullscreen API state changes.
  • Multiple editors remain isolated: only the editor contained by document.fullscreenElement uses its local root.
  • The shared UI tooltip now accepts Base UI's native portal container option instead of adding a parallel portal implementation.

Steps for testing

  1. Run pnpm lint && pnpm format:check && pnpm build && pnpm test.
  2. Run pnpm exec playwright test standalone/webapp/tests/e2e/fullscreen-portals.spec.ts.
  3. In the local editor, fullscreen the editor subtree, edit a node, hover a chrome control, and drag a palette item; confirm the popover, tooltip, and drag preview stay visible.
  4. Exit fullscreen and confirm floating surfaces return to the document portal.

Screenshots / screencasts

Not attached: this fixes top-layer behavior rather than static appearance. The added Playwright test exercises the behavior in a real browser.

Checklist

  • Linked to a related issue (not applicable)
  • Added a changeset whose summary is written in the user's voice
  • PR title's Conventional Commit type matches the kind of change
  • Tests added or updated
  • Ran pnpm lint && pnpm format:check && pnpm build && pnpm test locally — green
  • Documentation updated (not applicable)
  • Screenshots or screencasts attached (behavior covered by Playwright)

@Claudia-Anthropica Claudia-Anthropica left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@FelixTJDietrich The editor-local portal routing is coherent for ordinary document-tree embeds, and the new coverage exercises those transitions. However, the containment check breaks when the embeddable editor is mounted inside a shadow root: fullscreen state is retargeted to the shadow host, so surfaces still portal outside the fullscreen subtree. Please resolve fullscreen state from the portal root's own tree and add shadow-DOM coverage.

() => null
)

if (portalContainer && fullscreenElement?.contains(portalContainer)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@FelixTJDietrich [medium] This check fails for editors mounted inside a shadow root. When the inner .apollon-editor enters fullscreen, document.fullscreenElement is retargeted to the shadow host, but host.contains(portalContainer) is false because shadow descendants belong to a separate node tree. The hook consequently returns document.body, leaving all floating surfaces outside the fullscreen subtree. Resolve the fullscreen element from the portal container's own getRootNode()—handling both Document and ShadowRoot—and add a shadow-root regression test.

🤖 Prompt for AI agents

In library/lib/components/ui/portalContainer.tsx, fullscreen containment is checked only against document.fullscreenElement, which is retargeted across shadow boundaries and leaves shadow-DOM editors portaling to document.body. Resolve the fullscreen element from the portal container's own root (Document or ShadowRoot) and cover a shadow-root mount in tests.

@github-project-automation github-project-automation Bot moved this from Backlog to In progress in Apollon Development Jul 31, 2026
@FelixTJDietrich
FelixTJDietrich force-pushed the fix/fullscreen-portals branch from f14aec5 to 3aad60a Compare July 31, 2026 07:00

@Claudia-Anthropica Claudia-Anthropica left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@FelixTJDietrich The current head still resolves fullscreen exclusively through document.fullscreenElement. For an editor mounted in a shadow root, that value is retargeted to the shadow host, whose contains(portalContainer) check fails, so the prior medium finding remains unresolved and requires shadow-root-aware lookup plus regression coverage.

@FelixTJDietrich
FelixTJDietrich force-pushed the fix/fullscreen-portals branch from 3aad60a to fafdc2c Compare July 31, 2026 07:08

@Claudia-Anthropica Claudia-Anthropica left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@FelixTJDietrich The exact head still resolves fullscreen solely through document.fullscreenElement. For a shadow-DOM embed, that value is retargeted to the shadow host, whose contains(portalContainer) check fails, leaving floating surfaces outside the fullscreen subtree. Please use the portal container's Document or ShadowRoot fullscreen state and add shadow-root regression coverage.

@FelixTJDietrich

Copy link
Copy Markdown
Contributor Author

Superseded by #841, which carries this work plus the rest of the fixes the Artemis integration needs on a single branch off main. Splitting the integration across two branches meant neither could be reviewed or released on its own. Closing in favour of that one.

@github-project-automation github-project-automation Bot moved this from In progress to Done in Apollon Development Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants