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

fix: add default empty value for items prop #934

Merged
merged 1 commit into from
Nov 18, 2021
Merged

fix: add default empty value for items prop #934

merged 1 commit into from
Nov 18, 2021

Conversation

MADEiN83
Copy link
Contributor

@MADEiN83 MADEiN83 commented Nov 18, 2021

Had an issue when I use the Menu from reakit/Menu to open a modal (code base from my company).

When just after the click of an MenuItem (without any focus of elements on the opened modal) I press any key, the app crashes every time with this exception:

image

Breakpoints:
image

The property items is always undefined. So, the .find will crash. I've just added a default value for it to avoid this issue. I don't really know how this issue occurs because I can't reproduce it on my own.. but my company has this issue and we're struggling to deal with.

Don't hesitate to tell me if I forgot something.

Thanks! ;-)

@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 0567744:

Sandbox Source
Reakit Configuration

@netlify
Copy link

netlify bot commented Nov 18, 2021

✔️ Deploy Preview for reakit ready!

🔨 Explore the source changes: 0567744

🔍 Inspect the deploy log: https://app.netlify.com/sites/reakit/deploys/61961d76d33c9a0008911a91

😎 Browse the preview: https://deploy-preview-934--reakit.netlify.app

@codecov
Copy link

codecov bot commented Nov 18, 2021

Codecov Report

Merging #934 (0567744) into master (0df44f6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #934   +/-   ##
=======================================
  Coverage   94.87%   94.87%           
=======================================
  Files         231      231           
  Lines        3510     3511    +1     
  Branches      950      951    +1     
=======================================
+ Hits         3330     3331    +1     
  Misses        179      179           
  Partials        1        1           
Impacted Files Coverage Δ
packages/reakit/src/Menu/__utils/useShortcuts.ts 100.00% <100.00%> (ø)

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 0df44f6...0567744. Read the comment docs.

Copy link
Member

@diegohaz diegohaz left a comment

Choose a reason for hiding this comment

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

Thanks! ❤️

@diegohaz diegohaz merged commit c676738 into ariakit:master Nov 18, 2021
@ariakit-bot
Copy link

Thanks a lot for contributing!

Based on our community guidelines, every person who has a PR of any kind merged is offered an invitation to the Reakit organization.

Should you accept, you'll get write access to the main repository and a fancy Reakit logo on your GitHub profile. You'll be able to label and close issues, create new branches etc. Make sure to read our contribution and community guidelines, which explains all of this in more detail.

If you have any questions, let me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants