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

BugFix: safely get the route for onIndexChange #10721

Closed
wants to merge 1 commit into from

Conversation

jafar-jabr
Copy link

Motivation

sometimes onIndexChange throws exception "can not read property name of undefined"

Test plan

Not a big changes, it will not break nothing.

@github-actions
Copy link

Hey jafar-jabr! Thanks for opening your first pull request in this repo. If you haven't already, make sure to read our contribution guidelines.

@netlify
Copy link

netlify bot commented Jul 29, 2022

Deploy Preview for react-navigation-example ready!

Name Link
🔨 Latest commit 47e6b30
🔍 Latest deploy log https://app.netlify.com/sites/react-navigation-example/deploys/62e97d062a50a20008344fe8
😎 Deploy Preview https://deploy-preview-10721--react-navigation-example.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@kacperkapusciak
Copy link
Member

Hey @jafar-jabr 👋
Thanks for the PR! It looks like the CI is failing with linting errors. Could you fix them?

You can use yarn lint to run the linter locally.

Cheers

@codecov-commenter
Copy link

Codecov Report

Merging #10721 (47e6b30) into main (818862b) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main   #10721      +/-   ##
==========================================
- Coverage   74.85%   74.82%   -0.03%     
==========================================
  Files         167      167              
  Lines        5122     5124       +2     
  Branches     1981     1982       +1     
==========================================
  Hits         3834     3834              
- Misses       1250     1252       +2     
  Partials       38       38              
Impacted Files Coverage Δ
...material-top-tabs/src/views/MaterialTopTabView.tsx 60.00% <0.00%> (-9.24%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us.

Copy link
Member

@satya164 satya164 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. However, we can't merge this as this is a workaround that masks the actual issue: i.e. the index is out of range.

Please open an issue with a repro that replicates this issue instead.

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

Successfully merging this pull request may close these issues.

None yet

4 participants