Skip to content

WIP: fix(Toast): resume timers if no longer over toast region after a toast is removed #7681

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

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
49 changes: 47 additions & 2 deletions packages/@react-aria/toast/src/useToastRegion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,56 @@ export function useToastRegion<T>(props: AriaToastRegionProps, state: ToastState
'aria-label': props['aria-label'] || stringFormatter.format('notifications', {count: state.visibleToasts.length})
}, ref);

let isHovered = useRef(false);
let pointerPosition = useRef({x: 0, y: 0});

let {hoverProps} = useHover({
onHoverStart: state.pauseAll,
onHoverEnd: state.resumeAll
onHoverStart: () => {
isHovered.current = true;
state.pauseAll();
},
onHoverEnd: () => {
isHovered.current = false;
state.resumeAll();
}
});

// If a toast is removed while we're hovered, check whether the pointer is still over the toast region.
// If not, resume all timers (handles case where onHoverEnd is not called).
let prevToastCount = useRef(state.visibleToasts.length);
useEffect(() => {
let currentCount = state.visibleToasts.length;
let prevCount = prevToastCount.current;
if (currentCount < prevCount && ref.current) {
if (isHovered.current) {
let rect = ref.current.getBoundingClientRect();
let {x, y} = pointerPosition.current;
let isOutside = x < rect.left || x > rect.right || y < rect.top || y > rect.bottom;
if (isOutside) {
state.resumeAll();
}
} else {
// If not stuck in hovered state (e.g. closed via touch), resume all timers.
state.resumeAll();
}
}
prevToastCount.current = currentCount;
}, [ref, state]);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}, [ref, state]);
}, [ref, state.visibleToasts, state.resumeAll]);

Copy link
Member Author

Choose a reason for hiding this comment

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

eslint[react-hooks/exhaustive-deps] still wants state here 🤔

Copy link
Member

Choose a reason for hiding this comment

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

huh.... ok then i guess, that's so weird
could always pull them off the state object beforehand, the unfortunate part is that the state object will likely always be new, at least, until the react compiler can run on that file


useEffect(() => {
let onPointerMove = (e: PointerEvent) => {
pointerPosition.current = {x: e.clientX, y: e.clientY};
};
if (prevToastCount.current === 1 && state.visibleToasts.length === 2) {
document.addEventListener('pointermove', onPointerMove);
} else if (prevToastCount.current === 2 && state.visibleToasts.length === 1) {
document.removeEventListener('pointermove', onPointerMove);
}
return () => {
document.removeEventListener('pointermove', onPointerMove);
};
}, [state.visibleToasts]);

// Manage focus within the toast region.
// If a focused containing toast is removed, move focus to the next toast, or the previous toast if there is no next toast.
// We might be making an assumption with how this works if someone implements the priority queue differently, or
Expand Down
112 changes: 111 additions & 1 deletion packages/@react-spectrum/toast/test/ToastContainer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,25 @@
* governing permissions and limitations under the License.
*/

import {act, fireEvent, pointerMap, render, within} from '@react-spectrum/test-utils-internal';
import {act, fireEvent, installPointerEvent, pointerMap, render, within} from '@react-spectrum/test-utils-internal';
import {Button} from '@react-spectrum/button';
import {clearToastQueue, ToastContainer, ToastQueue} from '../src/ToastContainer';
import {defaultTheme} from '@adobe/react-spectrum';
import {Provider} from '@react-spectrum/provider';
import React, {useState} from 'react';
import userEvent from '@testing-library/user-event';

function pointerEvent(type, opts) {
let evt = new Event(type, {bubbles: true, cancelable: true});
Object.assign(evt, {
ctrlKey: false,
metaKey: false,
shiftKey: false,
button: opts.button || 0
}, opts);
return evt;
}

function RenderToastButton(props = {}) {
return (
<div>
Expand All @@ -40,6 +51,8 @@ function renderComponent(contents) {
}

describe('Toast Provider and Container', function () {
installPointerEvent();

let user;
beforeAll(() => {
user = userEvent.setup({delay: null, pointerMap});
Expand Down Expand Up @@ -408,4 +421,101 @@ describe('Toast Provider and Container', function () {
let region = getByRole('region');
expect(region).toHaveAttribute('aria-label', 'Toasts');
});

// TODO
it.skip('resumes timers if user closes the top toast while hovered and pointer ends up outside region', async () => {
// Mock getBoundingClientRect so we can simulate the toast region shrinking
let boundingRect = {
left: 0, top: 0, width: 300, height: 300,
right: 300, bottom: 300, x: 0, y: 0,
toJSON() {}
};
let rectSpy = jest
.spyOn(HTMLElement.prototype, 'getBoundingClientRect')
.mockImplementation(() => boundingRect);

let {getAllByRole, getByRole} = render(
<Provider theme={defaultTheme}>
<ToastContainer />
<Button onPress={() => ToastQueue.neutral('Test toast', {timeout: 5000})}>
Show Toast
</Button>
</Provider>
);

let button = getByRole('button');

await user.click(button);
await user.click(button);

let toasts = getAllByRole('alertdialog');
expect(toasts).toHaveLength(2);

// Advance timeout by 1s
act(() => jest.advanceTimersByTime(1000));

// Simulate hover over the toast region (causing timers to pause)
let region = getByRole('region');
fireEvent(region, pointerEvent('pointerenter', {pointerType: 'mouse'}));
fireEvent(region, pointerEvent('pointermove', {pointerType: 'mouse'}));

// Timeout shouldn't expire since we're paused
act(() => jest.advanceTimersByTime(6000));
expect(toasts).toHaveLength(2);

// Close the top toast
let closeButton = within(toasts[0]).getByRole('button');
await user.click(closeButton);

// Simulate the toast region shrinking so that the pointer is outside
boundingRect = {
left: 0, top: 0, width: 100, height: 100,
right: 100, bottom: 100, x: 0, y: 0,
toJSON() {}
};

expect(getAllByRole('alertdialog')).toHaveLength(1);

// Let the timeout expire
act(() => jest.advanceTimersByTime(5000));

expect(getAllByRole('alertdialog')).toHaveLength(0);

rectSpy.mockRestore();
});

// TODO
it.skip('resumes timers if a toast is closed after no pointermove event (i.e. touch) ', async () => {
let {getAllByRole, getByRole} = render(
<Provider theme={defaultTheme}>
<ToastContainer />
<Button onPress={() => ToastQueue.info('Another toast', {timeout: 5000})}>
Show Toast
</Button>
</Provider>
);

let button = getByRole('button');

await user.click(button);
await user.click(button);
let toasts = getAllByRole('alertdialog');
expect(toasts).toHaveLength(2);

// Advance timeout by 1s
act(() => jest.advanceTimersByTime(1000));

let closeButton = within(toasts[0]).getByRole('button');
await user.click(closeButton);

expect(getAllByRole('alertdialog')).toHaveLength(1);

// Advance timer so the second toast times out.
act(() => jest.advanceTimersByTime(6000));

toasts = getAllByRole('alertdialog');
expect(toasts).toHaveLength(1);

expect(getAllByRole('alertdialog')).toHaveLength(0);
});
});