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 #130 and #114 -- removing tabs error #131

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stefanuros
Copy link

@stefanuros stefanuros commented Oct 30, 2019

Fix #130 and #114 as they stem from the same issue of index checking elements that are out of bounds. This fix will prevent crashes when closing tabs and modals.

@codecov-io
Copy link

codecov-io commented Oct 30, 2019

Codecov Report

Merging #131 into master will decrease coverage by 0.44%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #131      +/-   ##
==========================================
- Coverage   87.27%   86.82%   -0.45%     
==========================================
  Files          20       20              
  Lines         330      334       +4     
  Branches       63       65       +2     
==========================================
+ Hits          288      290       +2     
- Misses         34       36       +2     
  Partials        8        8
Impacted Files Coverage Δ
src/TabList.js 75.93% <50%> (-0.81%) ⬇️

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 d85c192...15db15f. Read the comment docs.

@ctxhou
Copy link
Owner

ctxhou commented Nov 1, 2019

Hey @stefanuros! Thanks for the contribution!
Would you mind add a test case here as well?

@anjalikk14
Copy link

Since it's been almost a year, seems like this isn't going to be updated. Can we merge the change in without the test case?

@matAlmeida
Copy link

matAlmeida commented Oct 19, 2020

Looks like @stefanuros had abandoned it. I tried to create some tests here but no success yet. Someone had progress? @anjalikk14 did you accomplished something?

Could you (@ctxhou) give me some clue in how to approach it?

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.

Closing a modal with no tabs in TabList results in an error
5 participants