trurl-master / jsdom-testing-mocks

A set of tools for emulating browser behavior in jsdom environment
MIT License
116 stars 7 forks source link

Animations aren't captured #45

Closed mooksz closed 1 year ago

mooksz commented 1 year ago

Hi I'm not sure if I'm doing anything wrong here or its a bug. The .getAnimations always returns an empty array.

Accordion.tsx

'use client';

import { v4 as uuidv4 } from 'uuid';
import { Heading } from '@components/Heading/Heading';
import styles from './Accordion.module.scss';
import { useRef, useState } from 'react';

type AccordionProps = {
    label: React.ReactNode;
    children: React.ReactNode;
};

export const Accordion = (props: AccordionProps) => {
    const { label, children } = props;
    const detailsRef = useRef<HTMLDetailsElement>(null);
    const labelRef = useRef<HTMLElement>(null);
    const contentRef = useRef<HTMLDivElement>(null);
    const [animation, setAnimation] = useState<Animation | null>(null);
    const [isClosing, setIsClosing] = useState(false);
    const [isExpanding, setIsExpanding] = useState(false);
    const id = uuidv4();

    const onLabelClick = (e: React.MouseEvent<HTMLElement>): void => {
        e.preventDefault();

        if (!detailsRef.current || !contentRef.current) return;

        detailsRef.current.style.overflow = 'hidden';

        if (isClosing || !detailsRef.current.open) {
            open();
            return;
        }

        if (isExpanding || detailsRef.current.open) {
            shrink();
            return;
        }
    };

    const shrink = () => {
        if (!detailsRef.current || !labelRef.current || !contentRef.current) return;

        // Set the element as "being closing"
        setIsClosing(true);

        // Store the current height of the element
        const startHeight = `${detailsRef.current.offsetHeight}px`;

        // Calculate the height of the summary
        const endHeight = `${detailsRef.current.offsetHeight - contentRef.current.offsetHeight}px`;

        // If there is already an animation running
        if (animation) {
            // Cancel the current animation
            animation.cancel();
        }

        // Start a WAAPI animation
        const currentAnimation = detailsRef.current.animate(
            {
                // Set the keyframes from the startHeight to endHeight
                height: [startHeight, endHeight],
            },
            {
                duration: 400,
                easing: 'ease-in-out',
            },
        );

        // Set animation state
        setAnimation(currentAnimation);

        // When the animation is complete, call onAnimationFinish()
        currentAnimation.onfinish = () => onAnimationFinish(false);

        // If the animation is cancelled, isClosing variable is set to false
        currentAnimation.oncancel = () => setIsClosing(false);
    };

    const open = () => {
        if (!detailsRef.current) return;

        detailsRef.current.style.height = `${detailsRef.current.offsetHeight}px`;

        detailsRef.current.open = true;

        window.requestAnimationFrame(() => expand());
    };

    const expand = () => {
        if (!detailsRef.current || !labelRef.current || !contentRef.current) return;

        // Set the element as "being expanding"
        setIsExpanding(true);

        // Get the current fixed height of the element
        const startHeight = `${detailsRef.current.offsetHeight}px`;

        // Calculate the open height of the element (summary height + content height)
        const endHeight = `${detailsRef.current.offsetHeight + contentRef.current.offsetHeight}px`;

        // If there is already an animation running
        if (animation) {
            // Cancel the current animation
            animation.cancel();
        }

        // Start a WAAPI animation
        const currentAnimation = detailsRef.current.animate(
            {
                // Set the keyframes from the startHeight to endHeight
                height: [startHeight, endHeight],
            },
            {
                duration: 400,
                easing: 'ease-in-out',
            },
        );

        // Set animation state
        setAnimation(currentAnimation);

        // When the animation is complete, call onAnimationFinish()
        currentAnimation.onfinish = () => onAnimationFinish(true);
        // If the animation is cancelled, isExpanding variable is set to false
        currentAnimation.oncancel = () => setIsExpanding(false);
    };

    const onAnimationFinish = (open: boolean) => {
        if (!detailsRef.current || !labelRef.current || !contentRef.current) return;

        // Set the open attribute based on the parameter
        detailsRef.current.open = open;

        labelRef.current.setAttribute('aria-expanded', `${open}`);

        // Clear the stored animation
        setAnimation(null);

        // Reset isClosing & isExpanding
        setIsClosing(false);
        setIsExpanding(false);

        // Remove the overflow hidden and the fixed height
        detailsRef.current.style.height = detailsRef.current.style.overflow = '';
    };

    return (
        <div
            className={`${styles['accordion-wrapper']}`}
            onClick={onLabelClick}
            aria-controls={id}
            aria-expanded="false"
            role="button"
            tabIndex={0}
        >
            <details ref={detailsRef as React.LegacyRef<HTMLDetailsElement>} className={`${styles['accordion']}`}>
                <summary
                    tabIndex={-1}
                    role="decoration"
                    ref={labelRef as React.LegacyRef<HTMLElement>}
                    className={`${styles['label']}`}
                >
                    <Heading className={`${styles['heading']}`} semanticLevel={4} styledLevel={6}>
                        {label}
                    </Heading>
                </summary>

                <div id={id} ref={contentRef as React.LegacyRef<HTMLDivElement>} className={`${styles['content']}`}>
                    {children}
                </div>
            </details>
        </div>
    );
};

