Skip to content

fix: user context menu opens in an incorrect position on screen#8775

Open
Maurizio-dcl wants to merge 3 commits into
devfrom
fix/8768-user-context-menu-opens-in-an-incorrect-position-on-screen
Open

fix: user context menu opens in an incorrect position on screen#8775
Maurizio-dcl wants to merge 3 commits into
devfrom
fix/8768-user-context-menu-opens-in-an-incorrect-position-on-screen

Conversation

@Maurizio-dcl
Copy link
Copy Markdown
Contributor

@Maurizio-dcl Maurizio-dcl commented May 14, 2026

Pull Request Description

Fix #8768 and #8767

What does this PR change?

Moved backgroundWorldRect calculation from OnViewInstantiated() to OnBeforeViewShow() in GenericContextMenuController

Test Instructions

These steps need to be verified in both windowed and fullscreen mode, and on different resolutions as well.

Test Steps

  1. Open the minimap context menu
  2. Verify the menu opens in the correct place (directly right of the three-dots button)
  3. Close the minimap context menu
  4. Open a user's context menu (right-click on their avatar)
  5. Verify the menu opens in the correct position (close to the cursor / avatar and on the left or right depending on how close the avatar is to the screen borders)

Quality Checklist

  • Changes have been tested locally
  • Documentation has been updated (if required)
  • Performance impact has been considered
  • For SDK features: Test scene is included

Code Review Reference

Please review our Branch & PR Standards before submitting. It explains the automated review flow, QA/DEV approval requirements, and what each label does — especially useful for first-time contributors.

- Moved backgroundWorldRect calculation from OnViewInstantiated() to OnBeforeViewShow()
@Maurizio-dcl Maurizio-dcl self-assigned this May 14, 2026
@Maurizio-dcl Maurizio-dcl requested review from a team as code owners May 14, 2026 13:18
@github-actions github-actions Bot requested a review from anicalbano May 14, 2026 13:18
@claude

This comment has been minimized.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Auto-approved by Claude — simple fix/chore with no blocking issues. QA approval is still required.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 14, 2026

Claude encountered an error —— View job


Review

  • Gather context and read changed files
  • Analyze the fix and root cause
  • Check for blocking issues
  • Post final review

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 21, 2026

Claude finished @Maurizio-dcl's task in 1m 52s —— View job


Review

  • Gather context and read changed files
  • Analyze the fix and root cause
  • Check for blocking issues
  • Post final review

Root-Cause Assessment

Problem: Context menu opens at incorrect positions on screen, across resolutions and windowed/fullscreen modes.

Root cause vs symptom: This is a root-cause fix. backgroundWorldRect (used as the clamping boundary in GetControlsPosition, GetFallbackDirections, AdjustSubmenuPositionToFitBounds) was computed once in OnViewInstantiated(), which fires only at view creation. Any resolution or window-mode change between instantiation and the next show would leave this rect stale. Moving the calculation to OnBeforeViewShow() ensures it reflects real screen layout on every open.

The secondary change — replacing BackgroundCloseButton.GetComponent<RectTransform>() with viewRectTransform as the rect source — is also correct: viewRectTransform is the full-screen canvas root, which is what the boundary should represent.


Blocking Issues

None found.

Behavioral note (informational, not blocking): Removing BackgroundCloseButton also removed viewInstance!.BackgroundCloseButton.Button.OnClickAsync(ct) from WaitForCloseIntentAsync (line 890). This means clicking outside the menu no longer triggers close via this path. Closing now relies solely on internalCloseTask and callers' inputData.CloseTask. This is committed intentionally (d7d79c15 Removed background close button entirely) and the author states local testing passed — but QA should verify click-outside-to-close still works for both the minimap and user context menus.


Summary

Clean, minimal fix. Stale-rect root cause correctly addressed. Removal of BackgroundCloseButton is deliberate. PASS.


REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Single-file UI lifecycle change moving a RectTransform calculation between two controller lifecycle methods, plus removal of a full-screen close-button overlay from the view and prefab.
QA_REQUIRED: YES

@m3taphysics
Copy link
Copy Markdown
Collaborator

PR #8775, run #26229057670

Builds: Windows change, Windows baseline, macOS change, macOS baseline

Framework 13 i7

Metric Change Baseline Delta Improvement
Samples 2700 2700
CPU average 33.3 ms 33.3 ms 0.0 ms -0.0%
CPU 1% worst 33.6 ms 33.6 ms 0.0 ms -0.1%
CPU 0.1% worst 35.1 ms 34.7 ms 0.5 ms -1.3%
GPU average 7.8 ms 7.5 ms 0.3 ms -3.4% 🔴
GPU 1% worst 20.2 ms 20.5 ms -0.3 ms 1.5%
GPU 0.1% worst 23.9 ms 24.9 ms -1.0 ms 3.9% 🟢

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.

[QA] Explorer | User context menu opens in an incorrect position on screen

3 participants