search

canonical / react-components

A set of components based on Vanilla Framework
https://canonical.github.io/react-components
95 stars 51 forks source link

feat: Add tooltip delay(500ms by default) after mouseover #998

Closed akmittal closed 9 months ago

akmittal commented 10 months ago

Done

QA

Storybook

To see rendered examples of all react-components, run:

yarn start

QA in your project

from react-components run:

yarn build
npm pack

Install the resulting tarball in your project with:

yarn add <path-to-tarball>

QA steps

Fixes

Fixes: #975 .

webteam-app commented 10 months ago

akmittal is not a collaborator of the repo

akmittal commented 10 months ago

Thanks a lot for this PR @akmittal!

I've left a few comments and it would also be good to add a few tests:

  • Check that the portal is not opened until the delay time.
  • Check that blur before the delay time cancels the timeout.
  • Check that setting the delay to 0 causes the tooltip to appear instantly.

To write those tests you will need to use Jest's fake timers.

Thanks for reviewing. I have resolved review comments. Unit tests are also complete. please re-review @huwshimi

akmittal commented 9 months ago

This is looking great. One last issue: when the tests run they have an error Warning: An update to Tooltip inside a test was not wrapped in act(...) on most of the tests. To fix this you'll need to wrap the events that open the tooltip in await act(async ...) eg.

    await act(async () => {
      await user.hover(screen.getByRole("button", { name: "Child" }));
    });

Completed the changes as suggested

huwshimi commented 9 months ago

@akmittal thanks a lot for those fixes. The test run is showing some more act() warnings: https://github.com/canonical/react-components/actions/runs/6961479419/job/18948453606?pr=998#step:7:9.

I really appreciate you're work on this PR, it's very close to being complete, but I'm happy to fix up those act calls if you'd like me to.

akmittal commented 9 months ago

@akmittal thanks a lot for those fixes. The test run is showing some more act() warnings: https://github.com/canonical/react-components/actions/runs/6961479419/job/18948453606?pr=998#step:7:9.

I really appreciate you're work on this PR, it's very close to being complete, but I'm happy to fix up those act calls if you'd like me to.

Yes please go ahead