From 316f946889c5c518e85f3a5f40438da8707e37de Mon Sep 17 00:00:00 2001 From: Joan Perals Tresserra Date: Thu, 16 Jan 2025 13:34:15 +0100 Subject: [PATCH 01/11] fix: Prevent dropdown misplacement in iOS --- src/internal/components/dropdown/index.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/internal/components/dropdown/index.tsx b/src/internal/components/dropdown/index.tsx index 9a449914fd..3a16fbf1f2 100644 --- a/src/internal/components/dropdown/index.tsx +++ b/src/internal/components/dropdown/index.tsx @@ -231,11 +231,13 @@ const Dropdown = ({ target.style.insetInlineStart = position.insetInlineStart; } + const targetRect = getLogicalBoundingClientRect(target); + // Position normal overflow dropdowns with fixed positioning relative to viewport if (expandToViewport && !interior) { target.style.position = 'fixed'; if (position.dropBlockStart) { - target.style.insetBlockEnd = `calc(100% - ${triggerBox.top}px)`; + target.style.insetBlockStart = `${triggerBox.top - targetRect.blockSize}px`; } else { target.style.insetBlockStart = `${triggerBox.bottom}px`; } @@ -393,9 +395,10 @@ const Dropdown = ({ if (triggerRef.current && dropdownRef.current && verticalContainerRef.current) { const triggerRect = getLogicalBoundingClientRect(triggerRef.current); const target = dropdownRef.current; + const targetRect = getLogicalBoundingClientRect(target); if (fixedPosition.current) { if (fixedPosition.current.dropBlockStart) { - dropdownRef.current.style.insetBlockEnd = `calc(100% - ${triggerRect.insetBlockStart}px)`; + target.style.insetBlockStart = `${triggerRect.insetBlockStart - targetRect.blockSize}px`; } else { target.style.insetBlockStart = `${triggerRect.insetBlockEnd}px`; } From 0f0296f114e8ff99613b128f13c766ee3c4bedc8 Mon Sep 17 00:00:00 2001 From: Joan Perals Tresserra Date: Fri, 17 Jan 2025 12:28:43 +0100 Subject: [PATCH 02/11] Revert "fix: Prevent dropdown misplacement in iOS" This reverts commit 5b9cecc5f7430033641a4a076c297b8b882da8bb. --- src/internal/components/dropdown/index.tsx | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/internal/components/dropdown/index.tsx b/src/internal/components/dropdown/index.tsx index 3a16fbf1f2..9a449914fd 100644 --- a/src/internal/components/dropdown/index.tsx +++ b/src/internal/components/dropdown/index.tsx @@ -231,13 +231,11 @@ const Dropdown = ({ target.style.insetInlineStart = position.insetInlineStart; } - const targetRect = getLogicalBoundingClientRect(target); - // Position normal overflow dropdowns with fixed positioning relative to viewport if (expandToViewport && !interior) { target.style.position = 'fixed'; if (position.dropBlockStart) { - target.style.insetBlockStart = `${triggerBox.top - targetRect.blockSize}px`; + target.style.insetBlockEnd = `calc(100% - ${triggerBox.top}px)`; } else { target.style.insetBlockStart = `${triggerBox.bottom}px`; } @@ -395,10 +393,9 @@ const Dropdown = ({ if (triggerRef.current && dropdownRef.current && verticalContainerRef.current) { const triggerRect = getLogicalBoundingClientRect(triggerRef.current); const target = dropdownRef.current; - const targetRect = getLogicalBoundingClientRect(target); if (fixedPosition.current) { if (fixedPosition.current.dropBlockStart) { - target.style.insetBlockStart = `${triggerRect.insetBlockStart - targetRect.blockSize}px`; + dropdownRef.current.style.insetBlockEnd = `calc(100% - ${triggerRect.insetBlockStart}px)`; } else { target.style.insetBlockStart = `${triggerRect.insetBlockEnd}px`; } From 5711fa48a67839bf55979c7c9ad456909eec649c Mon Sep 17 00:00:00 2001 From: Joan Perals Tresserra Date: Fri, 17 Jan 2025 12:37:45 +0100 Subject: [PATCH 03/11] Use absolute position --- src/internal/components/dropdown/index.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/internal/components/dropdown/index.tsx b/src/internal/components/dropdown/index.tsx index 9a449914fd..fc12ed0bb1 100644 --- a/src/internal/components/dropdown/index.tsx +++ b/src/internal/components/dropdown/index.tsx @@ -233,11 +233,11 @@ const Dropdown = ({ // Position normal overflow dropdowns with fixed positioning relative to viewport if (expandToViewport && !interior) { - target.style.position = 'fixed'; + target.style.position = 'absolute'; if (position.dropBlockStart) { - target.style.insetBlockEnd = `calc(100% - ${triggerBox.top}px)`; + target.style.insetBlockEnd = `calc(100% - ${document.documentElement.scrollTop + triggerBox.top}px)`; } else { - target.style.insetBlockStart = `${triggerBox.bottom}px`; + target.style.insetBlockStart = `${document.documentElement.scrollTop + triggerBox.bottom}px`; } if (position.dropInlineStart) { target.style.insetInlineStart = `calc(${triggerBox.right}px - ${position.inlineSize})`; @@ -395,9 +395,9 @@ const Dropdown = ({ const target = dropdownRef.current; if (fixedPosition.current) { if (fixedPosition.current.dropBlockStart) { - dropdownRef.current.style.insetBlockEnd = `calc(100% - ${triggerRect.insetBlockStart}px)`; + dropdownRef.current.style.insetBlockEnd = `calc(100% - ${document.documentElement.scrollTop + triggerRect.insetBlockStart}px)`; } else { - target.style.insetBlockStart = `${triggerRect.insetBlockEnd}px`; + target.style.insetBlockStart = `${document.documentElement.scrollTop + triggerRect.insetBlockEnd}px`; } if (fixedPosition.current.dropInlineStart) { target.style.insetInlineStart = `calc(${triggerRect.insetInlineEnd}px - ${fixedPosition.current.inlineSize})`; From 5a37cd4c82b0d58636bf1cef2b420e16582bdf11 Mon Sep 17 00:00:00 2001 From: Joan Perals Tresserra Date: Fri, 17 Jan 2025 17:59:03 +0100 Subject: [PATCH 04/11] Use absolute positioning only when iOS virtual keyboard is present --- src/internal/components/dropdown/index.tsx | 30 +++++++++++++++------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/internal/components/dropdown/index.tsx b/src/internal/components/dropdown/index.tsx index fc12ed0bb1..9880e4d24d 100644 --- a/src/internal/components/dropdown/index.tsx +++ b/src/internal/components/dropdown/index.tsx @@ -233,16 +233,20 @@ const Dropdown = ({ // Position normal overflow dropdowns with fixed positioning relative to viewport if (expandToViewport && !interior) { - target.style.position = 'absolute'; + const isIOSVirtualKeyboardPresent = + window.visualViewport?.height && window.visualViewport.height < window.innerHeight; + target.style.position = isIOSVirtualKeyboardPresent ? 'absolute' : 'fixed'; + const verticalScrollOffset = isIOSVirtualKeyboardPresent ? document.documentElement.scrollTop : 0; + const horizontalScrollOffset = isIOSVirtualKeyboardPresent ? document.documentElement.scrollLeft : 0; if (position.dropBlockStart) { - target.style.insetBlockEnd = `calc(100% - ${document.documentElement.scrollTop + triggerBox.top}px)`; + target.style.insetBlockEnd = `calc(100% - ${verticalScrollOffset + triggerBox.top}px)`; } else { - target.style.insetBlockStart = `${document.documentElement.scrollTop + triggerBox.bottom}px`; + target.style.insetBlockStart = `${verticalScrollOffset + triggerBox.bottom}px`; } if (position.dropInlineStart) { - target.style.insetInlineStart = `calc(${triggerBox.right}px - ${position.inlineSize})`; + target.style.insetInlineStart = `calc(${horizontalScrollOffset + triggerBox.right}px - ${position.inlineSize})`; } else { - target.style.insetInlineStart = `${triggerBox.left}px`; + target.style.insetInlineStart = `${horizontalScrollOffset + triggerBox.left}px`; } // Keep track of the initial dropdown position and direction. // Dropdown direction doesn't need to change as the user scrolls, just needs to stay attached to the trigger. @@ -391,18 +395,26 @@ const Dropdown = ({ } const updateDropdownPosition = () => { if (triggerRef.current && dropdownRef.current && verticalContainerRef.current) { + const isIOSVirtualKeyboardPresent = + window.visualViewport?.height && window.visualViewport.height < window.innerHeight; + + dropdownRef.current.style.position = isIOSVirtualKeyboardPresent ? 'absolute' : 'fixed'; + + const verticalScrollOffset = isIOSVirtualKeyboardPresent ? document.documentElement.scrollTop : 0; + const horizontalScrollOffset = isIOSVirtualKeyboardPresent ? document.documentElement.scrollLeft : 0; + const triggerRect = getLogicalBoundingClientRect(triggerRef.current); const target = dropdownRef.current; if (fixedPosition.current) { if (fixedPosition.current.dropBlockStart) { - dropdownRef.current.style.insetBlockEnd = `calc(100% - ${document.documentElement.scrollTop + triggerRect.insetBlockStart}px)`; + dropdownRef.current.style.insetBlockEnd = `calc(100% - ${verticalScrollOffset + triggerRect.insetBlockStart}px)`; } else { - target.style.insetBlockStart = `${document.documentElement.scrollTop + triggerRect.insetBlockEnd}px`; + target.style.insetBlockStart = `${verticalScrollOffset + triggerRect.insetBlockEnd}px`; } if (fixedPosition.current.dropInlineStart) { - target.style.insetInlineStart = `calc(${triggerRect.insetInlineEnd}px - ${fixedPosition.current.inlineSize})`; + target.style.insetInlineStart = `calc(${horizontalScrollOffset + triggerRect.insetInlineEnd}px - ${fixedPosition.current.inlineSize})`; } else { - target.style.insetInlineStart = `${triggerRect.insetInlineStart}px`; + target.style.insetInlineStart = `${horizontalScrollOffset + triggerRect.insetInlineStart}px`; } } } From 69c948d13c52251430c4f6c79b47ad144e453ad9 Mon Sep 17 00:00:00 2001 From: Joan Perals Tresserra Date: Tue, 21 Jan 2025 16:19:18 +0100 Subject: [PATCH 05/11] Refactor --- .../dropdown/dropdown-fit-handler.ts | 5 +- .../components/dropdown/dropdown-position.ts | 47 +++++++++++++++++ src/internal/components/dropdown/index.tsx | 52 +++++-------------- 3 files changed, 63 insertions(+), 41 deletions(-) create mode 100644 src/internal/components/dropdown/dropdown-position.ts diff --git a/src/internal/components/dropdown/dropdown-fit-handler.ts b/src/internal/components/dropdown/dropdown-fit-handler.ts index d856fd7cc1..0144763e18 100644 --- a/src/internal/components/dropdown/dropdown-fit-handler.ts +++ b/src/internal/components/dropdown/dropdown-fit-handler.ts @@ -4,6 +4,7 @@ import { getLogicalBoundingClientRect } from '@cloudscape-design/component-toolk import { getBreakpointValue } from '../../breakpoints'; import { BoundingBox, getOverflowParentDimensions, getOverflowParents } from '../../utils/scrollable-containers'; +import { LogicalDOMRect } from './dropdown-position'; import styles from './styles.css.js'; @@ -361,7 +362,7 @@ export const calculatePosition = ( isMobile: boolean, minWidth?: number, stretchBeyondTriggerWidth?: boolean -): [DropdownPosition, DOMRect] => { +): [DropdownPosition, LogicalDOMRect] => { // cleaning previously assigned values, // so that they are not reused in case of screen resize and similar events verticalContainerElement.style.maxBlockSize = ''; @@ -393,6 +394,6 @@ export const calculatePosition = ( isMobile, stretchBeyondTriggerWidth, }); - const triggerBox = triggerElement.getBoundingClientRect(); + const triggerBox = getLogicalBoundingClientRect(triggerElement); return [position, triggerBox]; }; diff --git a/src/internal/components/dropdown/dropdown-position.ts b/src/internal/components/dropdown/dropdown-position.ts new file mode 100644 index 0000000000..831eac9c34 --- /dev/null +++ b/src/internal/components/dropdown/dropdown-position.ts @@ -0,0 +1,47 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { getIsRtl } from '@cloudscape-design/component-toolkit/internal'; + +import { DropdownPosition } from './dropdown-fit-handler'; + +export interface LogicalDOMRect { + blockSize: number; + inlineSize: number; + insetBlockStart: number; + insetBlockEnd: number; + insetInlineStart: number; + insetInlineEnd: number; +} + +export function applyFixedDropdownPosition({ + position, + dropdownElement, + triggerRect, +}: { + position: DropdownPosition; + dropdownElement: HTMLElement; + triggerRect: LogicalDOMRect; +}) { + const useAbsolutePositioning = window.visualViewport?.height && window.visualViewport.height < window.innerHeight; + + const verticalScrollOffset = useAbsolutePositioning ? document.documentElement.scrollTop : 0; + const horizontalScrollOffset = useAbsolutePositioning + ? getIsRtl(document.body) + ? window.innerWidth - document.documentElement.scrollLeft + : document.documentElement.scrollLeft + : 0; + + dropdownElement.style.position = useAbsolutePositioning ? 'absolute' : 'fixed'; + + if (position.dropBlockStart) { + dropdownElement.style.insetBlockEnd = `calc(100% - ${verticalScrollOffset + triggerRect.insetBlockStart}px)`; + } else { + dropdownElement.style.insetBlockStart = `${verticalScrollOffset + triggerRect.insetBlockEnd}px`; + } + if (position.dropInlineStart) { + dropdownElement.style.insetInlineStart = `calc(${horizontalScrollOffset + triggerRect.insetInlineEnd}px - ${position.inlineSize})`; + } else { + dropdownElement.style.insetInlineStart = `${horizontalScrollOffset + triggerRect.insetInlineStart}px`; + } +} diff --git a/src/internal/components/dropdown/index.tsx b/src/internal/components/dropdown/index.tsx index 9880e4d24d..d1c72588b7 100644 --- a/src/internal/components/dropdown/index.tsx +++ b/src/internal/components/dropdown/index.tsx @@ -26,6 +26,7 @@ import { hasEnoughSpaceToStretchBeyondTriggerWidth, InteriorDropdownPosition, } from './dropdown-fit-handler'; +import { applyFixedDropdownPosition, LogicalDOMRect } from './dropdown-position'; import { DropdownProps } from './interfaces'; import styles from './styles.css.js'; @@ -196,7 +197,7 @@ const Dropdown = ({ const setDropdownPosition = ( position: DropdownPosition | InteriorDropdownPosition, - triggerBox: DOMRect, + triggerBox: LogicalDOMRect, target: HTMLDivElement, verticalContainer: HTMLDivElement ) => { @@ -233,21 +234,11 @@ const Dropdown = ({ // Position normal overflow dropdowns with fixed positioning relative to viewport if (expandToViewport && !interior) { - const isIOSVirtualKeyboardPresent = - window.visualViewport?.height && window.visualViewport.height < window.innerHeight; - target.style.position = isIOSVirtualKeyboardPresent ? 'absolute' : 'fixed'; - const verticalScrollOffset = isIOSVirtualKeyboardPresent ? document.documentElement.scrollTop : 0; - const horizontalScrollOffset = isIOSVirtualKeyboardPresent ? document.documentElement.scrollLeft : 0; - if (position.dropBlockStart) { - target.style.insetBlockEnd = `calc(100% - ${verticalScrollOffset + triggerBox.top}px)`; - } else { - target.style.insetBlockStart = `${verticalScrollOffset + triggerBox.bottom}px`; - } - if (position.dropInlineStart) { - target.style.insetInlineStart = `calc(${horizontalScrollOffset + triggerBox.right}px - ${position.inlineSize})`; - } else { - target.style.insetInlineStart = `${horizontalScrollOffset + triggerBox.left}px`; - } + applyFixedDropdownPosition({ + position, + dropdownElement: target, + triggerRect: triggerBox, + }); // Keep track of the initial dropdown position and direction. // Dropdown direction doesn't need to change as the user scrolls, just needs to stay attached to the trigger. fixedPosition.current = position; @@ -394,29 +385,12 @@ const Dropdown = ({ return; } const updateDropdownPosition = () => { - if (triggerRef.current && dropdownRef.current && verticalContainerRef.current) { - const isIOSVirtualKeyboardPresent = - window.visualViewport?.height && window.visualViewport.height < window.innerHeight; - - dropdownRef.current.style.position = isIOSVirtualKeyboardPresent ? 'absolute' : 'fixed'; - - const verticalScrollOffset = isIOSVirtualKeyboardPresent ? document.documentElement.scrollTop : 0; - const horizontalScrollOffset = isIOSVirtualKeyboardPresent ? document.documentElement.scrollLeft : 0; - - const triggerRect = getLogicalBoundingClientRect(triggerRef.current); - const target = dropdownRef.current; - if (fixedPosition.current) { - if (fixedPosition.current.dropBlockStart) { - dropdownRef.current.style.insetBlockEnd = `calc(100% - ${verticalScrollOffset + triggerRect.insetBlockStart}px)`; - } else { - target.style.insetBlockStart = `${verticalScrollOffset + triggerRect.insetBlockEnd}px`; - } - if (fixedPosition.current.dropInlineStart) { - target.style.insetInlineStart = `calc(${horizontalScrollOffset + triggerRect.insetInlineEnd}px - ${fixedPosition.current.inlineSize})`; - } else { - target.style.insetInlineStart = `${horizontalScrollOffset + triggerRect.insetInlineStart}px`; - } - } + if (triggerRef.current && dropdownRef.current && verticalContainerRef.current && fixedPosition.current) { + applyFixedDropdownPosition({ + position: fixedPosition.current, + dropdownElement: dropdownRef.current, + triggerRect: getLogicalBoundingClientRect(triggerRef.current), + }); } }; From ef79ef3fcf9983e09318d6f30a74f892fd6f0a28 Mon Sep 17 00:00:00 2001 From: Joan Perals Tresserra Date: Tue, 21 Jan 2025 17:11:58 +0100 Subject: [PATCH 06/11] Apply absolute positioning on mobile --- src/internal/components/dropdown/dropdown-position.ts | 4 +++- src/internal/components/dropdown/index.tsx | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/internal/components/dropdown/dropdown-position.ts b/src/internal/components/dropdown/dropdown-position.ts index 831eac9c34..bc26626768 100644 --- a/src/internal/components/dropdown/dropdown-position.ts +++ b/src/internal/components/dropdown/dropdown-position.ts @@ -18,12 +18,14 @@ export function applyFixedDropdownPosition({ position, dropdownElement, triggerRect, + isMobile, }: { position: DropdownPosition; dropdownElement: HTMLElement; triggerRect: LogicalDOMRect; + isMobile: boolean; }) { - const useAbsolutePositioning = window.visualViewport?.height && window.visualViewport.height < window.innerHeight; + const useAbsolutePositioning = isMobile; const verticalScrollOffset = useAbsolutePositioning ? document.documentElement.scrollTop : 0; const horizontalScrollOffset = useAbsolutePositioning diff --git a/src/internal/components/dropdown/index.tsx b/src/internal/components/dropdown/index.tsx index d1c72588b7..14f321f180 100644 --- a/src/internal/components/dropdown/index.tsx +++ b/src/internal/components/dropdown/index.tsx @@ -238,6 +238,7 @@ const Dropdown = ({ position, dropdownElement: target, triggerRect: triggerBox, + isMobile, }); // Keep track of the initial dropdown position and direction. // Dropdown direction doesn't need to change as the user scrolls, just needs to stay attached to the trigger. @@ -390,6 +391,7 @@ const Dropdown = ({ position: fixedPosition.current, dropdownElement: dropdownRef.current, triggerRect: getLogicalBoundingClientRect(triggerRef.current), + isMobile, }); } }; @@ -402,7 +404,7 @@ const Dropdown = ({ return () => { controller.abort(); }; - }, [open, expandToViewport]); + }, [open, expandToViewport, isMobile]); const referrerId = useUniqueId(); From 614d7cf0acee4e479c43c056b5d00b1ac771dd79 Mon Sep 17 00:00:00 2001 From: Joan Perals Tresserra Date: Wed, 22 Jan 2025 09:01:22 +0100 Subject: [PATCH 07/11] Use scrollLeft also in RTL --- src/internal/components/dropdown/dropdown-position.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/internal/components/dropdown/dropdown-position.ts b/src/internal/components/dropdown/dropdown-position.ts index bc26626768..b628fd4dce 100644 --- a/src/internal/components/dropdown/dropdown-position.ts +++ b/src/internal/components/dropdown/dropdown-position.ts @@ -1,8 +1,6 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { getIsRtl } from '@cloudscape-design/component-toolkit/internal'; - import { DropdownPosition } from './dropdown-fit-handler'; export interface LogicalDOMRect { @@ -28,11 +26,7 @@ export function applyFixedDropdownPosition({ const useAbsolutePositioning = isMobile; const verticalScrollOffset = useAbsolutePositioning ? document.documentElement.scrollTop : 0; - const horizontalScrollOffset = useAbsolutePositioning - ? getIsRtl(document.body) - ? window.innerWidth - document.documentElement.scrollLeft - : document.documentElement.scrollLeft - : 0; + const horizontalScrollOffset = useAbsolutePositioning ? document.documentElement.scrollLeft : 0; dropdownElement.style.position = useAbsolutePositioning ? 'absolute' : 'fixed'; From d0bd249e79439ff88dd5280f0094a3888e8f0fa6 Mon Sep 17 00:00:00 2001 From: Joan Perals Tresserra Date: Wed, 22 Jan 2025 09:55:07 +0100 Subject: [PATCH 08/11] Add unit test coverage --- .../__tests__/dropdown-position.test.ts | 91 +++++++++++++++++++ .../components/dropdown/dropdown-position.ts | 3 +- src/internal/components/dropdown/index.tsx | 6 +- 3 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 src/internal/components/dropdown/__tests__/dropdown-position.test.ts diff --git a/src/internal/components/dropdown/__tests__/dropdown-position.test.ts b/src/internal/components/dropdown/__tests__/dropdown-position.test.ts new file mode 100644 index 0000000000..cbc740d1c1 --- /dev/null +++ b/src/internal/components/dropdown/__tests__/dropdown-position.test.ts @@ -0,0 +1,91 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { applyDropdownPositionRelativeToViewport } from '../../../../../lib/components/internal/components/dropdown/dropdown-position'; + +describe('applyDropdownPositionRelativeToViewport', () => { + const triggerRect = { + blockSize: 50, + inlineSize: 100, + insetBlockStart: 100, + insetInlineStart: 100, + insetBlockEnd: 150, + insetInlineEnd: 200, + }; + + const baseDropdownPosition = { + blockSize: '100px', + inlineSize: '100px', + insetInlineStart: '100px', + dropBlockStart: false, + dropInlineStart: false, + }; + + test("anchored to the trigger's block start", () => { + const dropdownElement = document.createElement('div'); + applyDropdownPositionRelativeToViewport({ + dropdownElement, + triggerRect, + position: { ...baseDropdownPosition, dropBlockStart: true }, + isMobile: false, + }); + expect(dropdownElement.style.insetBlockEnd).toBeTruthy(); + expect(dropdownElement.style.insetBlockStart).toBeFalsy(); + }); + + test("anchored to the trigger's block end", () => { + const dropdownElement = document.createElement('div'); + applyDropdownPositionRelativeToViewport({ + dropdownElement, + triggerRect, + position: baseDropdownPosition, + isMobile: false, + }); + expect(dropdownElement.style.insetBlockEnd).toBeFalsy(); + expect(dropdownElement.style.insetBlockStart).toEqual(`${triggerRect.insetBlockEnd}px`); + }); + + test("anchored to the trigger's inline start", () => { + const dropdownElement = document.createElement('div'); + applyDropdownPositionRelativeToViewport({ + dropdownElement, + triggerRect, + position: { ...baseDropdownPosition, dropInlineStart: true }, + isMobile: false, + }); + expect(dropdownElement.style.insetInlineStart).toBeTruthy(); + }); + + test("not anchored to the trigger's inline start", () => { + const dropdownElement = document.createElement('div'); + applyDropdownPositionRelativeToViewport({ + dropdownElement, + triggerRect, + position: baseDropdownPosition, + isMobile: false, + }); + expect(dropdownElement.style.insetInlineStart).toEqual(`${triggerRect.insetInlineStart}px`); + }); + + test('uses fixed position on desktop', () => { + const dropdownElement = document.createElement('div'); + applyDropdownPositionRelativeToViewport({ + dropdownElement, + triggerRect, + position: baseDropdownPosition, + isMobile: false, + }); + expect(dropdownElement.style.position).toEqual('fixed'); + }); + + test('uses absolute position on mobile', () => { + const dropdownElement = document.createElement('div'); + applyDropdownPositionRelativeToViewport({ + dropdownElement, + triggerRect, + position: baseDropdownPosition, + isMobile: true, + }); + expect(dropdownElement.style.position).toEqual('absolute'); + }); +}); diff --git a/src/internal/components/dropdown/dropdown-position.ts b/src/internal/components/dropdown/dropdown-position.ts index b628fd4dce..47194e7e56 100644 --- a/src/internal/components/dropdown/dropdown-position.ts +++ b/src/internal/components/dropdown/dropdown-position.ts @@ -12,7 +12,8 @@ export interface LogicalDOMRect { insetInlineEnd: number; } -export function applyFixedDropdownPosition({ +// Applies its position to the dropdown element when expandToViewport is set to true. +export function applyDropdownPositionRelativeToViewport({ position, dropdownElement, triggerRect, diff --git a/src/internal/components/dropdown/index.tsx b/src/internal/components/dropdown/index.tsx index 14f321f180..124a84d054 100644 --- a/src/internal/components/dropdown/index.tsx +++ b/src/internal/components/dropdown/index.tsx @@ -26,7 +26,7 @@ import { hasEnoughSpaceToStretchBeyondTriggerWidth, InteriorDropdownPosition, } from './dropdown-fit-handler'; -import { applyFixedDropdownPosition, LogicalDOMRect } from './dropdown-position'; +import { applyDropdownPositionRelativeToViewport, LogicalDOMRect } from './dropdown-position'; import { DropdownProps } from './interfaces'; import styles from './styles.css.js'; @@ -234,7 +234,7 @@ const Dropdown = ({ // Position normal overflow dropdowns with fixed positioning relative to viewport if (expandToViewport && !interior) { - applyFixedDropdownPosition({ + applyDropdownPositionRelativeToViewport({ position, dropdownElement: target, triggerRect: triggerBox, @@ -387,7 +387,7 @@ const Dropdown = ({ } const updateDropdownPosition = () => { if (triggerRef.current && dropdownRef.current && verticalContainerRef.current && fixedPosition.current) { - applyFixedDropdownPosition({ + applyDropdownPositionRelativeToViewport({ position: fixedPosition.current, dropdownElement: dropdownRef.current, triggerRect: getLogicalBoundingClientRect(triggerRef.current), From 973698bdf2b7bb7ef557c8d6c1b189bb1d2df068 Mon Sep 17 00:00:00 2001 From: Joan Perals Tresserra Date: Wed, 22 Jan 2025 13:31:27 +0100 Subject: [PATCH 09/11] Add comments --- src/internal/components/dropdown/dropdown-position.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/internal/components/dropdown/dropdown-position.ts b/src/internal/components/dropdown/dropdown-position.ts index 47194e7e56..d592d3bdb2 100644 --- a/src/internal/components/dropdown/dropdown-position.ts +++ b/src/internal/components/dropdown/dropdown-position.ts @@ -24,8 +24,13 @@ export function applyDropdownPositionRelativeToViewport({ triggerRect: LogicalDOMRect; isMobile: boolean; }) { + // Fixed positions is not respected in iOS when the virtual keyboard is being displayed. + // For this reason we use absolute positioning in mobile. const useAbsolutePositioning = isMobile; + // Since when using expandToViewport=true the dropdown is attached to the root of the body, + // the same coordinates can be used for fixed or absolute position, + // except when using absolute position we need to take into account the scroll position of the body itself. const verticalScrollOffset = useAbsolutePositioning ? document.documentElement.scrollTop : 0; const horizontalScrollOffset = useAbsolutePositioning ? document.documentElement.scrollLeft : 0; From aa7c16f73baa712f62bcf353fdf288914793aea2 Mon Sep 17 00:00:00 2001 From: Joan Perals Tresserra Date: Wed, 22 Jan 2025 13:59:45 +0100 Subject: [PATCH 10/11] Refactor tests --- .../dropdown/__tests__/dropdown-position.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/internal/components/dropdown/__tests__/dropdown-position.test.ts b/src/internal/components/dropdown/__tests__/dropdown-position.test.ts index cbc740d1c1..897d918723 100644 --- a/src/internal/components/dropdown/__tests__/dropdown-position.test.ts +++ b/src/internal/components/dropdown/__tests__/dropdown-position.test.ts @@ -21,7 +21,7 @@ describe('applyDropdownPositionRelativeToViewport', () => { dropInlineStart: false, }; - test("anchored to the trigger's block start", () => { + test("sets block end when the dropdown is anchored to the trigger's block start (expands up)", () => { const dropdownElement = document.createElement('div'); applyDropdownPositionRelativeToViewport({ dropdownElement, @@ -33,7 +33,7 @@ describe('applyDropdownPositionRelativeToViewport', () => { expect(dropdownElement.style.insetBlockStart).toBeFalsy(); }); - test("anchored to the trigger's block end", () => { + test("aligns block start with the trigger's block end when the dropdown is anchored to the trigger's block end (expands down)", () => { const dropdownElement = document.createElement('div'); applyDropdownPositionRelativeToViewport({ dropdownElement, @@ -45,26 +45,26 @@ describe('applyDropdownPositionRelativeToViewport', () => { expect(dropdownElement.style.insetBlockStart).toEqual(`${triggerRect.insetBlockEnd}px`); }); - test("anchored to the trigger's inline start", () => { + test("aligns inline start with the trigger's inline start when the dropdown is anchored to the trigger's inline start (anchored from the left in LTR)", () => { const dropdownElement = document.createElement('div'); applyDropdownPositionRelativeToViewport({ dropdownElement, triggerRect, - position: { ...baseDropdownPosition, dropInlineStart: true }, + position: baseDropdownPosition, isMobile: false, }); - expect(dropdownElement.style.insetInlineStart).toBeTruthy(); + expect(dropdownElement.style.insetInlineStart).toEqual(`${triggerRect.insetInlineStart}px`); }); - test("not anchored to the trigger's inline start", () => { + test("sets inline end when the dropdown is anchored to the trigger's inline start (anchored from the right in LTR)", () => { const dropdownElement = document.createElement('div'); applyDropdownPositionRelativeToViewport({ dropdownElement, triggerRect, - position: baseDropdownPosition, + position: { ...baseDropdownPosition, dropInlineStart: true }, isMobile: false, }); - expect(dropdownElement.style.insetInlineStart).toEqual(`${triggerRect.insetInlineStart}px`); + expect(dropdownElement.style.insetInlineStart).toBeTruthy(); }); test('uses fixed position on desktop', () => { From 69c62072d7525c53f608010e160e7eb1b1b481e4 Mon Sep 17 00:00:00 2001 From: Joan Perals Tresserra Date: Wed, 22 Jan 2025 14:21:52 +0100 Subject: [PATCH 11/11] Typo --- src/internal/components/dropdown/dropdown-position.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal/components/dropdown/dropdown-position.ts b/src/internal/components/dropdown/dropdown-position.ts index d592d3bdb2..3ad3aaf2c0 100644 --- a/src/internal/components/dropdown/dropdown-position.ts +++ b/src/internal/components/dropdown/dropdown-position.ts @@ -24,7 +24,7 @@ export function applyDropdownPositionRelativeToViewport({ triggerRect: LogicalDOMRect; isMobile: boolean; }) { - // Fixed positions is not respected in iOS when the virtual keyboard is being displayed. + // Fixed positions are not respected in iOS when the virtual keyboard is being displayed. // For this reason we use absolute positioning in mobile. const useAbsolutePositioning = isMobile;