Accordion.test.ts

import { screen, render } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { Accordion } from '../Accordion';
import { mockAnimationsApi } from 'jsdom-testing-mocks';

mockAnimationsApi();

describe('Accordion', () => {
    it('should render', () => {
        render(<Accordion label="Label">Content</Accordion>);
        expect(screen.getByText('Label')).toBeInTheDocument();
    });

    it('should open and close', async () => {
        render(<Accordion label="Label">Content</Accordion>);

        const wrapper = screen.getByRole('button');
        const details = screen.getByRole('group');

        expect(details.getAttribute('open')).toBeNull();

        await userEvent.click(wrapper);
        const animation = details.getAnimations(); // <!-- Always empty array -->

        expect(details.getAttribute('open')).not.toBeNull();
    });
});
trurl-master commented 1 year ago

Hi there!

Thank you for the issue. After a quick glance at the code, specifically at this part:

window.requestAnimationFrame(() => expand());

my first hypothesis is that .getAnimations is being called before the expand. requestAnimationFrame in jest normally delays its callback execution by 16ms

mooksz commented 1 year ago

Hi @trurl-master,

Thanks for the quick response.

I did manage to capture the animation by just calling expand(), outside of requestAnimationFrame.

However I also had to update my test with act():

it('should open and close', async () => {
    setup(<Accordion label="Label">Content</Accordion>);

    const wrapper = screen.getByRole('button');
    const details = screen.getByRole('group');

    expect(details.getAttribute('open')).toBeNull();

    await act(async () => {
        await userEvent.click(wrapper);
    });
    const animation = details.getAnimations();    
    console.log(animation); <!-- Sees animation now -->

    expect(details.getAttribute('open')).not.toBeNull();
});
trurl-master commented 1 year ago

I did manage to capture the animation by just calling expand(), outside of requestAnimationFrame.

I don't like that you have to modify your code for the test to pass. Maybe try putting getAnimations and your assertion inside waitFor? Or, alternatively, use fake timers (which should also speed up the test)

However I also had to update my test with act():

That's a bit weird that you need wrap it in act, as all of testing lib utilities are already wrapped in it internally (as far as I know).

mooksz commented 1 year ago

I wasn't able to do it with waitFor or fake timers, both instances I wasn't able to capture the animations. I do have to add that I'm just starting to learn unit / component testing, so it be my lack of knowledge. But I basicly ran into three issues:

  1. I was unable to wrap shrink() & expand() in requestAnimationFrame().
  2. I was required to await userEvent.click(), however in all the docs I never seen await use for userEvent.
  3. I was required to wrap userEvent.click & Element.getAnimations() inside act().

As I'm not skilled enough to debug these items I did make a sample repo with the Accordion component and also an AccordionList component. Which tests if an opened Accordion closes when a new Accordion opens. All my tests are passing now, because this was the only way I was able to capture the animations and wait for de Animation.onfinish() callback to run which updates the details element attribute open that I'm testing for. And seeing this Mock was experimental it might give you some information about the case. If you'd like me to provide extra information I'd be glad to.

