From c2424eb33fc10972617930bda16657bd2975493c Mon Sep 17 00:00:00 2001 From: consistent-k <67736912+consistent-k@users.noreply.github.com> Date: Thu, 29 Aug 2024 10:30:04 +0000 Subject: [PATCH 1/5] fix: DOMRect value issues --- src/hooks/useAlign.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/hooks/useAlign.ts b/src/hooks/useAlign.ts index 6c15a09d..44bc608b 100644 --- a/src/hooks/useAlign.ts +++ b/src/hooks/useAlign.ts @@ -213,13 +213,15 @@ export default function useAlign( } else { const rect = target.getBoundingClientRect(); targetRect = { - x: rect.x, - y: rect.y, + x: rect.x || rect.left, + y: rect.y || rect.top, width: rect.width, height: rect.height, }; } const popupRect = popupElement.getBoundingClientRect(); + popupRect.x = popupRect.x || popupRect.left; + popupRect.y = popupRect.y || popupRect.top; const { clientWidth, clientHeight, From f2e92ee26ea2843eae5ae6f69f1954dd02988810 Mon Sep 17 00:00:00 2001 From: consistent-k <67736912+consistent-k@users.noreply.github.com> Date: Thu, 29 Aug 2024 11:00:49 +0000 Subject: [PATCH 2/5] fix: DOMRect value issues --- tests/rect.test.tsx | 297 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 297 insertions(+) create mode 100644 tests/rect.test.tsx diff --git a/tests/rect.test.tsx b/tests/rect.test.tsx new file mode 100644 index 00000000..6abbc229 --- /dev/null +++ b/tests/rect.test.tsx @@ -0,0 +1,297 @@ +import { act, cleanup, fireEvent, render } from '@testing-library/react'; +import { spyElementPrototypes } from 'rc-util/lib/test/domHook'; +import React from 'react'; +import type { TriggerProps, TriggerRef } from '../src'; +import Trigger from '../src'; +import { awaitFakeTimer } from './util'; + +import { _rs } from 'rc-resize-observer'; + +export const triggerResize = (target: Element) => { + act(() => { + _rs([{ target } as ResizeObserverEntry]); + }); +}; + +describe('Trigger.Rect', () => { + let targetVisible = true; + + let rectX = 100; + let rectY = 100; + let rectWidth = 100; + let rectHeight = 100; + + beforeAll(() => { + spyElementPrototypes(HTMLDivElement, { + getBoundingClientRect: () => ({ + left: rectX, + top: rectY, + width: rectWidth, + height: rectHeight, + right: 200, + bottom: 200, + }), + }); + + spyElementPrototypes(HTMLElement, { + offsetParent: { + get: () => (targetVisible ? document.body : null), + }, + }); + spyElementPrototypes(SVGElement, { + offsetParent: { + get: () => (targetVisible ? document.body : null), + }, + }); + }); + + beforeEach(() => { + targetVisible = true; + + rectX = 100; + rectY = 100; + rectWidth = 100; + rectHeight = 100; + + jest.useFakeTimers(); + }); + + afterEach(() => { + cleanup(); + jest.useRealTimers(); + }); + + it('not show', async () => { + const onAlign = jest.fn(); + + const Demo = (props: Partial) => { + const scrollRef = React.useRef(null); + + return ( + <> +
+ trigger} + getPopupContainer={() => scrollRef.current!} + {...props} + > +
+ + + ); + }; + + const { rerender, container } = render(); + const scrollDiv = container.querySelector('.scroll')!; + + const mockAddEvent = jest.spyOn(scrollDiv, 'addEventListener'); + + expect(mockAddEvent).not.toHaveBeenCalled(); + + // Visible + rerender(); + expect(mockAddEvent).toHaveBeenCalled(); + + // Scroll + onAlign.mockReset(); + fireEvent.scroll(scrollDiv); + + await awaitFakeTimer(); + expect(onAlign).toHaveBeenCalled(); + }); + + it('resize align', async () => { + const onAlign = jest.fn(); + + const { container } = render( + trigger} + > + + , + ); + + await Promise.resolve(); + onAlign.mockReset(); + + // Resize + const target = container.querySelector('.target')!; + triggerResize(target); + + await awaitFakeTimer(); + expect(onAlign).toHaveBeenCalled(); + }); + + it('placement is higher than popupAlign', async () => { + render( + } + builtinPlacements={{ + top: {}, + }} + popupPlacement="top" + popupAlign={{}} + > + + , + ); + + await awaitFakeTimer(); + + expect( + document.querySelector('.rc-trigger-popup-placement-top'), + ).toBeTruthy(); + }); + + it('invisible should not align', async () => { + const onPopupAlign = jest.fn(); + const triggerRef = React.createRef(); + + render( + } + popupAlign={{}} + onPopupAlign={onPopupAlign} + ref={triggerRef} + > + + , + ); + + await awaitFakeTimer(); + + expect(onPopupAlign).toHaveBeenCalled(); + onPopupAlign.mockReset(); + + for (let i = 0; i < 10; i += 1) { + triggerRef.current!.forceAlign(); + + await awaitFakeTimer(); + expect(onPopupAlign).toHaveBeenCalled(); + onPopupAlign.mockReset(); + } + + // Make invisible + targetVisible = false; + + triggerRef.current!.forceAlign(); + await awaitFakeTimer(); + expect(onPopupAlign).not.toHaveBeenCalled(); + }); + + it('align should merge into placement', async () => { + render( + } + builtinPlacements={{ + top: { + targetOffset: [0, 0], + }, + }} + popupPlacement="top" + popupAlign={{ + targetOffset: [-903, -1128], + }} + > + + , + ); + + await awaitFakeTimer(); + + expect( + document.querySelector('.rc-trigger-popup-placement-top'), + ).toHaveStyle({ + left: `753px`, + top: `978px`, + }); + }); + + it('targetOffset support ptg', async () => { + render( + } + popupAlign={{ + targetOffset: ['50%', '-50%'], + }} + > +
+ , + ); + + await awaitFakeTimer(); + + // Correct this if I miss understand the value calculation + // from https://github.com/yiminghe/dom-align/blob/master/src/getElFuturePos.js + expect(document.querySelector('.rc-trigger-popup')).toHaveStyle({ + left: `-50px`, + top: `50px`, + }); + }); + + it('support dynamicInset', async () => { + render( + } + popupAlign={{ + points: ['bc', 'tc'], + _experimental: { + dynamicInset: true, + }, + }} + > +
+ , + ); + + await awaitFakeTimer(); + + expect(document.querySelector('.rc-trigger-popup')).toHaveStyle({ + bottom: `100px`, + }); + }); + + it('round when decimal precision', async () => { + rectX = 22.6; + rectY = 33.4; + rectWidth = 33.7; + rectHeight = 55.9; + + render( + } + popupAlign={{ + points: ['tl', 'bl'], + }} + > +
+ , + ); + + await awaitFakeTimer(); + + expect(document.querySelector('.rc-trigger-popup')).toHaveStyle({ + top: `56px`, + }); + }); +}); From 7984a144695b4456423a95c42c78145308061334 Mon Sep 17 00:00:00 2001 From: consistent-k <67736912+consistent-k@users.noreply.github.com> Date: Fri, 30 Aug 2024 02:57:41 +0000 Subject: [PATCH 3/5] fix: DOMRect value issues --- tests/rect.test.tsx | 223 +------------------------------------------- 1 file changed, 2 insertions(+), 221 deletions(-) diff --git a/tests/rect.test.tsx b/tests/rect.test.tsx index 6abbc229..8f708bfe 100644 --- a/tests/rect.test.tsx +++ b/tests/rect.test.tsx @@ -1,18 +1,9 @@ -import { act, cleanup, fireEvent, render } from '@testing-library/react'; +import { cleanup, render } from '@testing-library/react'; import { spyElementPrototypes } from 'rc-util/lib/test/domHook'; import React from 'react'; -import type { TriggerProps, TriggerRef } from '../src'; import Trigger from '../src'; import { awaitFakeTimer } from './util'; -import { _rs } from 'rc-resize-observer'; - -export const triggerResize = (target: Element) => { - act(() => { - _rs([{ target } as ResizeObserverEntry]); - }); -}; - describe('Trigger.Rect', () => { let targetVisible = true; @@ -61,193 +52,7 @@ describe('Trigger.Rect', () => { jest.useRealTimers(); }); - it('not show', async () => { - const onAlign = jest.fn(); - - const Demo = (props: Partial) => { - const scrollRef = React.useRef(null); - - return ( - <> -
- trigger} - getPopupContainer={() => scrollRef.current!} - {...props} - > -
- - - ); - }; - - const { rerender, container } = render(); - const scrollDiv = container.querySelector('.scroll')!; - - const mockAddEvent = jest.spyOn(scrollDiv, 'addEventListener'); - - expect(mockAddEvent).not.toHaveBeenCalled(); - - // Visible - rerender(); - expect(mockAddEvent).toHaveBeenCalled(); - - // Scroll - onAlign.mockReset(); - fireEvent.scroll(scrollDiv); - - await awaitFakeTimer(); - expect(onAlign).toHaveBeenCalled(); - }); - - it('resize align', async () => { - const onAlign = jest.fn(); - - const { container } = render( - trigger} - > - - , - ); - - await Promise.resolve(); - onAlign.mockReset(); - - // Resize - const target = container.querySelector('.target')!; - triggerResize(target); - - await awaitFakeTimer(); - expect(onAlign).toHaveBeenCalled(); - }); - - it('placement is higher than popupAlign', async () => { - render( - } - builtinPlacements={{ - top: {}, - }} - popupPlacement="top" - popupAlign={{}} - > - - , - ); - - await awaitFakeTimer(); - - expect( - document.querySelector('.rc-trigger-popup-placement-top'), - ).toBeTruthy(); - }); - - it('invisible should not align', async () => { - const onPopupAlign = jest.fn(); - const triggerRef = React.createRef(); - - render( - } - popupAlign={{}} - onPopupAlign={onPopupAlign} - ref={triggerRef} - > - - , - ); - - await awaitFakeTimer(); - - expect(onPopupAlign).toHaveBeenCalled(); - onPopupAlign.mockReset(); - - for (let i = 0; i < 10; i += 1) { - triggerRef.current!.forceAlign(); - - await awaitFakeTimer(); - expect(onPopupAlign).toHaveBeenCalled(); - onPopupAlign.mockReset(); - } - - // Make invisible - targetVisible = false; - - triggerRef.current!.forceAlign(); - await awaitFakeTimer(); - expect(onPopupAlign).not.toHaveBeenCalled(); - }); - - it('align should merge into placement', async () => { - render( - } - builtinPlacements={{ - top: { - targetOffset: [0, 0], - }, - }} - popupPlacement="top" - popupAlign={{ - targetOffset: [-903, -1128], - }} - > - - , - ); - - await awaitFakeTimer(); - - expect( - document.querySelector('.rc-trigger-popup-placement-top'), - ).toHaveStyle({ - left: `753px`, - top: `978px`, - }); - }); - - it('targetOffset support ptg', async () => { - render( - } - popupAlign={{ - targetOffset: ['50%', '-50%'], - }} - > -
- , - ); - - await awaitFakeTimer(); - - // Correct this if I miss understand the value calculation - // from https://github.com/yiminghe/dom-align/blob/master/src/getElFuturePos.js - expect(document.querySelector('.rc-trigger-popup')).toHaveStyle({ - left: `-50px`, - top: `50px`, - }); - }); - - it('support dynamicInset', async () => { + it('getBoundingClientRect top and left', async () => { render( { }); }); - it('round when decimal precision', async () => { - rectX = 22.6; - rectY = 33.4; - rectWidth = 33.7; - rectHeight = 55.9; - - render( - } - popupAlign={{ - points: ['tl', 'bl'], - }} - > -
- , - ); - - await awaitFakeTimer(); - - expect(document.querySelector('.rc-trigger-popup')).toHaveStyle({ - top: `56px`, - }); - }); }); From ad3d5e6a1911cada45f840f1e48539b82b0376d8 Mon Sep 17 00:00:00 2001 From: consistent-k <67736912+consistent-k@users.noreply.github.com> Date: Fri, 30 Aug 2024 05:09:47 +0000 Subject: [PATCH 4/5] fix: DOMRect value issues --- src/hooks/useAlign.ts | 6 ++++-- tests/align.test.tsx | 2 ++ tests/arrow.test.jsx | 2 ++ tests/flip-visibleFirst.test.tsx | 12 ++++++++++++ tests/flip.test.tsx | 14 ++++++++++++++ tests/flipShift.test.tsx | 6 +++++- 6 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/hooks/useAlign.ts b/src/hooks/useAlign.ts index 44bc608b..584f6988 100644 --- a/src/hooks/useAlign.ts +++ b/src/hooks/useAlign.ts @@ -212,9 +212,11 @@ export default function useAlign( }; } else { const rect = target.getBoundingClientRect(); + rect.x = rect.x || rect.left; + rect.y = rect.y || rect.top targetRect = { - x: rect.x || rect.left, - y: rect.y || rect.top, + x: rect.x, + y: rect.y, width: rect.width, height: rect.height, }; diff --git a/tests/align.test.tsx b/tests/align.test.tsx index 9366cc7c..945b132e 100644 --- a/tests/align.test.tsx +++ b/tests/align.test.tsx @@ -26,6 +26,8 @@ describe('Trigger.Align', () => { getBoundingClientRect: () => ({ x: rectX, y: rectY, + left: rectX, + top: rectY, width: rectWidth, height: rectHeight, right: 200, diff --git a/tests/arrow.test.jsx b/tests/arrow.test.jsx index 80be36ad..24df7055 100644 --- a/tests/arrow.test.jsx +++ b/tests/arrow.test.jsx @@ -53,6 +53,8 @@ describe('Trigger.Arrow', () => { () => ({ x: 200, y: 200, + left: 200, + top: 200, width: 100, height: 50, }), diff --git a/tests/flip-visibleFirst.test.tsx b/tests/flip-visibleFirst.test.tsx index e9ef9891..f44c530f 100644 --- a/tests/flip-visibleFirst.test.tsx +++ b/tests/flip-visibleFirst.test.tsx @@ -56,6 +56,8 @@ describe('Trigger.VisibleFirst', () => { let containerRect = { x: 0, y: 0, + left: 0, + top: 0, width: 100, height: 100, }; @@ -63,6 +65,8 @@ describe('Trigger.VisibleFirst', () => { let targetRect = { x: 0, y: 0, + left: 0, + top: 0, width: 1, height: 1, }; @@ -70,6 +74,8 @@ describe('Trigger.VisibleFirst', () => { let popupRect = { x: 0, y: 0, + left: 0, + top: 0, width: 100, height: 100, }; @@ -84,18 +90,24 @@ describe('Trigger.VisibleFirst', () => { containerRect = { x: 0, y: 0, + left: 0, + top: 0, width: 100, height: 100, }; targetRect = { x: 250, y: 250, + left: 250, + top: 250, width: 1, height: 1, }; popupRect = { x: 0, y: 0, + left: 0, + top: 0, width: 100, height: 100, }; diff --git a/tests/flip.test.tsx b/tests/flip.test.tsx index fa98faee..17fb0dd4 100644 --- a/tests/flip.test.tsx +++ b/tests/flip.test.tsx @@ -58,6 +58,8 @@ describe('Trigger.Align', () => { let spanRect = { x: 0, y: 0, + left: 0, + top: 0, width: 1, height: 1, }; @@ -65,6 +67,8 @@ describe('Trigger.Align', () => { let popupRect = { x: 0, y: 0, + left: 0, + top: 0, width: 100, height: 100, }; @@ -112,12 +116,16 @@ describe('Trigger.Align', () => { spanRect = { x: 0, y: 0, + left: 0, + top: 0, width: 1, height: 1, }; popupRect = { x: 0, y: 0, + left: 0, + top: 0, width: 100, height: 100, }; @@ -253,6 +261,8 @@ describe('Trigger.Align', () => { popupRect = { x: 100, y: 1, + left: 100, + top: 1, width: 100, height: 100, }; @@ -339,6 +349,8 @@ describe('Trigger.Align', () => { ({ x: 100, y: 100, + left: 100, + top: 100, width: 300, height: 300, } as any); @@ -384,6 +396,8 @@ describe('Trigger.Align', () => { popupRect = { x: 0, y: 0, + left: 0, + top: 0, width: 200, height: 200, }; diff --git a/tests/flipShift.test.tsx b/tests/flipShift.test.tsx index ed4b3170..d9e02a03 100644 --- a/tests/flipShift.test.tsx +++ b/tests/flipShift.test.tsx @@ -79,12 +79,14 @@ const builtinPlacements = { }; describe('Trigger.Flip+Shift', () => { - let spanRect = { x: 0, y: 0, width: 0, height: 0 }; + let spanRect = { x: 0, y: 0, left: 0, top: 0, width: 0, height: 0 }; beforeEach(() => { spanRect = { x: 0, y: 100, + left: 0, + top: 100, width: 100, height: 100, }; @@ -113,6 +115,8 @@ describe('Trigger.Flip+Shift', () => { return { x: 0, y: 0, + left: 0, + top: 0, width: 100, height: 300, }; From a68a3db7a6a2c8c6dae053a5bcfd57214b02fb46 Mon Sep 17 00:00:00 2001 From: consistent-k <67736912+consistent-k@users.noreply.github.com> Date: Fri, 30 Aug 2024 05:44:18 +0000 Subject: [PATCH 5/5] fix: DOMRect value issues --- src/hooks/useAlign.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hooks/useAlign.ts b/src/hooks/useAlign.ts index 584f6988..6eacbf2f 100644 --- a/src/hooks/useAlign.ts +++ b/src/hooks/useAlign.ts @@ -212,8 +212,8 @@ export default function useAlign( }; } else { const rect = target.getBoundingClientRect(); - rect.x = rect.x || rect.left; - rect.y = rect.y || rect.top + rect.x = rect.x ?? rect.left; + rect.y = rect.y ?? rect.top targetRect = { x: rect.x, y: rect.y, @@ -222,8 +222,8 @@ export default function useAlign( }; } const popupRect = popupElement.getBoundingClientRect(); - popupRect.x = popupRect.x || popupRect.left; - popupRect.y = popupRect.y || popupRect.top; + popupRect.x = popupRect.x ?? popupRect.left; + popupRect.y = popupRect.y ?? popupRect.top; const { clientWidth, clientHeight,