Skip to content

fix: stop nested scroll regions from chaining to the notebook - #10807

Merged
mscolnick merged 1 commit into
mainfrom
kg/css-project-remaining-work
Sep 15, 2026
Merged

mscolnick merged 1 commit into
mainfrom
kg/css-project-remaining-work

Conversation

@kirangadhave

@kirangadhave kirangadhave commented Sep 11, 2026

Copy link
Copy Markdown
Member

📝 Summary

Add overscroll-behavior: contain to the output area, the console output area, the JSON tree output, and the table scroll wrapper. A wheel or trackpad gesture that reaches the edge of one of these regions no longer scrolls the whole notebook.

Closes MO-6579

Add overscroll-behavior: contain to output, console, JSON, and table scroll containers so a wheel or trackpad gesture at their edge no longer scrolls the whole notebook.
@vercel

vercel Bot commented Sep 11, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated
marimo-docs Ready Ready Preview Sep 14, 2026 11:22pm UTC

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@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.

No issues found across 4 files

Re-trigger cubic

@kirangadhave kirangadhave added the internal A refactor or improvement that is not user facing label Sep 11, 2026
@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Coverage Report for ./frontend

Status Category Percentage Covered / Total
🔵 Lines 57.72% 21973 / 38063
🔵 Statements 57.66% 22359 / 38774
🔵 Functions 50.99% 4971 / 9748
🔵 Branches 51.16% 11450 / 22377
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
frontend/src/components/editor/output/Outputs.css 0% 0% 0% 0%
frontend/src/components/ui/table.tsx 91.66% 100% 75% 91.66% 48-53, 102-107
frontend/src/css/app/Cell.css 0% 0% 0% 0%
Generated in workflow #21431 for commit 62ccafc by the Vitest Coverage Report Action

@kirangadhave
kirangadhave marked this pull request as ready for review September 11, 2026 20:31
Copilot AI lite review requested due to automatic review settings September 11, 2026 20:31

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.

🔵 Needs a closer look

Two moderate findings remain regarding scratchpad scroll chaining and tooltip positioning under layout containment.

Pull request overview

Improves notebook scrolling stability and isolates cell layout recalculation.

Changes:

  • Contains overscroll in outputs, console areas, JSON trees, and tables.
  • Reserves stable scrollbar space on #App.
  • Adds layout containment to notebook cells.
File summaries
File Summary
frontend/src/css/app/Cell.css Adds console overscroll containment and cell layout containment. Two moderate findings remain (1 vote each).
frontend/src/css/app/App.css Reserves the scrollbar gutter.
frontend/src/components/ui/table.tsx Prevents table scroll chaining.
frontend/src/components/editor/output/Outputs.css Contains JSON output overscroll.
Review details

Suppressed comments (2)

frontend/src/css/app/Cell.css:88

  • This declaration is nested under .interactive, so it only applies to console output inside an interactive notebook cell. The scratchpad renders ConsoleOutput with its own overflow-auto (components/scratchpad/scratchpad.tsx:287-297) but has no .interactive ancestor, so reaching that pane's edge can still chain the wheel gesture to the parent/notebook. Put overscroll-behavior: contain on the base .console-output-area rule while keeping the edit-mode sizing rules nested here.
      overscroll-behavior: contain;

frontend/src/css/app/Cell.css:18

  • contain: layout makes the cell a containing block for positioned descendants. The data-table hover tooltip keeps its fixed trigger in the table subtree (components/data-table/hover-tooltip/hover-tooltip.tsx:37-48) but feeds it viewport getBoundingClientRect() coordinates, so hover tooltips in notebook tables will be offset by the cell's position instead of remaining viewport-fixed. Please either exempt/relocate this containment for such cells or portal the trigger outside the contained cell.
  contain: layout;
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@kirangadhave
kirangadhave marked this pull request as draft September 11, 2026 23:34
@kirangadhave
kirangadhave force-pushed the kg/css-project-remaining-work branch from 8f1257a to 60c7090 Compare September 14, 2026 22:49
@kirangadhave
kirangadhave marked this pull request as ready for review September 14, 2026 22:50
@kirangadhave kirangadhave changed the title fix: contain nested scrolling, reserve the scrollbar gutter, and contain cell layout fix: contain nested scrolling and cell layout Sep 14, 2026
@kirangadhave kirangadhave changed the title fix: contain nested scrolling and cell layout fix: stop nested scroll regions from chaining to the notebook Sep 14, 2026
@kirangadhave
kirangadhave force-pushed the kg/css-project-remaining-work branch from 60c7090 to 62ccafc Compare September 14, 2026 23:21
@mscolnick
mscolnick merged commit e750ab9 into main Sep 15, 2026
31 checks passed
@mscolnick
mscolnick deleted the kg/css-project-remaining-work branch September 15, 2026 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal A refactor or improvement that is not user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants