Skip to content

fix: align code selection menu with selection across editors#866

Closed
ML-dev-crypto wants to merge 2 commits intoaccordproject:mainfrom
ML-dev-crypto:fix/code-selection-menu-position
Closed

fix: align code selection menu with selection across editors#866
ML-dev-crypto wants to merge 2 commits intoaccordproject:mainfrom
ML-dev-crypto:fix/code-selection-menu-position

Conversation

@ML-dev-crypto
Copy link
Copy Markdown
Contributor

@ML-dev-crypto ML-dev-crypto commented Apr 1, 2026

Closes #865

Problem

The code selection menu was not correctly aligned with selected text, especially in the JSON editor. The horizontal position was constrained and did not reflect the actual selection position.

Changes

  • Replaced hardcoded X positioning with position.x
  • Used getBoundingClientRect() to convert editor-local coords to viewport coords in the fallback branch
  • Added early return if getDomNode() returns null
  • Improved multi-line selection handling

Testing

  • Verified menu alignment in TemplateMark editor
  • Verified menu alignment in JSON editor
  • Tested multi-line selections

Author Checklist

  • DCO sign-off provided (--signoff)
  • Commit messages follow AP conventional commits format
  • No documentation changes required
  • Merging to main from ML-dev-crypto:fix/code-selection-menu-position

- Replace hardcoded x=225 with position.x so menu follows selection horizontally
- Convert editor-relative coordinates to viewport coordinates by adding editorRect.left/top
- Use getEndPosition for Y so menu appears above last line of multi-line selections
- Use getStartPosition for X so menu anchors to selection start, not end cursor

Signed-off-by: ML-dev-crypto <ml.dev.crypto@example.com>
@ML-dev-crypto ML-dev-crypto requested a review from a team as a code owner April 1, 2026 19:43
Copilot AI review requested due to automatic review settings April 1, 2026 19:43
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 1, 2026

Deploy Preview for ap-template-playground ready!

Name Link
🔨 Latest commit c77844c
🔍 Latest deploy log https://app.netlify.com/projects/ap-template-playground/deploys/69ce8b232db89e0008a6fc3e
😎 Deploy Preview https://deploy-preview-866--ap-template-playground.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the CodeSelectionMenu placement so it follows the actual text selection position (X/Y) across Monaco-based editors, addressing misalignment that was especially visible in the JSON editor.

Changes:

  • Use position.x for the menu’s horizontal placement instead of a fixed X value.
  • Update useCodeSelection to compute menu coordinates using editor selection start/end visible positions and editor container offsets.
  • Improve multi-line selection handling by mixing start (X) and end (Y) selection coordinates.

Comment thread src/components/CodeSelectionMenu.tsx
Comment thread src/components/CodeSelectionMenu.tsx
When .editorwrapper container is not found, the fallback branch was using
editor-local coordinates directly as viewport coordinates, causing the menu
to appear near the page origin.
Fix: use editor.getDomNode().getBoundingClientRect() to get the editor's
viewport offset and add it to startPosition.left and endPosition.top before
clamping. Return early if getDomNode() returns null.

Signed-off-by: Ansh Rai <anshrai331@gmail.com>
@ML-dev-crypto ML-dev-crypto force-pushed the fix/code-selection-menu-position branch from 3b4a6a7 to c77844c Compare April 2, 2026 15:28
@ML-dev-crypto
Copy link
Copy Markdown
Contributor Author

Hi! @DianaLease any feedback

@ML-dev-crypto
Copy link
Copy Markdown
Contributor Author

Hi! @mttrbrts any feedback..????

@mttrbrts
Copy link
Copy Markdown
Member

Thanks @ML-dev-crypto! This is a clean fix for the code selection menu positioning.

Review Notes

  • Good use of early return for null checks
  • Fixed the dead code in fallback branch as noted
  • Properly handles multi-line selections

One small suggestion: the magic number -40 in the Y calculation could benefit from a comment explaining what it represents (menu height offset?), but not blocking.

Ready for approval.


This comment was generated by AI on behalf of @mttrbrts.

@mttrbrts
Copy link
Copy Markdown
Member

Thanks for working on this @ML-dev-crypto, but after testing I'm going to close this PR.

Issues Found

  1. Menu still covers text — The core UX problem remains: the action menu overlays the selected text, making it hard to see what you've selected
  2. Strange horizontal movement — When adjusting the selection left/right, the menu position behaves unexpectedly

For Future PRs

For UI/UX fixes like this, please include:

  • Before/after screenshots or video showing the improvement
  • Multiple test scenarios (different editors, selection sizes, edge cases)

This helps reviewers verify the fix works as intended before spending time on code review.

Feel free to open a new PR if you'd like to take another approach that addresses these issues.


This comment was generated by AI on behalf of @mttrbrts.

@mttrbrts mttrbrts closed this Apr 26, 2026
@ML-dev-crypto
Copy link
Copy Markdown
Contributor Author

Thanks for the detailed feedback @mttrbrts this is really helpful.
I see the issues with the menu overlapping the selection and the unstable horizontal positioning. I’ll revisit the positioning logic to ensure the menu stays outside the selected region (likely anchoring relative to the selection bounds with a proper offset) and stabilise updates during drag.
I’ll validate this across editors and include before/after recordings in the next PR so the improvement is clear.

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.

Code selection menu always appears at fixed X position, ignoring actual selection location

3 participants