Skip to content
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

chore: Add disclosure-animated example #981

Closed
wants to merge 6 commits into from

Conversation

johnsonthedev
Copy link
Contributor

@johnsonthedev johnsonthedev commented Jan 13, 2022

Screen Shot 2022-01-14 at 10 19 24

I tried to use tailwinds transition utilities but they don't have an effect. I know you customized the tailwind.config. Could it be that this disabled the transition & animation classes?

@vercel
Copy link

vercel bot commented Jan 13, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

ariakit – ./

🔍 Inspect: https://vercel.com/ariakit/ariakit/4CXXCRWtStgeRPsbF34xfKG3X1dD
✅ Preview: https://ariakit-git-example-disclosure-animated-ariakit.vercel.app

reakit – ./

🔍 Inspect: https://vercel.com/ariakit/reakit/8ctKitoLVZGKL7vEbcYoWT2GAnHX
✅ Preview: Canceled

[Deployment for 043d702 canceled]

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 13, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@diegohaz
Copy link
Member

diegohaz commented Jan 13, 2022

It's okay not to use Tailwind for animations here. I think it would add specific -tw-variable references to the generated style.css file, which is also not great for the documentation.

@codecov
Copy link

codecov bot commented Jan 14, 2022

Codecov Report

Merging #981 (0d73680) into main (d54a6a0) will decrease coverage by 15.52%.
The diff coverage is 100.00%.

❗ Current head 0d73680 differs from pull request most recent head 043d702. Consider uploading reports for the commit 043d702 to get more accurate results

@@             Coverage Diff             @@
##             main     #981       +/-   ##
===========================================
- Coverage   80.73%   65.20%   -15.53%     
===========================================
  Files         229      175       -54     
  Lines        5699     4294     -1405     
  Branches     1584     1176      -408     
===========================================
- Hits         4601     2800     -1801     
- Misses       1094     1493      +399     
+ Partials        4        1        -3     
Impacted Files Coverage Δ
...closure/__examples__/disclosure-animated/index.tsx 100.00% <100.00%> (ø)
packages/ariakit/src/toolbar/__utils.ts 0.00% <0.00%> (-100.00%) ⬇️
packages/ariakit/src/toolbar/toolbar.tsx 0.00% <0.00%> (-100.00%) ⬇️
packages/ariakit/src/menu/menu-bar-state.ts 0.00% <0.00%> (-100.00%) ⬇️
packages/ariakit/src/toolbar/toolbar-item.ts 0.00% <0.00%> (-100.00%) ⬇️
packages/ariakit/src/toolbar/toolbar-state.ts 0.00% <0.00%> (-100.00%) ⬇️
packages/ariakit/src/toolbar/toolbar-separator.ts 0.00% <0.00%> (-100.00%) ⬇️
...iakit/src/dialog/__utils/prepend-hidden-dismiss.ts 0.00% <0.00%> (-100.00%) ⬇️
...alog/__utils/disable-accessibility-tree-outside.ts 0.00% <0.00%> (-93.75%) ⬇️
...ges/ariakit/src/hovercard/hovercard-disclosure.tsx 7.40% <0.00%> (-92.60%) ⬇️
... and 140 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d54a6a0...043d702. Read the comment docs.

@johnsonthedev
Copy link
Contributor Author

Don't know if this is what you expected from an animated example :) Still learning all of this :)

I tried to test the animation but it failed. It throws that content is visible, while it's not. I guessed it's because of the timeout delay and added a sleep function, but no effect.

Here is what I did:

const sleep = (ms:number) => new Promise((resolve) => setTimeout(resolve, ms))

test("show/hide on click", async () => {
 render(<Example />);
 expect(getContent()).not.toBeVisible();
 expect(getDisclosure()).toHaveAttribute("aria-expanded", "false");

 await click(getDisclosure());
 expect(getContent()).toBeVisible();
 expect(getDisclosure()).toHaveAttribute("aria-expanded", "true");

 await click(getDisclosure());
 await sleep(3000);
 //Here it throws that content is still visible
 expect(getContent()).not.toBeVisible();
 expect(getDisclosure()).toHaveAttribute("aria-expanded", "false");
});

@diegohaz
Copy link
Member

Still learning all of this :)

That's what matters! :)

I tried to test the animation but it failed. It throws that content is visible, while it's not. I guessed it's because of the timeout delay and added a sleep function, but no effect.

I'm not sure if JSDOM supports animation, but that sleep won't work anyway. Look at other tests that are using waitFor. That's what you need.

@johnsonthedev
Copy link
Contributor Author

Still learning all of this :)

That's what matters! :)

I tried to test the animation but it failed. It throws that content is visible, while it's not. I guessed it's because of the timeout delay and added a sleep function, but no effect.

I'm not sure if JSDOM supports animation, but that sleep won't work anyway. Look at other tests that are using waitFor. That's what you need.

failed as well. I guess it won't be supported then

  await click(getDisclosure());
  await waitFor(expect(getContent()).not.toBeVisible, {
    timeout: 2000,
  });

@johnsonthedev johnsonthedev marked this pull request as ready for review January 17, 2022 11:28
@diegohaz
Copy link
Member

That's the case where we'll probably need e2e tests. I was thinking about adding e2e test support for examples as well (they could be e2e.ts files inside the example folder) using BrowserStack, but still haven't had the time to explore this.

@diegohaz diegohaz changed the base branch from v2 to master February 10, 2022 22:45
@saideepesh000 saideepesh000 mentioned this pull request Feb 16, 2022
69 tasks
@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 043d702:

Sandbox Source
Ariakit Configuration

@stale
Copy link

stale bot commented Oct 15, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 15, 2022
@diegohaz diegohaz removed the stale label Oct 16, 2022
@stale
Copy link

stale bot commented May 21, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 21, 2023
@stale stale bot closed this Aug 12, 2023
@diegohaz diegohaz deleted the example/disclosure-animated branch August 12, 2023 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants