Kylart / KawAnime

Desktop app for anime fans
MIT License
664 stars 50 forks source link

Fix undefined in history #49

Closed ghost closed 5 years ago

ghost commented 5 years ago

The history modal adds an 'undefined' to the bottom, as you can see here: screenshot_2018-09-03_17-15-39

This should get rid of it.

codecov-io commented 5 years ago

Codecov Report

Merging #49 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master     #49   +/-   ##
======================================
  Coverage    89.4%   89.4%           
======================================
  Files          25      25           
  Lines         406     406           
======================================
  Hits          363     363           
  Misses         43      43

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 be5c689...d2c67b5. Read the comment docs.

Kylart commented 5 years ago

mmmh I'm not really a big fan of this. This undefined must come from something. Instead of just ignoring it, the best move would be to find the root of this and correct it. I think a fix like this one would just hide the problem 🤔

ghost commented 5 years ago

In the for loop, couldn't you set l = keys.length instead of this.nbElems? That would also remove the undefined issue. I am not sure why that is used in the loop, as keys might not have enough elements to not start producing undefined.

Kylart commented 5 years ago

I think I set l = this.nbElems.length so that elems contains at most nbElems elements. It's part of the "lazy-loading" I set up in history.

Problem occurs when there are not many elements I guess.

Hence, you fix is ok! Thank you for making KawAnime a better place 😄