Skip to content

Upgrade React Transition Group #3706

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

Merged
merged 8 commits into from
Nov 18, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 83 additions & 14 deletions packages/@react-spectrum/combobox/test/ComboBox.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2024,23 +2024,55 @@ describe('ComboBox', function () {
);

let button = getByRole('button');
let combobox = getByRole('combobox');

await waitFor(() => expect(load).toHaveBeenCalledTimes(1));
expect(load).toHaveBeenCalledTimes(1);
expect(onOpenChange).toHaveBeenCalledTimes(0);
expect(onLoadMore).toHaveBeenCalledTimes(0);

// open menu
await act(async () => {
triggerPress(button);
jest.runAllTimers();
// advance to open state from Transition
jest.advanceTimersToNextTimer();
});

let listbox = getByRole('listbox');
expect(listbox).toBeVisible();
jest.spyOn(listbox, 'clientHeight', 'get').mockImplementation(() => 100);
// update size, virtualizer raf kicks in
act(() => {jest.advanceTimersToNextTimer();});
// onLoadMore queued by previous timer, run it now
act(() => {jest.advanceTimersToNextTimer();});

expect(onOpenChange).toHaveBeenCalledTimes(1);
expect(onOpenChange).toHaveBeenCalledWith(true, 'manual');
expect(onLoadMore).toHaveBeenCalledTimes(1);
await waitFor(() => expect(load).toHaveBeenCalledTimes(1));
expect(load).toHaveBeenCalledTimes(1);

if (React.version.startsWith('17') || React.version.startsWith('16')) {
// close menu
act(() => {
combobox.blur();
// on exiting
jest.advanceTimersToNextTimer();
// raf from virtualizer relayout
jest.advanceTimersToNextTimer();
// advance to onExited
jest.advanceTimersToNextTimer();
});
} else {
// close menu
act(() => {
combobox.blur();
// raf from virtualizer relayout
jest.advanceTimersToNextTimer();
});
// previous act wraps up onExiting
act(() => {
// advance to onExited
jest.advanceTimersToNextTimer();
});
}
});

