ubuntu-touch-coreapps-reviewers team mailing list archive

[Andrew Hayzen <ahayzen@xxxxxxxxx>]

Review: Needs Information

1 inline comment so far....

1) This should be addSourcesFromModel (plural) ?

Diff comments:

> 
> === modified file 'app/components/NewPlayer.qml'
> --- app/components/NewPlayer.qml	2015-11-02 17:10:45 +0000
> +++ app/components/NewPlayer.qml	2015-11-12 20:47:08 +0000
> @@ -106,39 +107,41 @@
>  
>                  // Check if there is pending shuffle
>                  // pendingShuffle holds the expected size of the model
> -                if (pendingShuffle > -1 && pendingShuffle <= mediaCount) {
> +                if (pendingShuffle > -1 && pendingShuffle <= itemCount) {
>                      mediaPlayerPlaylist.shuffle();
>  
>                      pendingShuffle = -1;
>                      mediaPlayer.next();  // play a random track
>                  }
>  
> +                console.debug("*** Saving play queue in onItemInserted");
>                  saveQueue()
>  
> -                // FIXME: shouldn't be needed? seems to be a bug where when appending currentSourceChanged is not emitted
> +                // FIXME: shouldn't be needed? seems to be a bug where when appending currentItemChanged is not emitted
>                  if (start === currentIndex) {
> -                    currentMeta = metaForSource(currentSource)
> +                    currentMeta = metaForSource(currentItemSource)
>                  }
>              }
> -            onMediaRemoved: {
> +            onItemRemoved: {
> +                console.debug("*** Saving play queue in onItemRemoved");
>                  saveQueue()
>  
> -                // FIXME: shouldn't be needed? seems to be a bug where when appending currentSourceChanged is not emitted
> +                // FIXME: shouldn't be needed? seems to be a bug where when appending currentItemChanged is not emitted
>                  if (start === currentIndex) {
> -                    currentMeta = metaForSource(currentSource)
> +                    currentMeta = metaForSource(currentItemSource)
>                  }
>              }
>  
>              // TODO: AP needs queue length
>  
> -            function addSourcesFromModel(model) {
> +            function addSourceFromModel(model) {

This should be addSourcesFromModel (plural) ?

>                  var sources = []
>  
>                  for (var i=0; i < model.rowCount; i++) {
>                      sources.push(Qt.resolvedUrl(model.get(i, model.RoleModelData).filename));
>                  }
>  
> -                addSources(sources);
> +                addItems(sources);
>              }
>  
>              function processPendingCurrentState() {


-- 
https://code.launchpad.net/~phablet-team/music-app/final-declarative-playlist-api/+merge/277357
Your team Music App Developers is subscribed to branch lp:~music-app-dev/music-app/media-hub-bg-playlists-rework.