core icon indicating copy to clipboard operation
core copied to clipboard

Track number ordering

Open Hyzual opened this issue 6 years ago • 2 comments

Related to phanan/koel#1089

Hi, this is my first pull request for koel ! The intent of this contribution is to order album song lists first by CD number, then by track number.

Help for testing:

  • Given an album with tracks in 2 CDs
  • When you browse that album, songs are ordered first by CD number, then by track number, so that the following order is followed:
  1. CD 1. Track 1
  2. CD 1. Track 2
  3. CD 2. Track 1
  • After browsing, when you sort again by track number (by clicking on the table header), the ordering still does not consider Disc number. This calls for a deeper refactoring and will need a separate contribution.

Note to reviewer: I have tried to add unit-tests for everything I've touched. Please let me know if more need to be added. I am not used at all to the "no-semicolon" style so apologies if some slipped through, I intended to eslint fix them all. Also I am used to a coding style where variables are in snake_case and functions are in camelCase, let me know if that is a problem and I'll adapt.

I do intend on contributing to koel some more and hope to be able to in the future. I can't promise anything on the frequency of those contributions, as family life and day job do not leave much time... I have experience working on PHP / VueJS codebases and I have a couple of ideas to make koel even better.

Looking forward to your feedback on this :)

Hyzual avatar Apr 06 '20 20:04 Hyzual

Thanks for the review 😊, I'll adjust when I get the chance.

Hyzual avatar Apr 27 '20 21:04 Hyzual

Codecov Report

Merging #31 into master will decrease coverage by 0.39%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #31      +/-   ##
==========================================
- Coverage   54.02%   53.62%   -0.40%     
==========================================
  Files         104      105       +1     
  Lines        1925     1984      +59     
  Branches      140      146       +6     
==========================================
+ Hits         1040     1064      +24     
- Misses        845      887      +42     
+ Partials       40       33       -7     
Impacted Files Coverage Δ
js/components/song/list.vue 53.22% <100.00%> (+7.32%) :arrow_up:
js/utils/filters.js 90.47% <100.00%> (+28.93%) :arrow_up:
js/components/ui/album-artist-thumbnail.vue 40.00% <0.00%> (-60.00%) :arrow_down:
js/services/audio.js 12.50% <0.00%> (-50.00%) :arrow_down:
js/components/ui/lyrics-pane.vue 85.71% <0.00%> (-14.29%) :arrow_down:
js/stores/album.js 66.66% <0.00%> (-11.91%) :arrow_down:
js/utils/$.js 5.55% <0.00%> (-11.12%) :arrow_down:
js/stores/artist.js 62.85% <0.00%> (-10.48%) :arrow_down:
js/components/layout/main-wrapper/extra-panel.vue 55.55% <0.00%> (-5.99%) :arrow_down:
js/services/playback.js 65.06% <0.00%> (-5.86%) :arrow_down:
... and 9 more

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 ed06031...6968b1d. Read the comment docs.

codecov-io avatar May 01 '20 11:05 codecov-io