Skip to content

Conversation

@cpmsmith
Copy link
Contributor

@cpmsmith cpmsmith commented Jul 6, 2020

This pull request:

  • Fixes a bug in an existing package
  • Updates documentation or example code

Fixes #636.

Since, in order to have an aria-controls attribute, MenuButton merely hides its Popover rather than not rendering it, the Popover is constantly running getBoundingClientRect by way of useRect whenever there's a MenuButton on the page. This change makes Popover pass the observe parameter to useRect according to its hidden prop, and adds a storybook example for demoing it.

I added the storybook in order to try and break it, i.e. by changing the location of the target while observe was false, and couldn't manage to do so.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 6, 2020

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 85c1e85:

Sandbox Source
reach/reach-ui: reach-ui Configuration

@cpmsmith
Copy link
Contributor Author

cpmsmith commented Jul 6, 2020

Here's a trace of the basic MenuButton storybook example before the change:

image

And after, with the menu open from ~2.7–5.0s:

image

@sciyoshi
Copy link

@chancestrickland @ryanflorence anything left before this PR can be merged? We've been running into this and it causes some performance issues - having a way to hide the popovers when not rendered would be great.

@chaance chaance added the Type: Enhancement General improvements or suggestions label Aug 21, 2020
@chaance chaance merged commit 0c53b58 into reach:develop Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Enhancement General improvements or suggestions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MenuButton performance issues

3 participants