ubuntu-touch-coreapps-reviewers team mailing list archive

[Andrew Hayzen <ahayzen@xxxxxxxxx>]

Review: Needs Information

Inline note to self :-)

Diff comments:

> === modified file 'app/components/Flickables/CardView.qml'
> --- app/components/Flickables/CardView.qml	2015-05-03 16:22:31 +0000
> +++ app/components/Flickables/CardView.qml	2015-06-28 03:20:00 +0000
> @@ -35,17 +35,11 @@
>      property alias delegate: flow.delegate
>      property var getter
>      property alias header: headerLoader.sourceComponent
> -    property var model: flow.model
> +    property alias model: flow.model

Confirm this works :-)

>      property real itemWidth: units.gu(15)
>  
>      onGetterChanged: flow.getter = getter  // cannot use alias to set a function (must be var)
>  
> -    onVisibleChanged: {
> -        if (visible) {  // only load model once CardView is visible
> -            flow.model = model
> -        }
> -    }
> -
>      Loader {
>          id: headerLoader
>          visible: sourceComponent !== undefined
> 
> === modified file 'app/components/MusicPage.qml'
> --- app/components/MusicPage.qml	2015-05-03 16:22:31 +0000
> +++ app/components/MusicPage.qml	2015-06-28 03:20:00 +0000
> @@ -29,14 +29,35 @@
>          bottomMargin: musicToolbar.visible ? musicToolbar.height : 0
>          fill: parent
>      }
> +    flickable: pageFlickable
>  
> +    default property alias content: pageChildrenContainer.data
>      property Dialog currentDialog
> +    property Flickable pageFlickable
>      property bool searchable: false
>      property int searchResultsCount
>  
> +    onVisibleChanged: {
> +        if (visible) {
> +            onPageVisible()
> +        }
> +    }
> +
> +    function onPageVisible() {
> +        mainPageStack.setPage(thisPage);
> +
> +        // Rebind sidebar so that it is visible on this page
> +        nowPlayingSidebarLoader.parent = parent
> +        nowPlayingSidebarLoader.anchors.bottom = parent.bottom
> +        nowPlayingSidebarLoader.anchors.right = parent.right
> +        nowPlayingSidebarLoader.anchors.top = parent.top
> +        // doesn't need to be bound due to us forcing the header to be shown in wideAspect mode
> +        nowPlayingSidebarLoader.anchors.topMargin = thisPage.header.height
> +    }
> +
>      Label {
>          anchors {
> -            centerIn: parent
> +            centerIn: pageChildrenContainer
>          }
>          text: i18n.tr("No items found")
>          visible: parent.state === "search" && searchResultsCount === 0
> @@ -56,9 +77,29 @@
>          }
>      }
>  
> -    onVisibleChanged: {
> -        if (visible) {
> -            mainPageStack.setPage(thisPage);
> +    Item {
> +        id: pageChildrenContainer
> +        anchors {
> +            bottom: parent.bottom
> +            left: parent.left
> +            top: parent.top
> +        }
> +        width: parent.width - (wideAspect ? nowPlayingSidebarLoader.width : 0)
> +    }
> +
> +    // Hack to keep the header visible as we cannot use flickable: null due to issues with the topMargin
> +    Connections {
> +        target: thisPage.header
> +        onYChanged: {
> +            if (wideAspect) {
> +                thisPage.header.y = 0
> +            }
> +        }
> +    }
> +
> +    Component.onCompleted: {
> +        if (visible) {  // onVisible is not sometimes called so use onComplete as well
> +            onPageVisible()
>          }
>      }
>  }
> 
> === added file 'app/components/NowPlayingSidebar.qml'
> --- app/components/NowPlayingSidebar.qml	1970-01-01 00:00:00 +0000
> +++ app/components/NowPlayingSidebar.qml	2015-06-28 03:20:00 +0000
> @@ -0,0 +1,60 @@
> +/*
> + * Copyright (C) 2013, 2014, 2015
> + *      Andrew Hayzen <ahayzen@xxxxxxxxx>
> + *      Daniel Holm <d.holmen@xxxxxxxxx>
> + *      Victor Thompson <victor.thompson@xxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 3.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +import QtQuick 2.3
> +import Ubuntu.Components 1.1
> +
> +
> +Queue {
> +    clip: true
> +    isSidebar: true
> +    header: Column {
> +        id: sidebarColumn
> +        anchors {
> +            left: parent.left
> +            right: parent.right
> +        }
> +
> +        NowPlayingFullView {
> +            anchors {
> +                fill: undefined
> +            }
> +            clip: true
> +            height: units.gu(30)
> +            width: parent.width
> +        }
> +
> +        NowPlayingToolbar {
> +            anchors {
> +                fill: undefined
> +            }
> +            bottomProgressHint: false
> +            height: units.gu(10)
> +            width: parent.width
> +        }
> +    }
> +
> +    // TODO: Currently the Queue's header is shown when the user
> +    //       pulls down on the ListView, this covers it up.
> +    Rectangle {
> +        anchors.fill: parent
> +        color: "#2c2c34"
> +        z: -1
> +    }
> +}
> 
> === modified file 'app/components/NowPlayingToolbar.qml'
> --- app/components/NowPlayingToolbar.qml	2015-05-03 16:22:31 +0000
> +++ app/components/NowPlayingToolbar.qml	2015-06-28 03:20:00 +0000
> @@ -30,6 +30,8 @@
>      }
>      color: styleMusic.common.black
>  
> +    property alias bottomProgressHint: playerControlsProgressBar.visible
> +
>      /* Repeat button */
>      MouseArea {
>          id: nowPlayingRepeatButton
> 
> === modified file 'app/components/Queue.qml'
> --- app/components/Queue.qml	2015-06-27 21:06:26 +0000
> +++ app/components/Queue.qml	2015-06-28 03:20:00 +0000
> @@ -37,6 +37,7 @@
>      model: trackQueue.model
>      objectName: "nowPlayingqueueList"
>  
> +    property bool isSidebar: false
>      property int normalHeight: units.gu(6)
>      property int transitionDuration: 250  // transition length of animations
>  
> @@ -44,7 +45,8 @@
>  
>      delegate: MusicListItem {
>          id: queueListItem
> -        color: player.currentIndex === index ? "#2c2c34" : styleMusic.mainView.backgroundColor
> +        color: isSidebar ? (player.currentIndex === index ? "#3d3d45" : "#2c2c34")
> +                         : (player.currentIndex === index ? "#2c2c34" : styleMusic.mainView.backgroundColor)
>          column: Column {
>              Label {
>                  id: trackTitle
> @@ -68,7 +70,7 @@
>          leftSideAction: Remove {
>              onTriggered: trackQueue.removeQueueList([index])
>          }
> -        multiselectable: true
> +        multiselectable: !isSidebar
>          reorderable: true
>          rightSideActions: [
>              AddToPlaylist{
> 
> === modified file 'app/music-app.qml'
> --- app/music-app.qml	2015-06-28 03:06:49 +0000
> +++ app/music-app.qml	2015-06-28 03:20:00 +0000
> @@ -17,6 +17,7 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +import QtSensors 5.0
>  import QtQuick 2.4
>  import Ubuntu.Components 1.2
>  import Ubuntu.Components.Popups 1.0
> @@ -36,7 +37,7 @@
>      objectName: "musicMainView"
>      applicationName: "com.ubuntu.music"
>      id: mainView
> -
> +    automaticOrientation: true
>      backgroundColor: styleMusic.mainView.backgroundColor
>      headerColor: styleMusic.mainView.headerColor
>  
> @@ -282,9 +283,23 @@
>  
>      signal listItemSwiping(int i)
>  
> -    property bool wideAspect: width >= units.gu(70) && loadedUI
> +    property bool landscape: orientationSensor.reading !== null && (orientationSensor.reading.orientation === OrientationReading.LeftUp || orientationSensor.reading.orientation === OrientationReading.RightUp)
> +    property bool wideAspect: loadedUI && (landscape ? height >= units.gu(70) : width >= units.gu(70))
>      property bool loadedUI: false  // property to detect if the UI has finished
>  
> +    onWideAspectChanged: {
> +        // FIXME: if user has gone nowPlaying->Add to playlist nowPlaying won't be popped
> +        if (wideAspect && mainPageStack.currentMusicPage !== null &&
> +                mainPageStack.currentMusicPage.objectName === "nowPlayingPage") {
> +            mainPageStack.pop()
> +        }
> +    }
> +
> +    OrientationSensor {
> +        id: orientationSensor
> +        active: true
> +    }
> +
>      // FUNCTIONS
>  
>      // Custom debug funtion that's easier to shut off
> @@ -706,10 +721,22 @@
>          visible: mainPageStack.currentPage.title !== i18n.tr("Now playing") &&
>                   mainPageStack.currentPage.title !== i18n.tr("Queue") &&
>                   mainPageStack.currentPage.title !== i18n.tr("Select playlist") &&
> -                 !firstRun
> +                 !firstRun && !wideAspect
>          z: 200  // put on top of everything else
>      }
>  
> +    Loader {
> +        id: nowPlayingSidebarLoader
> +        active: wideAspect
> +        anchors {  // start offscreen
> +            right: parent.right
> +        }
> +        asynchronous: true
> +        source: "components/NowPlayingSidebar.qml"
> +        visible: active
> +        width: units.gu(40)
> +    }
> +
>      PageStack {
>          id: mainPageStack
>  
> @@ -972,6 +999,10 @@
>  
>              function pushNowPlaying()
>              {
> +                if (wideAspect) {  // no now playing page in wideAspect mode
> +                    return;
> +                }
> +
>                  // only push if on a different page
>                  if (mainPageStack.currentPage.title !== i18n.tr("Now playing")
>                          && mainPageStack.currentPage.title !== i18n.tr("Queue")) {
> 
> === modified file 'app/ui/AddToPlaylist.qml'
> --- app/ui/AddToPlaylist.qml	2015-06-27 21:06:26 +0000
> +++ app/ui/AddToPlaylist.qml	2015-06-28 03:20:00 +0000
> @@ -41,6 +41,7 @@
>  // Page that will be used when adding tracks to playlists
>  MusicPage {
>      id: addToPlaylistPage
> +    pageFlickable: addtoPlaylistView
>      objectName: "addToPlaylistPage"
>      // TRANSLATORS: this appears in the header with limited space (around 20 characters)
>      title: i18n.tr("Select playlist")
> 
> === modified file 'app/ui/Albums.qml'
> --- app/ui/Albums.qml	2015-05-03 16:22:31 +0000
> +++ app/ui/Albums.qml	2015-06-28 03:20:00 +0000
> @@ -29,6 +29,7 @@
>  MusicPage {
>      id: albumsPage
>      objectName: "albumsPage"
> +    pageFlickable: albumCardView
>      title: i18n.tr("Albums")
>      searchable: true
>      searchResultsCount: albumsModelFilter.count
> 
> === modified file 'app/ui/ArtistView.qml'
> --- app/ui/ArtistView.qml	2015-05-03 16:22:31 +0000
> +++ app/ui/ArtistView.qml	2015-06-28 03:20:00 +0000
> @@ -33,6 +33,7 @@
>  MusicPage {
>      id: artistViewPage
>      objectName: "artistViewPage"
> +    pageFlickable: artistAlbumView
>      visible: false
>  
>      property string artist: ""
> 
> === modified file 'app/ui/Artists.qml'
> --- app/ui/Artists.qml	2015-05-03 16:22:31 +0000
> +++ app/ui/Artists.qml	2015-06-28 03:20:00 +0000
> @@ -35,6 +35,7 @@
>  MusicPage {
>      id: artistsPage
>      objectName: "artistsPage"
> +    pageFlickable: artistCardView
>      title: i18n.tr("Artists")
>      searchable: true
>      searchResultsCount: artistsModelFilter.count
> 
> === modified file 'app/ui/Genres.qml'
> --- app/ui/Genres.qml	2015-05-03 16:22:31 +0000
> +++ app/ui/Genres.qml	2015-06-28 03:20:00 +0000
> @@ -29,6 +29,7 @@
>  MusicPage {
>      id: genresPage
>      objectName: "genresPage"
> +    pageFlickable: genreCardView
>      title: i18n.tr("Genres")
>      searchable: true
>      searchResultsCount: genresModelFilter.count
> 
> === modified file 'app/ui/Playlists.qml'
> --- app/ui/Playlists.qml	2015-06-27 21:06:26 +0000
> +++ app/ui/Playlists.qml	2015-06-28 03:20:00 +0000
> @@ -33,6 +33,7 @@
>  MusicPage {
>      id: playlistsPage
>      objectName: "playlistsPage"
> +    pageFlickable: playlistslist
>      // TRANSLATORS: this is the name of the playlists page shown in the tab header.
>      // Remember to keep the translation short to fit the screen width
>      title: i18n.tr("Playlists")
> 
> === modified file 'app/ui/Recent.qml'
> --- app/ui/Recent.qml	2015-06-27 21:06:26 +0000
> +++ app/ui/Recent.qml	2015-06-28 03:20:00 +0000
> @@ -34,6 +34,7 @@
>  MusicPage {
>      id: recentPage
>      objectName: "recentPage"
> +    pageFlickable: recentCardView
>      title: i18n.tr("Recent")
>  
>      property bool changed: false
> 
> === modified file 'app/ui/Songs.qml'
> --- app/ui/Songs.qml	2015-06-27 21:06:26 +0000
> +++ app/ui/Songs.qml	2015-06-28 03:20:00 +0000
> @@ -34,6 +34,7 @@
>  MusicPage {
>      id: songsPage
>      objectName: "songsPage"
> +    pageFlickable: tracklist
>      title: i18n.tr("Songs")
>      searchable: true
>      searchResultsCount: songsModelFilter.count
> 
> === modified file 'app/ui/SongsView.qml'
> --- app/ui/SongsView.qml	2015-06-27 21:06:26 +0000
> +++ app/ui/SongsView.qml	2015-06-28 03:20:00 +0000
> @@ -36,6 +36,7 @@
>  MusicPage {
>      id: songStackPage
>      objectName: "songsPage"
> +    pageFlickable: albumtrackslist
>      visible: false
>  
>      property string line1: ""
> @@ -228,11 +229,11 @@
>                      }
>                  }
>              }
> -            property int baseHeight: header.width > units.gu(60) ? units.gu(33.5) : ((header.width - units.gu(5)) / 2) + units.gu(12)
> +            property int baseHeight: blurredHeader.width > units.gu(60) ? units.gu(33.5) : ((blurredHeader.width - units.gu(5)) / 2) + units.gu(12)
>              coverSources: songStackPage.covers
>              height: songStackPage.line1 !== i18n.tr("Playlist") &&
>                      songStackPage.line1 !== i18n.tr("Genre") &&
> -                    header.width <= units.gu(60) ?
> +                    blurredHeader.width <= units.gu(60) ?
>                          baseHeight + units.gu(3) : baseHeight
>              bottomColumn: Column {
>                  Label {
> 


-- 
https://code.launchpad.net/~ahayzen/music-app/refactor-pull-now-playing-sidebar/+merge/253839
Your team Music App Developers is subscribed to branch lp:music-app.