https://github.com/mooksz/WAAPI-Mock-Accordion-Sample

trurl-master commented 1 year ago

I looked at the code, and seems to be working correctly using waitFor if done in this manner:

await userEvent.click(wrapper);

await waitFor(async () => {
    const animation = details.getAnimations()[0];

    await animation.finished;

    expect(details.getAttribute('open')).not.toBeNull();
});
  1. I was unable to wrap shrink() & expand() in requestAnimationFrame().

I tried both with and without requestAnimationFrame, both cases work

  1. I was required to await userEvent.click(), however in all the docs I never seen await use for userEvent.

Testing Library changed their API since version 14, now you need to await all api calls and (optionally) call userEvent.setup at the beginning of each test, and use the user object it returns

  1. I was required to wrap userEvent.click & Element.getAnimations() inside act().

The act warning you're having is probably because of changing state in the finish callback. It will always be outside any act's. waitFor suppresses this warning

mooksz commented 1 year ago

@trurl-master thanks! It seemed to have fixed everything except when the user.click() function is called on a firstItemWrapper the act() wrapper testing-library provides seem to be scoped to that element. So in my case when user.click() is called on firstItemWrapper, .getAnimations() only works on that element at its children.

So this fails:

// Click on second accordion to open
await user.click(secondItemWrapper);

await waitFor(async () => {
    // Capture closing animation of first accordion (which is set by `useEffect()` state update)
    const animation = firstItemDetails.getAnimations()[0];
    // Wait for animation to finish
    await animation.finished;

    // Assert
    expect(firstItemDetails).toBeInTheDocument();
    expect(secondItemDetails).toBeInTheDocument();
    expect(firstItemDetails.getAttribute('open')).toBeNull();
    expect(secondItemDetails.getAttribute('open')).not.toBeNull();
});

But this works:

// Click on second accordion to open
await user.click(secondItemWrapper);

await act(async () => {
    // Capture closing animation of first accordion (which is set by `useEffect()` state update)
    const animation = firstItemDetails.getAnimations()[0];
    // Wait for animation to finish
    await animation.finished;
});

await waitFor(async () => {
    // Assert
    expect(firstItemDetails).toBeInTheDocument();
    expect(secondItemDetails).toBeInTheDocument();
    expect(firstItemDetails.getAttribute('open')).toBeNull();
    expect(secondItemDetails.getAttribute('open')).not.toBeNull();
});

I have updated the sample repo with a failing and passing AccordionsList test.

What also seemed very weird is that in the sample repo (a clone of my actual project and just removed a few components) I had to install @testing-library/dom because I had an error of a missing helper function. In my actual project I didn't have @testing-library/dom installed without any missing helper function errors, but with the exact same passing code from the sample repro it threw a bunch of error's, that I still had to wrap everything inside of act() functions. After installing @testing-library/dom in my real project it passed without these act() errors.

trurl-master commented 1 year ago

Hey @mooksz !

In this particular case I don't see why would you need to wait for the animation to be finished at all.

Something like

await user.click(secondItemWrapper);

await waitFor(async () => {
    expect(firstItemDetails).toBeInTheDocument();
    expect(secondItemDetails).toBeInTheDocument();
    expect(firstItemDetails.getAttribute('open')).toBeNull();
    expect(secondItemDetails.getAttribute('open')).not.toBeNull();
});

would do the job just fine. But if you need to, I think the correct way is this:

await user.click(secondItemWrapper);

await waitFor(async () => {
  const animation = firstItemDetails.getAnimations()[0]
  expect(animation).toBeDefined();
});

const animation = firstItemDetails.getAnimations()[0]

await animation.finished

expect(firstItemDetails).toBeInTheDocument();
// other assertions

I was wrong to put the await animation.finished inside waitFor earlier.

And when it comes to act, i just released a beta version that supports passing act using configMocks, this way all callbacks will be always wrapped in act.

Let me know what you think.