Skip to content

fix(super-editor): keep the context menu within the editor viewport and scroll area - #3839

Open
hitpopdimestop wants to merge 5 commits into
superdoc:v1from
hitpopdimestop:fix/context-menu-viewport-clamp
Open

fix(super-editor): keep the context menu within the editor viewport and scroll area#3839
hitpopdimestop wants to merge 5 commits into
superdoc:v1from
hitpopdimestop:fix/context-menu-viewport-clamp

Conversation

@hitpopdimestop

@hitpopdimestop hitpopdimestop commented Jul 21, 2026

Copy link
Copy Markdown

What & why

The right-click context menu opened at the raw click point with no clamping, so right-clicking near the right or bottom edge pushed it partly off-screen. Even a plain viewport clamp would leave it under the editor scroll container's scrollbar.

After the menu renders (so its size is known), it now clamps its position to the viewport intersected with the editor scroll container's content boxclientWidth/clientHeight exclude that container's scrollbar, so the menu never lands under it. If the menu is larger than the available space on an axis, it renders as-is rather than trading one clipped edge for another.

The positioning math lives in a small pure helper (menu-position.js) so it's unit-testable independently of the Vue component.

Screenshots

Before — the context menu is clipped at the right edge:
Знімок екрана 2026-07-21 о 15 30 41

After — the context menu stays fully on-screen, clear of the scrollbar:
Знімок екрана 2026-07-21 о 15 30 28

Test plan

  • menu-position.test.js: bounds clamp (right / bottom / left / top overflow), the scroll-container scrollbar case, the oversized "render as-is" case, and resolveMenuBounds (viewport-only + scroll-container intersection).
  • Context-menu test suite: 170 passing; Prettier clean. (The repo's ESLint config ignores .vue and test files; menu-position.js lints clean.)
  • Verified live in the dev app: right-clicking near the right content edge lands the menu at right = content-edge − 8px, clear of the 15px scrollbar.

@hitpopdimestop
hitpopdimestop requested a review from a team as a code owner July 21, 2026 12:43
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@qodo-code-review

qodo-code-review Bot commented Aug 14, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)



View medium (1)
🟠 **Medium**
1. Fallback disables clamping 🐞
Description
When getEditorSurfaceElement(props.editor) is null, ContextMenu.vue falls back to passing
menuRef.value into resolveMenuBounds, which causes the bounds to collapse to the menu’s own rect
(because the menu has overflow: hidden). This makes clampMenuPositionToBounds treat the menu as
“too large to fit” and skip clamping, so the menu can still render off-screen/under scrollbars in
that fallback scenario.
Code

packages/super-editor/src/editors/v1/components/context-menu/ContextMenu.vue[R591-592]

+      const bounds = resolveMenuBounds(getEditorSurfaceElement(props.editor) ?? menuRef.value, window);
+      menuPosition.value = clampMenuPositionToBounds(menuPosition.value, menuRect, bounds);
Evidence
The new fallback passes menuRef.value into resolveMenuBounds. Since .context-menu sets
overflow: hidden, resolveMenuBounds treats the menu itself as a clipping ancestor and intersects
bounds with its own client box. That makes bounds ~ rect, causing clampMenuPositionToBounds’s
fitsX/fitsY checks to fail (because it requires room for 2*gutter), so it skips clamping on
those axes. Separately, the context-menu plugin code demonstrates the editor surface can be absent
in some flows, making this fallback reachable.

