Skip to content

Commit 09af9ad

Browse files
fix(getAnchoredPosition): add better support for floatingElements inside dialogs (#681)
* add repro * lock scroll * implement new "displayInVisibleViewport" setting for getAnchoredPosition * rename story * Fix getAnchoredPosition for better dialog support Enhance getAnchoredPosition to support floatingElements in dialogs with a new setting. * Refactor pureCalculateAnchoredPosition to eliminate DOM dependency (#683) * Initial plan * Refactor pureCalculateAnchoredPosition to accept viewport dimensions as parameter Co-authored-by: francinelucca <40550942+francinelucca@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: francinelucca <40550942+francinelucca@users.noreply.github.com> * rename displayInVisibleViewport to displayInViewport * Update quick-masks-camp.md --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
1 parent 8432a5a commit 09af9ad

File tree

5 files changed

+568
-13
lines changed

5 files changed

+568
-13
lines changed

.changeset/quick-masks-camp.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@primer/behaviors": patch
3+
---
4+
5+
fix(getAnchoredPosition): add better support for floatingElements inside dialogs through new "displayInViewport" setting

src/__tests__/anchored-position.test.ts

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,4 +573,128 @@ describe('getAnchoredPosition', () => {
573573
expect(result.left).toEqual(290) // anchorRect.left - parentRect.left
574574
})
575575
})
576+
577+
describe('displayInViewport option', () => {
578+
beforeEach(() => {
579+
// Mock window dimensions for consistent testing
580+
Object.defineProperty(window, 'innerWidth', {value: 1024, writable: true})
581+
Object.defineProperty(window, 'innerHeight', {value: 768, writable: true})
582+
})
583+
584+
it('defaults to false when not specified', () => {
585+
const anchorRect = makeDOMRect(300, 200, 50, 50)
586+
const floatingRect = makeDOMRect(NaN, NaN, 100, 100)
587+
const {float, anchor} = createVirtualDOM(makeDOMRect(20, 20, 1920, 1080), anchorRect, floatingRect)
588+
589+
const {top, left} = getAnchoredPosition(float, anchor, {side: 'outside-bottom'})
590+
591+
// Should use normal clipping behavior (relative to clipping parent)
592+
expect(top).toEqual(234) // anchorRect.top + anchorRect.height + 4 - parentRect.top
593+
expect(left).toEqual(280) // anchorRect.left - parentRect.left
594+
})
595+
596+
it('constrains to visible viewport when set to true', () => {
597+
const anchorRect = makeDOMRect(950, 700, 50, 50) // Near bottom-right of viewport
598+
const floatingRect = makeDOMRect(NaN, NaN, 200, 200) // Large floating element
599+
const {float, anchor} = createVirtualDOM(
600+
makeDOMRect(0, 0, 2000, 2000), // Large container
601+
anchorRect,
602+
floatingRect,
603+
)
604+
605+
const {top, left} = getAnchoredPosition(float, anchor, {
606+
side: 'outside-bottom',
607+
displayInViewport: true,
608+
})
609+
610+
// Should be constrained to fit within viewport (1024x768)
611+
expect(left).toBeLessThanOrEqual(1024 - 200) // left + width <= viewport width
612+
expect(top + 200).toBeLessThanOrEqual(768) // top + height <= viewport height
613+
})
614+
615+
it('flips to top when bottom would exceed viewport height', () => {
616+
const anchorRect = makeDOMRect(400, 700, 50, 50) // Near bottom of viewport
617+
const floatingRect = makeDOMRect(NaN, NaN, 100, 100)
618+
const {float, anchor} = createVirtualDOM(makeDOMRect(0, 0, 1920, 1080), anchorRect, floatingRect)
619+
620+
const {top, anchorSide} = getAnchoredPosition(float, anchor, {
621+
side: 'outside-bottom',
622+
displayInViewport: true,
623+
})
624+
625+
// Should flip to top side to fit in viewport
626+
expect(anchorSide).toEqual('outside-top')
627+
expect(top).toBeGreaterThanOrEqual(0)
628+
expect(top + 100).toBeLessThanOrEqual(768)
629+
})
630+
631+
it('adjusts left position when right edge would exceed viewport width', () => {
632+
const anchorRect = makeDOMRect(950, 300, 50, 50) // Near right edge of viewport
633+
const floatingRect = makeDOMRect(NaN, NaN, 150, 100)
634+
const {float, anchor} = createVirtualDOM(makeDOMRect(0, 0, 1920, 1080), anchorRect, floatingRect)
635+
636+
const {left} = getAnchoredPosition(float, anchor, {
637+
side: 'outside-bottom',
638+
align: 'start',
639+
displayInViewport: true,
640+
})
641+
642+
// Should be pushed left to fit in viewport
643+
expect(left + 150).toBeLessThanOrEqual(1024) // left + width <= viewport width
644+
})
645+
646+
it('works with inside positioning', () => {
647+
const anchorRect = makeDOMRect(100, 100, 800, 600) // Large anchor covering most viewport
648+
const floatingRect = makeDOMRect(NaN, NaN, 200, 100)
649+
const {float, anchor} = createVirtualDOM(makeDOMRect(0, 0, 1920, 1080), anchorRect, floatingRect)
650+
651+
const {top, left} = getAnchoredPosition(float, anchor, {
652+
side: 'inside-center',
653+
align: 'center',
654+
displayInViewport: true,
655+
})
656+
657+
// Should position inside the anchor and within viewport
658+
expect(left).toBeGreaterThanOrEqual(0)
659+
expect(top).toBeGreaterThanOrEqual(0)
660+
expect(left + 200).toBeLessThanOrEqual(1024)
661+
expect(top + 100).toBeLessThanOrEqual(768)
662+
})
663+
664+
it('still respects allowOutOfBounds when both options are set', () => {
665+
const anchorRect = makeDOMRect(950, 700, 50, 50)
666+
const floatingRect = makeDOMRect(NaN, NaN, 200, 200)
667+
const {float, anchor} = createVirtualDOM(makeDOMRect(0, 0, 1920, 1080), anchorRect, floatingRect)
668+
669+
const {top, left} = getAnchoredPosition(float, anchor, {
670+
side: 'outside-bottom',
671+
displayInViewport: true,
672+
allowOutOfBounds: true,
673+
})
674+
675+
// When allowOutOfBounds is true, should allow overflow even with displayInViewport
676+
// The exact behavior depends on implementation, but it should not throw an error
677+
expect(typeof top).toBe('number')
678+
expect(typeof left).toBe('number')
679+
})
680+
681+
it('handles edge case when anchor is outside viewport', () => {
682+
const anchorRect = makeDOMRect(1200, 900, 50, 50) // Outside 1024x768 viewport
683+
const floatingRect = makeDOMRect(NaN, NaN, 100, 100)
684+
const {float, anchor} = createVirtualDOM(makeDOMRect(0, 0, 2000, 2000), anchorRect, floatingRect)
685+
686+
const {top, left} = getAnchoredPosition(float, anchor, {
687+
side: 'outside-bottom',
688+
displayInViewport: true,
689+
})
690+
691+
// When anchor is outside viewport, the position calculation should still work
692+
// but the final nudging should ensure no part of the floating element exceeds viewport
693+
// Since the anchor is at (1200, 900) with 'outside-bottom', the floating element
694+
// would initially be positioned around (1200, 954), which exceeds viewport bounds
695+
// The displayInViewport option should constrain this
696+
expect(left + 100).toBeLessThanOrEqual(1024) // right edge within viewport
697+
expect(top + 100).toBeLessThanOrEqual(768) // bottom edge within viewport
698+
})
699+
})
576700
})

