Skip to content

Commit 8c20e1c

Browse files
authored
fix: UAP buttons misplaced when drawer opens via keyboard (#4238)
1 parent 406ec7f commit 8c20e1c

File tree

2 files changed

+92
-5
lines changed

2 files changed

+92
-5
lines changed

src/internal/components/drag-handle-wrapper/__tests__/drag-handle-wrapper.test.tsx

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
import React from 'react';
5-
import { fireEvent, render } from '@testing-library/react';
5+
import { act, fireEvent, render } from '@testing-library/react';
66

77
import { getLogicalBoundingClientRect } from '@cloudscape-design/component-toolkit/internal';
88

@@ -32,6 +32,16 @@ const position = (inlineStart: number, blockStart: number) => ({
3232
insetBlockEnd: blockStart + size.blockSize,
3333
});
3434

35+
// Store RAF callbacks globally so flush() can be used in tests
36+
const rafCallbacks = new Set<FrameRequestCallback>();
37+
const flushAnimationFrames = () => {
38+
act(() => {
39+
const callbacks = [...rafCallbacks]; // Snapshot before clearing to preserve re-registered callbacks
40+
rafCallbacks.clear();
41+
callbacks.forEach(cb => cb(performance.now()));
42+
});
43+
};
44+
3545
beforeAll(() => {
3646
(window as any).PointerEvent ??= PointerEventMock;
3747
});
@@ -42,6 +52,16 @@ beforeEach(() => {
4252
jest.mocked(getLogicalBoundingClientRect).mockReturnValue(position(viewport.width / 2, viewport.height / 2));
4353
Object.defineProperty(window, 'innerWidth', { value: viewport.width, writable: true });
4454
Object.defineProperty(window, 'innerHeight', { value: viewport.height, writable: true });
55+
56+
// Clear any leftover callbacks from previous tests
57+
rafCallbacks.clear();
58+
59+
// Mock requestAnimationFrame to collect callbacks for explicit flushing
60+
jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb: FrameRequestCallback) => {
61+
rafCallbacks.add(cb);
62+
return rafCallbacks.size;
63+
});
64+
jest.spyOn(window, 'cancelAnimationFrame').mockImplementation(() => {});
4565
});
4666