it('onLoadMore is not called on when previously opened', async () => {
Expand All @@ -2052,29 +2084,58 @@ describe('ComboBox', function () {

let button = getByRole('button');
let combobox = getByRole('combobox');
await waitFor(() => expect(load).toHaveBeenCalledTimes(1));
expect(load).toHaveBeenCalledTimes(1);
expect(onOpenChange).toHaveBeenCalledTimes(0);
expect(onLoadMore).toHaveBeenCalledTimes(0);

// this call and the one below are more correct for how the code should
// behave, the inital call would have a height of zero and after that a measureable height
// behave, the initial call would have a height of zero and after that a measureable height
clientHeightSpy.mockRestore();
clientHeightSpy = jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementationOnce(() => 0).mockImplementation(() => 40);
// open menu
await act(async () => {
triggerPress(button);
jest.runAllTimers();
// advance to open state from Transition
jest.advanceTimersToNextTimer();
});
let listbox = getByRole('listbox');
expect(listbox).toBeVisible();
jest.spyOn(listbox, 'clientHeight', 'get').mockImplementation(() => 100);
// update size, virtualizer raf kicks in
act(() => {jest.advanceTimersToNextTimer();});
// onLoadMore queued by previous timer, run it now
act(() => {jest.advanceTimersToNextTimer();});

expect(onOpenChange).toHaveBeenCalledTimes(1);
expect(onOpenChange).toHaveBeenCalledWith(true, 'manual');
expect(onLoadMore).toHaveBeenCalledTimes(1);

// close menu
act(() => {
combobox.blur();
jest.runAllTimers();
});
if (React.version.startsWith('17') || React.version.startsWith('16')) {
// close menu
act(() => {
combobox.blur();
// on exiting
jest.advanceTimersToNextTimer();
// raf from virtualizer relayout
jest.advanceTimersToNextTimer();
// advance to onExited
jest.advanceTimersToNextTimer();
});
} else {
// close menu
act(() => {
combobox.blur();
// raf from virtualizer relayout
jest.advanceTimersToNextTimer();
});
// previous act wraps up onExiting
act(() => {
// advance to onExited
jest.advanceTimersToNextTimer();
});
}

expect(listbox).not.toBeInTheDocument();

expect(onOpenChange).toHaveBeenCalledTimes(2);
expect(onOpenChange).toHaveBeenLastCalledWith(false, undefined);
Expand All @@ -2085,8 +2146,16 @@ describe('ComboBox', function () {
// reopen menu
await act(async () => {
triggerPress(button);
jest.runAllTimers();
// advance to open state from Transition
jest.advanceTimersToNextTimer();
});
listbox = getByRole('listbox');
expect(listbox).toBeVisible();
jest.spyOn(listbox, 'clientHeight', 'get').mockImplementation(() => 100);
// update size, virtualizer raf kicks in
act(() => {jest.advanceTimersToNextTimer();});
// onLoadMore queued by previous timer, run it now
act(() => {jest.advanceTimersToNextTimer();});

expect(onOpenChange).toHaveBeenCalledTimes(3);
expect(onOpenChange).toHaveBeenLastCalledWith(true, 'manual');
Expand All @@ -2095,7 +2164,7 @@ describe('ComboBox', function () {
// because the browser limits the popover height via calculatePosition(),
// while the test doesn't, causing virtualizer to try to load more
expect(onLoadMore).toHaveBeenCalledTimes(2);
await waitFor(() => expect(load).toHaveBeenCalledTimes(1));
expect(load).toHaveBeenCalledTimes(1);
});
});

Expand Down
2 changes: 1 addition & 1 deletion packages/@react-spectrum/list/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
"@react-types/grid": "^3.1.4",
"@react-types/shared": "^3.15.0",
"@spectrum-icons/ui": "^3.3.3",
"react-transition-group": "^2.2.0"
"react-transition-group": "^4.4.5"
},
"devDependencies": {
"@adobe/spectrum-css-temp": "^3.0.0-alpha.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/@react-spectrum/overlays/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"@react-stately/overlays": "^3.4.2",
"@react-types/overlays": "^3.6.4",
"@react-types/shared": "^3.15.0",
"react-transition-group": "^2.2.0"
"react-transition-group": "^4.4.5"
},
"devDependencies": {
"@adobe/spectrum-css-temp": "3.0.0-alpha.1"
Expand Down
2 changes: 1 addition & 1 deletion packages/@react-spectrum/overlays/src/OpenTransition.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/

import React from 'react';
import Transition from 'react-transition-group/Transition';
import {Transition} from 'react-transition-group';

const OPEN_STATES = {
entering: false,
Expand Down
43 changes: 29 additions & 14 deletions packages/@react-stately/virtualizer/src/useVirtualizerState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/

import {Collection} from '@react-types/shared';
import {Key, useCallback, useEffect, useMemo, useState} from 'react';
import {Key, useCallback, useEffect, useMemo, useRef, useState} from 'react';
import {Layout} from './Layout';
import {Rect} from './Rect';
import {ReusableView} from './ReusableView';
Expand Down Expand Up @@ -80,22 +80,37 @@ export function useVirtualizerState<T extends object, V, W>(opts: VirtualizerPro
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

return {
let setVisibleRect = useCallback((rect) => {
virtualizer.visibleRect = rect;
}, [virtualizer]);
let startScrolling = useCallback(() => {
virtualizer.startScrolling();
setScrolling(true);
}, [virtualizer]);
let endScrolling = useCallback(() => {
virtualizer.endScrolling();
setScrolling(false);
}, [virtualizer]);

let state = useMemo(() => ({
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

memo state so it doesn't cause unnecessary onLoadMore calls, this appeared after blurring the combobox and it beginning to exit in 16 and 17

virtualizer,
visibleViews,
setVisibleRect: useCallback((rect) => {
virtualizer.visibleRect = rect;
}, [virtualizer]),
setVisibleRect,
contentSize,
isAnimating,
isScrolling,
startScrolling: useCallback(() => {
virtualizer.startScrolling();
setScrolling(true);
}, [virtualizer]),
endScrolling: useCallback(() => {
virtualizer.endScrolling();
setScrolling(false);
}, [virtualizer])
};
startScrolling,
endScrolling
}), [
virtualizer,
visibleViews,
setVisibleRect,
contentSize,
isAnimating,
isScrolling,
startScrolling,
endScrolling
]);

return state;
}
30 changes: 13 additions & 17 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,7 @@
core-js-pure "^3.0.0"
regenerator-runtime "^0.13.4"

"@babel/[email protected]", "@babel/runtime@^7.0.0", "@babel/runtime@^7.1.2", "@babel/runtime@^7.10.2", "@babel/runtime@^7.12.1", "@babel/runtime@^7.12.5", "@babel/runtime@^7.17.8", "@babel/runtime@^7.18.9", "@babel/runtime@^7.5.0", "@babel/runtime@^7.6.2", "@babel/runtime@^7.7.6", "@babel/runtime@^7.8.4", "@babel/runtime@^7.9.2":
"@babel/[email protected]", "@babel/runtime@^7.0.0", "@babel/runtime@^7.10.2", "@babel/runtime@^7.12.1", "@babel/runtime@^7.12.5", "@babel/runtime@^7.17.8", "@babel/runtime@^7.18.9", "@babel/runtime@^7.5.0", "@babel/runtime@^7.5.5", "@babel/runtime@^7.6.2", "@babel/runtime@^7.7.6", "@babel/runtime@^7.8.4", "@babel/runtime@^7.8.7", "@babel/runtime@^7.9.2":
version "7.12.5"
resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.12.5.tgz#410e7e487441e1b360c29be715d870d9b985882e"
integrity sha512-plcc+hbExy3McchJCEQG3knOsuh3HH+Prx1P6cLIkET/0dLuQDEnrT+s27Axgc9bqfsmNUNHfscgMUdBpC9xfg==
Expand Down Expand Up @@ -8687,12 +8687,13 @@ dom-converter@^0.2:
dependencies:
utila "~0.4"

dom-helpers@^3.4.0:
version "3.4.0"
resolved "https://registry.yarnpkg.com/dom-helpers/-/dom-helpers-3.4.0.tgz#e9b369700f959f62ecde5a6babde4bccd9169af8"
integrity sha512-LnuPJ+dwqKDIyotW1VzmOZ5TONUN7CwkCR5hrgawTUbkBGYdeoNLZo6nNfGkCrjtE1nXXaj7iMMpDa8/d9WoIA==
dom-helpers@^5.0.1:
version "5.2.1"
resolved "https://registry.yarnpkg.com/dom-helpers/-/dom-helpers-5.2.1.tgz#d9400536b2bf8225ad98fe052e029451ac40e902"
integrity sha512-nRCa7CK3VTrM2NmGkIy4cbK7IZlgBE/PYMn55rrXefr5xXDP0LdtfPnblFDoVdcAfslJ7or6iqAUnx0CCGIWQA==
dependencies:
"@babel/runtime" "^7.1.2"
"@babel/runtime" "^7.8.7"
csstype "^3.0.2"

dom-serializer@0:
version "0.2.2"
Expand Down Expand Up @@ -18618,11 +18619,6 @@ react-is@^16.12.0, react-is@^16.13.1, react-is@^16.8.6:
resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.13.1.tgz#789729a4dc36de2999dc156dd6c1d9c18cea56a4"
integrity sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ==

react-lifecycles-compat@^3.0.4:
version "3.0.4"
resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362"
integrity sha512-fBASbA6LnOU9dOU2eW7aQ8xmYBSXUIWr+UmF9b1efZBazGNO+rcXT/icdKnYm2pTwcRylVUYwW7H1PHfLekVzA==

react-lowlight@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/react-lowlight/-/react-lowlight-2.0.0.tgz#3c9fa072c6516add8552f9470287e24e13d38eff"
Expand Down Expand Up @@ -18656,15 +18652,15 @@ react-test-renderer@^16.9.0:
react-is "^16.8.6"
scheduler "^0.16.2"

react-transition-group@^2.2.0:
version "2.9.0"
resolved "https://registry.yarnpkg.com/react-transition-group/-/react-transition-group-2.9.0.tgz#df9cdb025796211151a436c69a8f3b97b5b07c8d"
integrity sha512-+HzNTCHpeQyl4MJ/bdE0u6XRMe9+XG/+aL4mCxVN4DnPBQ0/5bfHWPDuOZUzYdMj94daZaZdCCc1Dzt9R/xSSg==
react-transition-group@^4.4.5:
version "4.4.5"
resolved "https://registry.yarnpkg.com/react-transition-group/-/react-transition-group-4.4.5.tgz#e53d4e3f3344da8521489fbef8f2581d42becdd1"
integrity sha512-pZcd1MCJoiKiBR2NRxeCRg13uCXbydPnmB4EOeRrY7480qNWO8IIgQG6zlDkm6uRMsURXPuKq0GWtiM59a5Q6g==
dependencies:
dom-helpers "^3.4.0"
"@babel/runtime" "^7.5.5"
dom-helpers "^5.0.1"
loose-envify "^1.4.0"
prop-types "^15.6.2"
react-lifecycles-compat "^3.0.4"

react@^18.0.0:
version "18.1.0"
Expand Down