[Popover] Fix popover content is outside the visible space

[Marvin Klein (MarvinKlein1508)]

Pull Request

📖 Description

This PR adds an additional check to FluentPopover to make sure that the content of the Popover won't be displayed outside of the visualViewport.

Before:

After:

🎫 Issues

Fix #5244

📑 Test Plan

✅ Checklist

General

• I have added tests for my changes.
• I have tested my changes.
• I have updated the project documentation to reflect my changes.
• I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

• I have added a new component
• I have added Unit Tests for my new component
• I have modified an existing component
• I have validated the Unit Tests for an existing component

⏭ Next Steps

I tested this on Firefox, Edge and Chrome on PC and on Safari on iOS. Please make sure to test this on Android as well.

[Copilot]

🟡 Changes recommended

The new clamp calculations incorrectly use dialogWidth (including offsetHorizontal) which can double-count offsets and mis-clamp (especially for negative offsets).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the Popover positioning logic in FluentPopover.ts to prevent the popover dialog from rendering outside the browser’s visualViewport, addressing off-screen/partially visible popovers (Issue #5244).

Changes:

• Adds horizontal clamping logic to keep the computed left position within the viewport bounds for both “start” and “end” aligned positioning.
• Uses visualViewport dimensions/offsets (already present) as the reference bounds for the new clamping checks.

File summaries

File	Description
src/Core.Scripts/src/Components/Popover/FluentPopover.ts	Adds horizontal viewport clamping when positioning the popover dialog.

Review details

Suppressed comments (1)

src/Core.Scripts/src/Components/Popover/FluentPopover.ts:371

• Same issue as positionDialogStart: the overflow check uses dialogWidth (includes offsetHorizontal) while left already incorporates the horizontal offset. Using this.dialog.offsetWidth for the clamp avoids double-counting and ensures correct behavior for negative offsets.

        // Clamp horizontally so the dialog stays inside the viewport
        if (left < viewportLeft) {
          left = viewportLeft;
        }
        if (left + dialogWidth > viewportRight) {
          left = Math.max(viewportLeft, viewportRight - dialogWidth);
        }

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

---

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

[Copilot]

🔵 Needs a closer look

The new horizontal clamping logic currently double-counts offsetHorizontal (via left and dialogWidth), which can over-clamp and misposition the popover.

Review details

Suppressed comments (2)

src/Core.Scripts/src/Components/Popover/FluentPopover.ts:365

• The horizontal clamping check double-counts offsetHorizontal: left already includes this.offsetHorizontal, but dialogWidth also includes it, so left + dialogWidth effectively adds the offset twice and can over-clamp the popover (shifting it further than needed). Clamp using the actual rendered dialog width (this.dialog.offsetWidth) instead.

        if (left + dialogWidth > viewportWidth) {
          left = Math.max(0, viewportWidth - dialogWidth);
        }

src/Core.Scripts/src/Components/Popover/FluentPopover.ts:383

• Same as above: dialogWidth includes offsetHorizontal while left already includes it, so the clamp condition can push the dialog too far left/right. Use this.dialog.offsetWidth (or compute an unclamped width) for the right-edge clamp calculation.

        if (left + dialogWidth > viewportWidth) {
          left = Math.max(0, viewportWidth - dialogWidth);
        }

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Denis Voituron (dvoituron)]

Can you check that?

The new horizontal clamping logic double-counts offsetHorizontal. The offset is already included in the calculated left position, but it is also added to dialogWidth when checking for overflow. Positive offsets therefore move the popover too far, while negative offsets may allow it to remain outside the viewport. This can notably regress FluentAppBar, which uses an offset of 320 px.

Please use the actual rendered width (this.dialog.offsetWidth) for clamping and apply offsetHorizontal only to the candidate position. It would also be useful to add tests covering positive and negative offsets in both LTR and RTL modes.

[Marvin Klein (MarvinKlein1508)]

Denis Voituron (@dvoituron) yes. I tried this before. This fixes the horizontal alignment for Appbar. But there is still an issue with the vertical alignment. However - whenever you change anything there it breaks for all other components.

[Marvin Klein (MarvinKlein1508)]

do not merge yet. Need to push my commit to address the CoPilot review first.

[Marvin Klein (MarvinKlein1508)]

Comments addressed. This is working now for all components except for FluentAppbar reliable. Not sure what'S going on with the FluentAppbar though. At least it is now visible on mobile as well.

Also related to #5215 and #5214