packages/super-editor/src/editors/v1/components/context-menu/ContextMenu.vue[558-593]
packages/super-editor/src/editors/v1/components/context-menu/ContextMenu.vue[685-697]
packages/super-editor/src/editors/v1/components/context-menu/menu-position.js[1-40]
packages/super-editor/src/editors/v1/components/context-menu/menu-position.js[52-70]
packages/super-editor/src/editors/v1/extensions/context-menu/context-menu.js[217-251]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ContextMenu.vue` calls `resolveMenuBounds(getEditorSurfaceElement(props.editor) ?? menuRef.value, window)`. If the editor surface element is unavailable, passing the menu element itself causes `resolveMenuBounds` to intersect the bounds with the menu’s own clipping box (the menu has `overflow: hidden`), collapsing `bounds` to approximately the menu’s rect.

`clampMenuPositionToBounds` then computes `fitsX/fitsY` against `bounds - 2*gutter`, which typically fails when bounds == rect, so it skips clamping and leaves the menu potentially off-screen.

## Issue Context
The editor surface lookup can be null in some flows; the context-menu plugin explicitly allows operation without a surface (it only positions relative to a surface when available).

## Fix
- Do not pass the menu element itself as the anchor to `resolveMenuBounds`.
- Use an ancestor/container instead (e.g. `menuRef.value?.parentElement`, or `menuRef.value?.closest('.super-editor')`, or if those are absent, pass `null` to fall back to viewport-only bounds).
- Add/adjust a unit test to cover the “surface is null” path so clamping still occurs.

## Fix Focus Areas
- packages/super-editor/src/editors/v1/components/context-menu/ContextMenu.vue[588-593]
- packages/super-editor/src/editors/v1/components/context-menu/tests/menu-position.test.js[1-155]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Context
Review mode: 🚀 Fast: This push only adds a localized test case and adjusts test mocks in one file, with no runtime or contract changes.

Tip of the day
💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗


Previous review results

Review updated until commit b782a35 🚀 Fast

Results up to commit 036c523 🚀 Fast


No changes from previous review

Results up to commit a5a4016 ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 📜 Skill insights (0)



View medium (1)
🟠 **Medium**
1. Fallback disables clamping 🐞
Description
When getEditorSurfaceElement(props.editor) is null, ContextMenu.vue falls back to passing
menuRef.value into resolveMenuBounds, which causes the bounds to collapse to the menu’s own rect
(because the menu has overflow: hidden). This makes clampMenuPositionToBounds treat the menu as
“too large to fit” and skip clamping, so the menu can still render off-screen/under scrollbars in
that fallback scenario.
Code

packages/super-editor/src/editors/v1/components/context-menu/ContextMenu.vue[R591-592]

+      const bounds = resolveMenuBounds(getEditorSurfaceElement(props.editor) ?? menuRef.value, window);
+      menuPosition.value = clampMenuPositionToBounds(menuPosition.value, menuRect, bounds);
Evidence
The new fallback passes menuRef.value into resolveMenuBounds. Since .context-menu sets
overflow: hidden, resolveMenuBounds treats the menu itself as a clipping ancestor and intersects
bounds with its own client box. That makes bounds ~ rect, causing clampMenuPositionToBounds’s
fitsX/fitsY checks to fail (because it requires room for 2*gutter), so it skips clamping on
those axes. Separately, the context-menu plugin code demonstrates the editor surface can be absent
in some flows, making this fallback reachable.

packages/super-editor/src/editors/v1/components/context-menu/ContextMenu.vue[558-593]
packages/super-editor/src/editors/v1/components/context-menu/ContextMenu.vue[685-697]
packages/super-editor/src/editors/v1/components/context-menu/menu-position.js[1-40]
packages/super-editor/src/editors/v1/components/context-menu/menu-position.js[52-70]
packages/super-editor/src/editors/v1/extensions/context-menu/context-menu.js[217-251]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ContextMenu.vue` calls `resolveMenuBounds(getEditorSurfaceElement(props.editor) ?? menuRef.value, window)`. If the editor surface element is unavailable, passing the menu element itself causes `resolveMenuBounds` to intersect the bounds with the menu’s own clipping box (the menu has `overflow: hidden`), collapsing `bounds` to approximately the menu’s rect.

`clampMenuPositionToBounds` then computes `fitsX/fitsY` against `bounds - 2*gutter`, which typically fails when bounds == rect, so it skips clamping and leaves the menu potentially off-screen.

## Issue Context
The editor surface lookup can be null in some flows; the context-menu plugin explicitly allows operation without a surface (it only positions relative to a surface when available).

## Fix
- Do not pass the menu element itself as the anchor to `resolveMenuBounds`.
- Use an ancestor/container instead (e.g. `menuRef.value?.parentElement`, or `menuRef.value?.closest('.super-editor')`, or if those are absent, pass `null` to fall back to viewport-only bounds).
- Add/adjust a unit test to cover the “surface is null” path so clamping still occurs.

## Fix Focus Areas
- packages/super-editor/src/editors/v1/components/context-menu/ContextMenu.vue[588-593]
- packages/super-editor/src/editors/v1/components/context-menu/tests/menu-position.test.js[1-155]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Results up to commit b50b45f 🚀 Fast


No changes from previous review

Results up to commit 9feb20b 🚀 Fast


No changes from previous review

Powered by Qodo

hitpopdimestop and others added 2 commits August 13, 2026 21:45
The context menu opened at the raw click point with no clamping, so
right-clicking near the right/bottom edge pushed it partly off-screen, and
even a viewport clamp would render it under the scroll container scrollbar.
Clamp the menu to the viewport intersected with the editor scroll container's
content box (which excludes its scrollbar) after it renders.
@caio-pizzol
caio-pizzol force-pushed the fix/context-menu-viewport-clamp branch from 036c523 to a5a4016 Compare August 14, 2026 01:15

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 4 files

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

Re-trigger cubic

Comment thread packages/super-editor/src/editors/v1/components/context-menu/menu-position.js Outdated
Comment thread packages/super-editor/src/editors/v1/components/context-menu/ContextMenu.vue Outdated
Comment thread packages/super-editor/src/editors/v1/components/context-menu/ContextMenu.vue Outdated

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 4 files (changes from recent commits).

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

Re-trigger cubic

@caio-pizzol
caio-pizzol enabled auto-merge (squash) August 14, 2026 09:44
@caio-pizzol

Copy link
Copy Markdown
Contributor

Thanks again for the contribution! We revisited this PR and decided to merge it. I pushed a few updates to improve the fix and its test coverage, and I also opened a separate PR for v2.

@caio-pizzol
caio-pizzol disabled auto-merge August 14, 2026 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants