Refactor song selection to a service

src/app/components/browse/browse.component.html

@@ -5,8 +5,6 @@
       <app-result-table
         #resultTable
         (rowClicked)="chartSidebar.onRowClicked($event)"
-        (songChecked)="statusBar.onSongChecked($event)"
-        (songUnchecked)="statusBar.onSongUnchecked($event)"
       ></app-result-table>
     </div>
     <div id="sidebar-column" class="column four wide">
@@ -14,4 +12,4 @@
     </div>
   </div>
 </div>
-<app-status-bar #statusBar (deselectSongs)="resultTable.onSongsDeselected($event)"></app-status-bar>
+<app-status-bar #statusBar></app-status-bar>

src/app/components/browse/result-table/result-table-row/result-table-row.component.ts

@@ -1,5 +1,6 @@
-import { Component, AfterViewInit, Input, ViewChild, ElementRef, Output, EventEmitter } from '@angular/core'
+import { Component, AfterViewInit, Input, ViewChild, ElementRef } from '@angular/core'
 import { SongResult } from '../../../../../electron/shared/interfaces/search.interface'
+import { SelectionService } from 'src/app/core/services/selection.service'
 
 @Component({
   selector: 'tr[app-result-table-row]',
@@ -8,35 +9,31 @@ import { SongResult } from '../../../../../electron/shared/interfaces/search.int
 })
 export class ResultTableRowComponent implements AfterViewInit {
   @Input() result: SongResult
-  @Output() songChecked = new EventEmitter<SongResult>()
-  @Output() songUnchecked = new EventEmitter<SongResult>()
 
   @ViewChild('checkbox', { static: true }) checkbox: ElementRef
 
-  constructor() { }
+  constructor(private selectionService: SelectionService) { }
 
   get songID() {
     return this.result.id
   }
 
   ngAfterViewInit() {
+    this.selectionService.onSelectionChanged(this.songID, (isChecked) => {
+      if (isChecked) {
+        $(this.checkbox.nativeElement).checkbox('check')
+      } else {
+        $(this.checkbox.nativeElement).checkbox('uncheck')
+      }
+    })
+
     $(this.checkbox.nativeElement).checkbox({
       onChecked: () => {
-        this.songChecked.emit(this.result)
+        this.selectionService.selectSong(this.songID)
       },
       onUnchecked: () => {
-        this.songUnchecked.emit(this.result)
+        this.selectionService.deselectSong(this.songID)
       }
     })
   }
-
-  check(isChecked: boolean) {
-    if (isChecked) {
-      $(this.checkbox.nativeElement).checkbox('check')
-      this.songChecked.emit(this.result)
-    } else {
-      $(this.checkbox.nativeElement).checkbox('uncheck')
-      this.songUnchecked.emit(this.result)
-    }
-  }
 }

src/app/components/browse/result-table/result-table.component.html

@@ -22,8 +22,6 @@
     #tableRow
     *ngFor="let result of results"
     (click)="onRowClicked(result)"
-    (songChecked)="onSongChecked($event)"
-    (songUnchecked)="onSongUnchecked($event)"
     [result]="result"></tr>
   </tbody>
 </table>

src/app/components/browse/result-table/result-table.component.ts

@@ -3,6 +3,7 @@ import { SongResult } from '../../../../electron/shared/interfaces/search.interf
 import { ResultTableRowComponent } from './result-table-row/result-table-row.component'
 import { CheckboxDirective } from 'src/app/core/directives/checkbox.directive'
 import { SearchService } from 'src/app/core/services/search.service'
+import { SelectionService } from 'src/app/core/services/selection.service'
 
 @Component({
   selector: 'app-result-table',
@@ -12,27 +13,21 @@ import { SearchService } from 'src/app/core/services/search.service'
 export class ResultTableComponent implements OnInit {
 
   @Output() rowClicked = new EventEmitter<SongResult>()
-  @Output() songChecked = new EventEmitter<SongResult>()
-  @Output() songUnchecked = new EventEmitter<SongResult>()
 
   @ViewChild(CheckboxDirective, { static: true }) checkboxColumn: CheckboxDirective
   @ViewChildren('tableRow') tableRows: QueryList<ResultTableRowComponent>
 
   results: SongResult[]
 
-  constructor(private searchService: SearchService) { }
+  constructor(private searchService: SearchService, private selectionService: SelectionService) { }
 
   ngOnInit() {
-    this.searchService.onNewSearch(() => {
-      this.checkboxColumn.check(false)
-      this.checkAll(false)
+    this.selectionService.onSelectAllChanged((selected) => {
+      this.checkboxColumn.check(selected)
     })
 
     this.searchService.onSearchChanged(results => {
       this.results = results
-      if (this.checkboxColumn.isChecked) {
-        this.checkAll(true)
-      }
     })
   }
 
@@ -40,19 +35,14 @@ export class ResultTableComponent implements OnInit {
     this.rowClicked.emit(result)
   }
 
-  onSongChecked(result: SongResult) {
-    this.songChecked.emit(result)
-  }
-
-  onSongUnchecked(result: SongResult) {
-    this.songUnchecked.emit(result)
-  }
-
+  /**
+   * Called when the user checks the `checkboxColumn`.
+   */
   checkAll(isChecked: boolean) {
-    this.tableRows.forEach(row => row.check(isChecked))
-  }
-
-  onSongsDeselected(songs: SongResult['id'][]) {
-    this.tableRows.forEach(row => row.check(!songs.includes(row.songID)))
+    if (isChecked) {
+      this.selectionService.selectAll()
+    } else {
+      this.selectionService.deselectAll()
+    }
   }
 }

src/app/components/browse/status-bar/status-bar.component.ts

@@ -1,10 +1,10 @@
-import { Component, ChangeDetectorRef, Output, EventEmitter } from '@angular/core'
-import { SongResult } from 'src/electron/shared/interfaces/search.interface'
+import { Component, ChangeDetectorRef } from '@angular/core'
 import { DownloadService } from 'src/app/core/services/download.service'
 import { ElectronService } from 'src/app/core/services/electron.service'
 import { groupBy } from 'src/electron/shared/UtilFunctions'
 import { VersionResult } from 'src/electron/shared/interfaces/songDetails.interface'
 import { SearchService } from 'src/app/core/services/search.service'
+import { SelectionService } from 'src/app/core/services/selection.service'
 
 @Component({
   selector: 'app-status-bar',
@@ -13,12 +13,9 @@ import { SearchService } from 'src/app/core/services/search.service'
 })
 export class StatusBarComponent {
 
-  @Output() deselectSongs = new EventEmitter<SongResult['id'][]>()
-
   resultCount = 0
   downloading = false
   percent = 0
-  selectedResults: SongResult[] = []
   batchResults: VersionResult[]
   chartGroups: VersionResult[][]
 
@@ -26,6 +23,7 @@ export class StatusBarComponent {
     private electronService: ElectronService,
     private downloadService: DownloadService,
     private searchService: SearchService,
+    private selectionService: SelectionService,
     ref: ChangeDetectorRef
   ) {
     downloadService.onDownloadUpdated(() => {
@@ -39,30 +37,20 @@ export class StatusBarComponent {
     searchService.onSearchChanged(() => {
       this.resultCount = searchService.resultCount
     })
-
-    searchService.onNewSearch(() => {
-      this.selectedResults = []
-    })
   }
 
   get allResultsVisible() {
     return this.searchService.allResultsVisible
   }
 
+  get selectedResults() {
+    return this.selectionService.getSelectedResults()
+  }
+
   showDownloads() {
     $('#downloadsModal').modal('show')
   }
 
-  onSongChecked(result: SongResult) {
-    if (this.selectedResults.findIndex(oldResult => oldResult.id == result.id) == -1) {
-      this.selectedResults.push(result)
-    }
-  }
-
-  onSongUnchecked(result: SongResult) {
-    this.selectedResults = this.selectedResults.filter(oldResult => oldResult.id != result.id)
-  }
-
   async downloadSelected() {
     this.chartGroups = []
     this.batchResults = await this.electronService.invoke('batch-song-details', this.selectedResults.map(result => result.id))
@@ -107,6 +95,8 @@ export class StatusBarComponent {
   }
 
   deselectSongsWithMultipleCharts() {
-    this.deselectSongs.emit(this.chartGroups.map(group => group[0].songID))
+    for (const chartGroup of this.chartGroups) {
+      this.selectionService.deselectSong(chartGroup[0].songID)
+    }
   }
 }

src/app/core/services/search.service.ts

@@ -22,14 +22,23 @@ export class SearchService {
     this.results = this.trimLastChart(await this.electronService.invoke('song-search', this.currentQuery))
     this.awaitingResults = false
 
-    this.newResultsEmitter.emit(this.results)
     this.resultsChangedEmitter.emit(this.results)
+    this.newResultsEmitter.emit(this.results)
   }
 
+  /**
+   * Event emitted when new search results are returned
+   * or when more results are added to an existing search.
+   * (emitted before `onNewSearch`)
+   */
   onSearchChanged(callback: (results: SongResult[]) => void) {
     this.resultsChangedEmitter.subscribe(callback)
   }
 
+  /**
+   * Event emitted when a new search query is typed in.
+   * (emitted after `onSearchChanged`)
+   */
   onNewSearch(callback: (results: SongResult[]) => void) {
     this.newResultsEmitter.subscribe(callback)
   }

src/app/core/services/selection.service.ts

@@ -0,0 +1,96 @@
+import { Injectable, EventEmitter } from '@angular/core'
+import { SongResult } from 'src/electron/shared/interfaces/search.interface'
+import { SearchService } from './search.service'
+
+// Note: this class prevents event cycles by only emitting events if the checkbox changes
+
+@Injectable({
+  providedIn: 'root'
+})
+export class SelectionService {
+
+  private searchResults: SongResult[] = []
+
+  private selectAllChangedEmitter = new EventEmitter<boolean>()
+  private selectionChangedEmitters: { [songID: number]: EventEmitter<boolean> } = {}
+
+  private allSelected = false
+  private selections: { [songID: number]: boolean | undefined } = {}
+
+  constructor(searchService: SearchService) {
+    searchService.onSearchChanged((results) => {
+      this.searchResults = results
+      this.removeOldListeners(results.map(result => result.id))
+
+      if (this.allSelected) {
+        this.selectAll() // Select newly added rows if allSelected
+      }
+    })
+
+    searchService.onNewSearch((results) => {
+      this.searchResults = results
+      this.removeOldListeners(results.map(result => result.id))
+
+      this.deselectAll()
+    })
+  }
+
+  private removeOldListeners(songIDs: number[]) {
+    for (const oldSongID in this.selectionChangedEmitters) {
+      if (!songIDs.find(newSongID => newSongID == Number(oldSongID))) {
+        delete this.selectionChangedEmitters[oldSongID]
+        delete this.selections[oldSongID]
+      }
+    }
+  }
+
+  getSelectedResults() {
+    return this.searchResults.filter(result => this.selections[result.id] == true)
+  }
+
+  onSelectAllChanged(callback: (selected: boolean) => void) {
+    this.selectAllChangedEmitter.subscribe(callback)
+  }
+
+  /**
+   * Emits an event when the selection for `songID` needs to change.
+   * (note: only one emitter can be registered per `songID`)
+   */
+  onSelectionChanged(songID: number, callback: (selection: boolean) => void) {
+    this.selectionChangedEmitters[songID] = new EventEmitter()
+    this.selectionChangedEmitters[songID].subscribe(callback)
+  }
+
+
+  deselectAll() {
+    if (this.allSelected) {
+      this.allSelected = false
+      this.selectAllChangedEmitter.emit(false)
+    }
+
+    setTimeout(() => this.searchResults.forEach(result => this.deselectSong(result.id)), 0)
+  }
+
+  selectAll() {
+    if (!this.allSelected) {
+      this.allSelected = true
+      this.selectAllChangedEmitter.emit(true)
+    }
+
+    setTimeout(() => this.searchResults.forEach(result => this.selectSong(result.id)), 0)
+  }
+
+  deselectSong(songID: number) {
+    if (this.selections[songID]) {
+      this.selections[songID] = false
+      this.selectionChangedEmitters[songID].emit(false)
+    }
+  }
+
+  selectSong(songID: number) {
+    if (!this.selections[songID] && this.selectionChangedEmitters[songID] != undefined) {
+      this.selections[songID] = true
+      this.selectionChangedEmitters[songID].emit(true)
+    }
+  }
+}

src/electron/ipc/download/DownloadQueue.ts

@@ -32,7 +32,7 @@ export class DownloadQueue {
     }
   }
 
-  private sort() { // TODO: make this order be reflected in the GUI (along with currentDownload)
+  private sort() {
     let comparator = Comparators.comparing('allFilesProgress', { reversed: true })
 
     const prioritizeArchives = true