4767
afterEach(() => {
@@ -759,6 +779,7 @@ describe('forced position behavior', () => {
759779
jest.mocked(getLogicalBoundingClientRect).mockReturnValue(position(viewport.width / 2, viewport.height / 2));
760780

761781
renderDragHandle(allDirections);
782+
flushAnimationFrames();
762783

763784
expect(getDirectionButton('block-start')).not.toBe(null);
764785
expect(getDirectionButton('block-end')).not.toBe(null);
@@ -771,6 +792,7 @@ describe('forced position behavior', () => {
771792
jest.mocked(getLogicalBoundingClientRect).mockReturnValue(position(viewport.width / 2, 25));
772793

773794
renderDragHandle(allDirections);
795+
flushAnimationFrames();
774796

775797
expect(getForcedDirectionButton('block-end', 'bottom', 3)).not.toBe(null);
776798
expect(getForcedDirectionButton('block-start', 'bottom', 2)).not.toBe(null);
@@ -783,6 +805,7 @@ describe('forced position behavior', () => {
783805
jest.mocked(getLogicalBoundingClientRect).mockReturnValue(position(25, 25));
784806

785807
renderDragHandle(allDirections);
808+
flushAnimationFrames();
786809

787810
expect(getForcedDirectionButton('block-end', 'bottom', 3)).not.toBe(null);
788811
expect(getForcedDirectionButton('block-start', 'bottom', 2)).not.toBe(null);
@@ -795,6 +818,7 @@ describe('forced position behavior', () => {
795818
jest.mocked(getLogicalBoundingClientRect).mockReturnValue(position(viewport.width - 75, 25));
796819

797820
renderDragHandle(allDirections);
821+
flushAnimationFrames();
798822

799823
expect(getForcedDirectionButton('block-end', 'bottom', 3)).not.toBe(null);
800824
expect(getForcedDirectionButton('block-start', 'bottom', 2)).not.toBe(null);
@@ -807,6 +831,7 @@ describe('forced position behavior', () => {
807831
jest.mocked(getLogicalBoundingClientRect).mockReturnValue(position(25, viewport.height / 2));
808832

809833
renderDragHandle(allDirections);
834+
flushAnimationFrames();
810835

811836
expect(getForcedDirectionButton('block-end', 'top', 0)).not.toBe(null);
812837
expect(getForcedDirectionButton('block-start', 'top', 1)).not.toBe(null);
@@ -819,6 +844,7 @@ describe('forced position behavior', () => {
819844
jest.mocked(getLogicalBoundingClientRect).mockReturnValue(position(viewport.width - 75, viewport.height / 2));
820845

821846
renderDragHandle(allDirections);
847+
flushAnimationFrames();
822848

823849
expect(getForcedDirectionButton('block-end', 'top', 0)).not.toBe(null);
824850
expect(getForcedDirectionButton('block-start', 'top', 1)).not.toBe(null);
@@ -831,6 +857,7 @@ describe('forced position behavior', () => {
831857
jest.mocked(getLogicalBoundingClientRect).mockReturnValue(position(viewport.width / 2, viewport.height - 75));
832858

833859
renderDragHandle(allDirections);
860+
flushAnimationFrames();
834861

835862
expect(getForcedDirectionButton('block-end', 'top', 0)).not.toBe(null);
836863
expect(getForcedDirectionButton('block-start', 'top', 1)).not.toBe(null);
@@ -843,6 +870,7 @@ describe('forced position behavior', () => {
843870
jest.mocked(getLogicalBoundingClientRect).mockReturnValue(position(viewport.width / 2, 25));
844871

845872
renderDragHandle({ directions: { 'inline-start': 'active', 'block-start': 'active' }, initialShowButtons: true });
873+
flushAnimationFrames();
846874

847875
expect(getForcedDirectionButton('block-start', 'bottom', 1)).not.toBe(null);
848876
expect(getForcedDirectionButton('inline-start', 'bottom', 0)).not.toBe(null);
@@ -853,6 +881,7 @@ describe('forced position behavior', () => {
853881
jest.mocked(getLogicalBoundingClientRect).mockReturnValue(position(viewport.width / 2, viewport.height - 75));
854882

855883
renderDragHandle({ directions: { 'inline-end': 'active', 'block-end': 'active' }, initialShowButtons: true });
884+
flushAnimationFrames();
856885

857886
expect(getForcedDirectionButton('block-end', 'top', 0)).not.toBe(null);
858887
expect(getForcedDirectionButton('inline-end', 'top', 1)).not.toBe(null);
@@ -863,6 +892,7 @@ describe('forced position behavior', () => {
863892
jest.mocked(getLogicalBoundingClientRect).mockReturnValue(position(25, 100));
864893

865894
renderDragHandle({ directions: { 'inline-start': 'active', 'block-end': 'active' }, initialShowButtons: true });
895+
flushAnimationFrames();
866896

867897
expect(getForcedDirectionButton('block-end', 'top', 0)).not.toBe(null);
868898
expect(getForcedDirectionButton('inline-start', 'top', 1)).not.toBe(null);
@@ -873,6 +903,7 @@ describe('forced position behavior', () => {
873903
jest.mocked(getLogicalBoundingClientRect).mockReturnValue(position(25, 100));
874904

875905
renderDragHandle(allDirections);
906+
flushAnimationFrames();
876907

877908
expect(getForcedDirectionButton('block-end', 'bottom', 3)).not.toBe(null);
878909
expect(getForcedDirectionButton('block-start', 'bottom', 2)).not.toBe(null);
@@ -888,6 +919,7 @@ describe('forced position behavior', () => {
888919
directions: { 'block-start': 'active', 'block-end': 'active', 'inline-end': 'active' },
889920
initialShowButtons: true,
890921
});
922+
flushAnimationFrames();
891923

892924
expect(getAnyForcedDirectionButton()).toBe(null);
893925
expect(getDirectionButton('block-start')).not.toBe(null);
@@ -902,6 +934,7 @@ describe('forced position behavior', () => {
902934
directions: { 'block-start': 'active', 'block-end': 'active', 'inline-start': 'active' },
903935
initialShowButtons: true,
904936
});
937+
flushAnimationFrames();
905938

906939
expect(getAnyForcedDirectionButton()).toBe(null);
907940
expect(getDirectionButton('block-start')).not.toBe(null);
@@ -916,6 +949,7 @@ describe('forced position behavior', () => {
916949
directions: { 'block-end': 'active', 'inline-start': 'active', 'inline-end': 'active' },
917950
initialShowButtons: true,
918951
});
952+
flushAnimationFrames();
919953

920954
expect(getAnyForcedDirectionButton()).toBe(null);
921955
expect(getDirectionButton('block-end')).not.toBe(null);
@@ -930,10 +964,44 @@ describe('forced position behavior', () => {
930964
directions: { 'block-start': 'active', 'inline-start': 'active', 'inline-end': 'active' },
931965
initialShowButtons: true,
932966
});
967+
flushAnimationFrames();
933968

934969
expect(getAnyForcedDirectionButton()).toBe(null);
935970
expect(getDirectionButton('block-start')).not.toBe(null);
936971
expect(getDirectionButton('inline-start')).not.toBe(null);
937972
expect(getDirectionButton('inline-end')).not.toBe(null);
938973
});
974+
975+
test('recomputes forced position when element position changes on animation frame', () => {
976+
// Use on-demand rAF execution for this test
977+
let rafCallback: FrameRequestCallback = () => {};
978+
jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb: FrameRequestCallback) => {
979+
rafCallback = cb;
980+
return 0;
981+
});
982+
983+
// Start with handle in center (no forced position needed)
984+
jest.mocked(getLogicalBoundingClientRect).mockReturnValue(position(viewport.width / 2, viewport.height / 2));
985+
986+
const { showButtons } = renderDragHandle({
987+
directions: { 'block-start': 'active', 'inline-start': 'active' },
988+
});
989+
showButtons();
990+
991+
// Execute the first animation frame - position is centered, no forced position
992+
act(() => rafCallback(0));
993+
expect(getAnyForcedDirectionButton()).toBe(null);
994+
expect(getDirectionButton('block-start')).not.toBe(null);
995+
expect(getDirectionButton('inline-start')).not.toBe(null);
996+
997+
// Simulate element moving to top edge (e.g., during CSS transition)
998+
jest.mocked(getLogicalBoundingClientRect).mockReturnValue(position(viewport.width / 2, 25));
999+
1000+
// Execute the next animation frame - should now have forced position
1001+
act(() => rafCallback(0));
1002+
expect(getForcedDirectionButton('block-start', 'bottom', 1)).not.toBe(null);
1003+
expect(getForcedDirectionButton('inline-start', 'bottom', 0)).not.toBe(null);
1004+
expect(getDirectionButton('block-start')).toBe(null);
1005+
expect(getDirectionButton('inline-start')).toBe(null);
1006+
});
9391007
});

src/internal/components/drag-handle-wrapper/index.tsx

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
import React, { useEffect, useLayoutEffect, useRef, useState } from 'react';
4+
import React, { useEffect, useRef, useState } from 'react';
55
import clsx from 'clsx';
66

77
import { nodeContains } from '@cloudscape-design/component-toolkit/dom';
@@ -184,8 +184,20 @@ export default function DragHandleWrapper({
184184
const directionsOrder = forcedPosition === 'bottom' ? [...DIRECTIONS_ORDER].reverse() : DIRECTIONS_ORDER;
185185
const visibleDirections = directionsOrder.filter(dir => directions[dir]);
186186

187-
useLayoutEffect(() => {
188-
if (showButtons && dragHandleRef.current) {
187+
// Continuously monitor position while buttons are shown to handle CSS transitions/animations.
188+
// The position needs to be recalculated as the element may animate into its final position.
189+
useEffect(() => {
190+
if (!showButtons || !dragHandleRef.current) {
191+
setForcedPosition(null);
192+
return;
193+
}
194+
195+
let frameId: number;
196+
197+
const checkPosition = () => {
198+
if (!dragHandleRef.current) {
199+
return;
200+
}
189201
const rect = getLogicalBoundingClientRect(dragHandleRef.current);
190202
const conflicts = {
191203
'block-start': rect.insetBlockStart < FORCED_POSITION_PROXIMITY_PX,
@@ -199,7 +211,14 @@ export default function DragHandleWrapper({
199211
} else {
200212
setForcedPosition(null);
201213
}
202-
}
214+
frameId = requestAnimationFrame(checkPosition);
215+
};
216+
217+
frameId = requestAnimationFrame(checkPosition);
218+
219+
return () => {
220+
cancelAnimationFrame(frameId);
221+
};
203222
}, [showButtons, visibleDirections]);
204223

205224
return (

0 commit comments

Comments
 (0)