Repository navigation
fix(design-system): keep resizing when the cursor crosses an iframe #531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| import { fireEvent, render, screen } from '@testing-library/react'; | ||
| import { afterEach, describe, expect, it } from 'vitest'; | ||
| import { HeightResizer } from './index'; | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. RUN v5.0.3 /home/tornado-softwares/Bureau/projects/JS/intlayer/packages/@intlayer/design-system
✓ src/components/HeightResizer/heightResizer.test.tsx (2 tests) 124ms
✓ HeightResizer (2)
✓ disables pointer events on iframes while dragging and restores them on release 108ms
✓ restores iframe pointer events when unmounted mid-drag 13ms
Test Files 1 passed (1)
Tests 2 passed (2)
Start at 21:12:16
Duration 829ms (environment 65%, tests 17%, import 10%, transform 6%, setup 1%, worker 1%)Ok |
||
| afterEach(() => { | ||
| for (const iframe of Array.from(document.querySelectorAll('iframe'))) { | ||
| iframe.remove(); | ||
| } | ||
| }); | ||
|
|
||
| describe('HeightResizer', () => { | ||
| it('disables pointer events on iframes while dragging and restores them on release', () => { | ||
| // Same iframe-swallows-pointer-events issue as WithResizer (#524). | ||
| const iframe = document.createElement('iframe'); | ||
| document.body.appendChild(iframe); | ||
|
|
||
| render( | ||
| <HeightResizer initialHeight={200}> | ||
| <div>content</div> | ||
| </HeightResizer> | ||
| ); | ||
|
|
||
| fireEvent.mouseDown(screen.getByRole('slider'), { clientY: 100 }); | ||
| expect(iframe.style.pointerEvents).toBe('none'); | ||
|
|
||
| fireEvent.mouseUp(window); | ||
| expect(iframe.style.pointerEvents).toBe(''); | ||
| }); | ||
|
|
||
| it('restores iframe pointer events when unmounted mid-drag', () => { | ||
| const iframe = document.createElement('iframe'); | ||
| document.body.appendChild(iframe); | ||
|
|
||
| const { unmount } = render( | ||
| <HeightResizer initialHeight={200}> | ||
| <div>content</div> | ||
| </HeightResizer> | ||
| ); | ||
|
|
||
| fireEvent.mouseDown(screen.getByRole('slider'), { clientY: 100 }); | ||
| expect(iframe.style.pointerEvents).toBe('none'); | ||
|
|
||
| unmount(); | ||
| expect(iframe.style.pointerEvents).toBe(''); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| import { fireEvent, render } from '@testing-library/react'; | ||
| import { afterEach, describe, expect, it } from 'vitest'; | ||
| import { WithResizer } from './index'; | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. RUN v5.0.3 /home/tornado-softwares/Bureau/projects/JS/intlayer/packages/@intlayer/design-system
✓ src/components/WithResizer/withResizer.test.tsx (2 tests) 140ms
✓ WithResizer (2)
✓ disables pointer events on iframes while dragging and restores them on release 124ms
✓ restores iframe pointer events when unmounted mid-drag 13ms
Test Files 1 passed (1)
Tests 2 passed (2)
Start at 21:10:13
Duration 1.14s (environment 72%, tests 13%, import 8%, transform 5%, setup 1%, worker 1%)Ok, Your tests pass and seem relevant to me. |
||
| const getHandle = (container: HTMLElement): HTMLElement => { | ||
| const slider = container.querySelector('[role="slider"]'); | ||
| if (!slider?.lastElementChild) throw new Error('resize handle not found'); | ||
| return slider.lastElementChild as HTMLElement; | ||
| }; | ||
|
|
||
| afterEach(() => { | ||
| for (const iframe of Array.from(document.querySelectorAll('iframe'))) { | ||
| iframe.remove(); | ||
| } | ||
| }); | ||
|
|
||
| describe('WithResizer', () => { | ||
| it('disables pointer events on iframes while dragging and restores them on release', () => { | ||
| // Iframes swallow pointer events once the cursor crosses them, which used | ||
| // to freeze the drag and lose the release over the iframe (#524). | ||
| const iframe = document.createElement('iframe'); | ||
| document.body.appendChild(iframe); | ||
| const styledIframe = document.createElement('iframe'); | ||
| styledIframe.style.pointerEvents = 'auto'; | ||
| document.body.appendChild(styledIframe); | ||
|
|
||
| const { container } = render( | ||
| <WithResizer initialWidth={300}> | ||
| <div>panel</div> | ||
| </WithResizer> | ||
| ); | ||
|
|
||
| fireEvent.mouseDown(getHandle(container), { clientX: 300 }); | ||
|
|
||
| expect(iframe.style.pointerEvents).toBe('none'); | ||
| expect(styledIframe.style.pointerEvents).toBe('none'); | ||
|
|
||
| fireEvent.mouseUp(window); | ||
|
|
||
| expect(iframe.style.pointerEvents).toBe(''); | ||
| expect(styledIframe.style.pointerEvents).toBe('auto'); | ||
| }); | ||
|
|
||
| it('restores iframe pointer events when unmounted mid-drag', () => { | ||
| const iframe = document.createElement('iframe'); | ||
| document.body.appendChild(iframe); | ||
|
|
||
| const { container, unmount } = render( | ||
| <WithResizer initialWidth={300}> | ||
| <div>panel</div> | ||
| </WithResizer> | ||
| ); | ||
|
|
||
| fireEvent.mouseDown(getHandle(container), { clientX: 300 }); | ||
| expect(iframe.style.pointerEvents).toBe('none'); | ||
|
|
||
| unmount(); | ||
| expect(iframe.style.pointerEvents).toBe(''); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import { useCallback, useRef } from 'react'; | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, it seems generally fine, except for the fact that if multiple resizers are used on a single page, I’m concerned that this hook might conflict with other instances of itself I’m not entirely sure, though; perhaps this behavior should be tested. |
||
| /** | ||
| * Shield against iframes swallowing pointer events during drag interactions. | ||
| * | ||
| * While dragging (e.g. resizing a panel), once the cursor crosses an iframe, | ||
| * `mousemove` / `mouseup` are dispatched to the iframe's document instead of | ||
| * the parent window — so listeners attached to `window` stop firing, the drag | ||
| * freezes, and releasing the mouse over the iframe is never seen. | ||
| * | ||
| * Call `disableIframePointerEvents` when the interaction starts to set | ||
| * `pointer-events: none` on every iframe in the document (previous inline | ||
| * values are saved), and `restoreIframePointerEvents` when it ends — including | ||
| * on unmount mid-interaction — to put them back. | ||
| * | ||
| * @returns The `disableIframePointerEvents` / `restoreIframePointerEvents` callbacks. | ||
| * | ||
| * @example | ||
| * ```tsx | ||
| * const { disableIframePointerEvents, restoreIframePointerEvents } = | ||
| * useIframePointerEventsShield(); | ||
| * | ||
| * const startDragging = () => { | ||
| * disableIframePointerEvents(); | ||
| * // attach window listeners… | ||
| * }; | ||
| * const stopDragging = () => { | ||
| * restoreIframePointerEvents(); | ||
| * // detach window listeners… | ||
| * }; | ||
| * ``` | ||
| */ | ||
| export const useIframePointerEventsShield = () => { | ||
| const savedPointerEventsRef = useRef< | ||
| Map<HTMLIFrameElement, string> | undefined | ||
| >(undefined); | ||
|
|
||
| /** Disable pointer events on all iframes so the drag keeps tracking over them. */ | ||
| const disableIframePointerEvents = useCallback(() => { | ||
| const saved = new Map<HTMLIFrameElement, string>(); | ||
| for (const iframe of Array.from(document.querySelectorAll('iframe'))) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This can create side effects, but it shouldn't be an issue for us, provided the state is restored at the end of the drag operation. |
||
| saved.set(iframe, iframe.style.pointerEvents); | ||
| iframe.style.pointerEvents = 'none'; | ||
| } | ||
| savedPointerEventsRef.current = saved; | ||
| }, []); | ||
|
|
||
| /** Restore the pointer events saved when the interaction started. */ | ||
| const restoreIframePointerEvents = useCallback(() => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is called correctly in our case, but if someone else uses this function and calls it a second time, it appears to overwrite the previous state.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry I merged quickly and did not take consideration of that comment @tornado-softwares Let's see how it can be fixed |
||
| const saved = savedPointerEventsRef.current; | ||
| if (!saved) return; | ||
| for (const [iframe, pointerEvents] of saved) { | ||
| iframe.style.pointerEvents = pointerEvents; | ||
| } | ||
| savedPointerEventsRef.current = undefined; | ||
| }, []); | ||
|
|
||
| return { disableIframePointerEvents, restoreIframePointerEvents }; | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, why not..
We should check that this works in Github CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RUN v5.0.3 /home/tornado-softwares/Bureau/projects/JS/intlayer ✓ packages/@intlayer/design-system/src/api/pushDictionariesInBatches.test.ts (6 tests) 20ms ✓ pushDictionariesInBatches (6) ✓ pushes every dictionary in batches and merges the results 4ms ✓ keeps at most PUSH_CONCURRENCY requests in flight 11ms ✓ reports the progress of the push 1ms ✓ keeps pushing when a batch throws, and reports its dictionaries 1ms ✓ throws when every batch fails 1ms ✓ reports a failed batch response 0ms Test Files 1 passed (1) Tests 6 passed (6) Start at 21:05:15 Duration 729ms (transform 67%, import 30%, tests 3%, worker 1%)Okay !