Skip to content

fix data editor in fullscreen - #10098

Merged
Light2Dark merged 4 commits into
mainfrom
sham/fix-portal-ref-data-editor
Jul 8, 2026
Merged

Light2Dark merged 4 commits into
mainfrom
sham/fix-portal-ref-data-editor

Conversation

@Light2Dark

Copy link
Copy Markdown
Member

📝 Summary

Closes #10079

mo.ui.data_editor cell edit overlays were invisible in browser fullscreen (output expand button and slides fullscreen). Double-clicking a cell had no effect; exiting fullscreen could leave a floating overlay artifact.

Root cause: Glide Data Grid renders edit overlays into a portal element. marimo previously used a single global #portal on document.body, which sits outside the fullscreen subtree — the Fullscreen API hides everything not descended from the fullscreen element.

Screen.Recording.2026-07-08.at.11.52.30.AM.mov

Fix:

  • Upgrade @glideapps/glide-data-grid to 6.0.4-alpha24 (adds portalElementRef)
  • Give each data editor its own overlay portal via portalElementRef, rendered to document.body with createPortal
  • On fullscreenchange, reparent the portal into document.fullscreenElement so overlays stay visible; restore to document.body on exit and dismiss any open overlay
  • Remove the global #portal div from index.html (and export template snapshots)

Portaling to document.body (rather than inside the cell output) preserves existing notebook focus behavior — overlay focus does not trigger the marimo cell selection ring.

📋 Pre-Review Checklist

  • For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
  • Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it.
  • Video or media evidence is provided for any visual changes (optional).

✅ Merge Checklist

  • I have read the contributor guidelines.
  • Documentation has been updated where applicable, including docstrings for API changes.
  • Tests have been added for the changes made.

Copilot AI review requested due to automatic review settings July 8, 2026 03:52
@vercel

vercel Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment Jul 8, 2026 4:14am

Request Review

@github-actions github-actions Bot added dependencies bash-focus Area to focus on during release bug bash labels Jul 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes mo.ui.data_editor edit overlays being invisible/uneditable when the editor is displayed in browser fullscreen (output fullscreen + slides fullscreen) by replacing the previous global #portal approach with a per-editor portal that can live inside the active fullscreen subtree.

Changes:

  • Upgrades @glideapps/glide-data-grid to 6.0.4-alpha24 to use portalElementRef.
  • Adds a per-instance portal component (GlideDataEditorPortal) and wires it into GlideDataEditor.
  • Removes the global #portal from frontend/index.html and updates export template snapshots + adds frontend tests.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
frontend/src/plugins/impl/data-editor/glide-portal.tsx Introduces the per-editor overlay portal and fullscreen handling logic.
frontend/src/plugins/impl/data-editor/glide-data-editor.tsx Passes portalElementRef into Glide Data Grid and mounts the new portal component.
frontend/src/plugins/impl/data-editor/__tests__/glide-data-editor.test.tsx Adds unit coverage for portal mounting and fullscreen reparenting behavior.
frontend/package.json Bumps @glideapps/glide-data-grid version used by the frontend.
pnpm-lock.yaml Lockfile updates for the Glide upgrade (including new peer dependency constraints).
frontend/index.html Removes the old global #portal element.
tests/_server/templates/data/index.html Removes the old global #portal element from server template fixtures.
tests/_server/templates/snapshots/export1.txt Snapshot update to reflect template portal removal.
tests/_server/templates/snapshots/export2.txt Snapshot update to reflect template portal removal.
tests/_server/templates/snapshots/export3.txt Snapshot update to reflect template portal removal.
tests/_server/templates/snapshots/export4.txt Snapshot update to reflect template portal removal.
tests/_server/templates/snapshots/export5.txt Snapshot update to reflect template portal removal.
tests/_server/templates/snapshots/export6.txt Snapshot update to reflect template portal removal.
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

Comment thread frontend/src/plugins/impl/data-editor/glide-portal.tsx Outdated
Comment thread frontend/package.json
"@emotion/cache": "^11.14.0",
"@emotion/react": "^11.14.0",
"@glideapps/glide-data-grid": "6.0.4-alpha9",
"@glideapps/glide-data-grid": "6.0.4-alpha24",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This doesn't break today, but we do have 3 versions of marked in this repo. Maybe worth standardizing and bumping all

@Light2Dark Light2Dark added the bug Something isn't working label Jul 8, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

2 issues found and verified against the latest diff

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread frontend/src/plugins/impl/data-editor/glide-portal.tsx
Comment thread frontend/package.json
@Light2Dark
Light2Dark requested a review from kirangadhave July 8, 2026 07:29
@Light2Dark Light2Dark changed the title fix portal ref data editor fix data editor in fullscreen Jul 8, 2026

@kirangadhave kirangadhave left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🚀

@Light2Dark
Light2Dark merged commit ddd52fd into main Jul 8, 2026
44 checks passed
@Light2Dark
Light2Dark deleted the sham/fix-portal-ref-data-editor branch July 8, 2026 15:45
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.23.14-dev25

williambdean pushed a commit to williambdean/marimo that referenced this pull request Sep 6, 2026
## 📝 Summary

<!--
If this PR closes any issues, list them here by number (e.g., Closes
marimo-team#123).

Detail the specific changes made in this pull request. Explain the
problem addressed and how it was resolved. If applicable, provide before
and after comparisons, screenshots, or any relevant details to help
reviewers understand the changes easily.
-->
Closes marimo-team#10079

mo.ui.data_editor cell edit overlays were invisible in browser
fullscreen (output expand button and slides fullscreen). Double-clicking
a cell had no effect; exiting fullscreen could leave a floating overlay
artifact.

Root cause: Glide Data Grid renders edit overlays into a portal element.
marimo previously used a single global #portal on document.body, which
sits outside the fullscreen subtree — the Fullscreen API hides
everything not descended from the fullscreen element.


https://github.com/user-attachments/assets/9c88cfb1-f053-4de5-98d2-4c1d8ef78453

Fix:

- Upgrade @glideapps/glide-data-grid to 6.0.4-alpha24 (adds
portalElementRef)
- Give each data editor its own overlay portal via portalElementRef,
rendered to document.body with createPortal
- On fullscreenchange, reparent the portal into
document.fullscreenElement so overlays stay visible; restore to
document.body on exit and dismiss any open overlay
- Remove the global #portal div from index.html (and export template
snapshots)

Portaling to document.body (rather than inside the cell output)
preserves existing notebook focus behavior — overlay focus does not
trigger the marimo cell selection ring.


## 📋 Pre-Review Checklist
<!-- These checks need to be completed before a PR is reviewed -->

- [ ] For large changes, or changes that affect the public API: this
change was discussed or approved through an issue, on
[Discord](https://marimo.io/discord?ref=pr), or the community
[discussions](https://github.com/marimo-team/marimo/discussions) (Please
provide a link if applicable).
- [ ] Any AI generated code has been reviewed line-by-line by the human
PR author, who stands by it.
- [x] Video or media evidence is provided for any visual changes
(optional). <!-- PR is more likely to be merged if evidence is provided
for changes made -->

## ✅ Merge Checklist

- [x] I have read the [contributor
guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md).
- [ ] Documentation has been updated where applicable, including
docstrings for API changes.
- [x] Tests have been added for the changes made.

This branch was successfully deployed

1 active deployment
Preview — af872ef3 Deployed Jul 8, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bash-focus Area to focus on during release bug bash bug Something isn't working dependencies

Projects

None yet

Development

Successfully merging this pull request may close these issues.

data_editor not editable in full screen mode

3 participants