src/anchored-position.ts

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,14 @@ export interface PositionSettings {
8787
* this direction.
8888
*/
8989
allowOutOfBounds: boolean
90+
91+
/**
92+
* If true, attempt to position the floating element entirely within the visible
93+
* viewport (window.innerWidth/innerHeight) without requiring scrolling.
94+
* This is useful when the anchor is inside a dialog or other container that
95+
* prevents scrolling of the main document.
96+
*/
97+
displayInViewport?: boolean
9098
}
9199

92100
// For each outside anchor position, list the order of alternate positions to try in
@@ -167,6 +175,7 @@ export function getAnchoredPosition(
167175
floatingElement.getBoundingClientRect(),
168176
anchorElement instanceof Element ? anchorElement.getBoundingClientRect() : anchorElement,
169177
getDefaultSettings(settings),
178+
{width: window.innerWidth, height: window.innerHeight},
170179
)
171180
}
172181

@@ -292,6 +301,7 @@ const positionDefaults: PositionSettings = {
292301
alignmentOffset: 4,
293302

294303
allowOutOfBounds: false,
304+
displayInViewport: false,
295305
}
296306

297307
/**
@@ -311,6 +321,7 @@ function getDefaultSettings(settings: Partial<PositionSettings> = {}): PositionS
311321
settings.alignmentOffset ??
312322
(align !== 'center' && side.startsWith('inside') ? positionDefaults.alignmentOffset : 0),
313323
allowOutOfBounds: settings.allowOutOfBounds ?? positionDefaults.allowOutOfBounds,
324+
displayInViewport: settings.displayInViewport ?? positionDefaults.displayInViewport,
314325
}
315326
}
316327

@@ -324,20 +335,34 @@ function getDefaultSettings(settings: Partial<PositionSettings> = {}): PositionS
324335
* @param floatingRect WidthAndHeight for the floating element
325336
* @param anchorRect BoxPosition for the anchor element
326337
* @param PositionSettings to customize the calculated position for the floating element.
338+
* @param visibleViewportSize Size of the visible viewport (window dimensions), used when displayInVisibleViewport is true
327339
*/
328340
function pureCalculateAnchoredPosition(
329341
viewportRect: BoxPosition,
330342
relativePosition: Position,
331343
floatingRect: Size,
332344
anchorRect: BoxPosition,
333-
{side, align, allowOutOfBounds, anchorOffset, alignmentOffset}: PositionSettings,
345+
{side, align, allowOutOfBounds, anchorOffset, alignmentOffset, displayInViewport}: PositionSettings,
346+
visibleViewportSize: Size,
334347
): AnchorPosition {
335348
// Compute the relative viewport rect, to bring it into the same coordinate space as `pos`
349+
let effectiveViewportRect = viewportRect
350+
351+
// If displayInViewport is true, constrain to the actual visible viewport
352+
if (displayInViewport) {
353+
effectiveViewportRect = {
354+
top: 0,
355+
left: 0,
356+
width: visibleViewportSize.width,
357+
height: visibleViewportSize.height,
358+
}
359+
}
360+
336361
const relativeViewportRect: BoxPosition = {
337-
top: viewportRect.top - relativePosition.top,
338-
left: viewportRect.left - relativePosition.left,
339-
width: viewportRect.width,
340-
height: viewportRect.height,
362+
top: effectiveViewportRect.top - relativePosition.top,
363+
left: effectiveViewportRect.left - relativePosition.left,
364+
width: effectiveViewportRect.width,
365+
height: effectiveViewportRect.height,
341366
}
342367

343368
let pos = calculatePosition(floatingRect, anchorRect, side, align, anchorOffset, alignmentOffset)
@@ -400,16 +425,14 @@ function pureCalculateAnchoredPosition(
400425
if (pos.left < relativeViewportRect.left) {
401426
pos.left = relativeViewportRect.left
402427
}
403-
if (pos.left + floatingRect.width > viewportRect.width + relativeViewportRect.left) {
404-
pos.left = viewportRect.width + relativeViewportRect.left - floatingRect.width
428+
if (pos.left + floatingRect.width > effectiveViewportRect.width + relativeViewportRect.left) {
429+
pos.left = effectiveViewportRect.width + relativeViewportRect.left - floatingRect.width
405430
}
406-
// If we have exhausted all possible positions and none of them worked, we
407-
// say that overflowing the bottom of the screen is acceptable since it is
408-
// likely to be able to scroll.
409-
if (alternateOrder && positionAttempt < alternateOrder.length) {
410-
if (pos.top + floatingRect.height > viewportRect.height + relativeViewportRect.top) {
431+
// Always apply bottom constraint when displayInViewport is true
432+
if (displayInViewport || (alternateOrder && positionAttempt < alternateOrder.length)) {
433+
if (pos.top + floatingRect.height > effectiveViewportRect.height + relativeViewportRect.top) {
411434
// This prevents top from being a negative value
412-
pos.top = Math.max(viewportRect.height + relativeViewportRect.top - floatingRect.height, 0)
435+
pos.top = Math.max(effectiveViewportRect.height + relativeViewportRect.top - floatingRect.height, 0)
413436
}
414437
}
415438
}

0 commit comments

Comments
 (0)