ubuntu-touch-coreapps-reviewers team mailing list archive

[Victor Thompson <victor.thompson@xxxxxxxxx>]

Review: Needs Fixing

Please see inline comments.

Diff comments:

> === modified file 'app/components/Delegates/MusicListItem.qml'
> --- app/components/Delegates/MusicListItem.qml	2015-10-29 23:02:17 +0000
> +++ app/components/Delegates/MusicListItem.qml	2016-01-06 16:34:44 +0000
> @@ -58,27 +61,52 @@
>          visible: false
>      }
>  
> -    MusicRow {
> -        id: musicRow
> -        anchors {
> -            fill: parent
> -            // When not in selectMode we want a margin between the Image and the left edge
> -            // when in selectMode the checkbox has its own margin so we don't want a double margin
> -            leftMargin: selectMode ? 0 : units.gu(2)
> -            rightMargin: selectMode ? 0 : units.gu(2)
> -        }
> -
> -        // Animate margin changes so it isn't noticible

Please fix the spelling of "noticeable"

> -        Behavior on anchors.leftMargin {
> -            NumberAnimation {
> -
> -            }
> -        }
> -
> -        Behavior on anchors.rightMargin {
> -            NumberAnimation {
> -
> -            }
> +    ListItemLayout {
> +        id: listItemLayout
> +
> +        padding.bottom: image.visible ? units.gu(.5) : units.gu(1.5)
> +        padding.top: image.visible ? units.gu(.5) : units.gu(1.5)
> +
> +        subtitle.color: styleMusic.common.subtitle
> +        subtitle.fontSize: "x-small"
> +        subtitle.wrapMode: Text.WrapAnywhere
> +
> +        title.color: styleMusic.common.music
> +        title.fontSize: "small"
> +        title.wrapMode: Text.WrapAnywhere
> +
> +        Image {
> +            id: image
> +            anchors {
> +                verticalCenter: parent.verticalCenter
> +            }
> +            asynchronous: true
> +            fillMode: Image.PreserveAspectCrop
> +            height: width
> +            SlotsLayout.position: SlotsLayout.Leading
> +            source: {
> +                if (imageSource !== undefined && imageSource !== "") {
> +                    if (imageSource.art !== undefined) {
> +                        imageSource.art
> +                    } else {
> +                        "image://albumart/artist=" + imageSource.author + "&album=" + imageSource.album
> +                    }
> +                } else {
> +                    ""
> +                }
> +            }
> +            sourceSize.height: height
> +            sourceSize.width: width
> +            width: units.gu(6)
> +            visible: imageSource !== undefined
> +
> +            onStatusChanged: {
> +                if (status === Image.Error) {
> +                    source = Qt.resolvedUrl("../graphics/music-app-cover@xxxxxx")
> +                }
> +            }
> +
> +            property var imageSource
>          }
>      }
>  }
> 
> === modified file 'app/components/Queue.qml'
> --- app/components/Queue.qml	2015-10-18 18:16:05 +0000
> +++ app/components/Queue.qml	2016-01-06 16:34:44 +0000
> @@ -69,6 +52,11 @@
>          multiselectable: true
>          objectName: "nowPlayingListItem" + index
>          reorderable: true
> +        title.color: player.currentIndex === index ? UbuntuColors.blue : styleMusic.common.music

Order alphabetically?

> +        title.text: model.title
> +        title.objectName: "titleLabel"
> +        subtitle.text: model.author
> +        subtitle.objectName: "artistLabel"
>          trailingActions: ListItemActions {
>              actions: [
>                  AddToPlaylist {
> 
> === modified file 'app/ui/Songs.qml'
> --- app/ui/Songs.qml	2015-10-18 18:16:05 +0000
> +++ app/ui/Songs.qml	2016-01-06 16:34:44 +0000
> @@ -93,25 +93,11 @@
>          delegate: MusicListItem {
>              id: track
>              objectName: "tracksPageListItem" + index
> -            column: Column {
> -                Label {
> -                    id: trackTitle
> -                    color: styleMusic.common.music
> -                    fontSize: "small"
> -                    objectName: "tracktitle"
> -                    text: model.title
> -                }
> -
> -                Label {
> -                    id: trackArtist
> -                    color: styleMusic.common.subtitle
> -                    fontSize: "x-small"
> -                    text: model.author
> -                }
> -            }
> -            height: units.gu(7)
>              imageSource: {"art": model.art}
>              multiselectable: true
> +            title.text: model.title

Order alphabetically?

> +            title.objectName: "tracktitle"
> +            subtitle.text: model.author
>              trailingActions: AddToQueueAndPlaylist {
>              }
>  
> 
> === modified file 'app/ui/SongsView.qml'
> --- app/ui/SongsView.qml	2015-10-28 01:05:33 +0000
> +++ app/ui/SongsView.qml	2016-01-06 16:34:44 +0000
> @@ -297,27 +297,13 @@
>          delegate: MusicListItem {
>              id: track
>              objectName: "songsPageListItem" + index
> -            column: Column {
> -                Label {
> -                    id: trackTitle
> -                    color: styleMusic.common.music
> -                    fontSize: "small"
> -                    objectName: "songspage-tracktitle"
> -                    text: model.title
> -                }
> -
> -                Label {
> -                    id: trackArtist
> -                    color: styleMusic.common.subtitle
> -                    fontSize: "x-small"
> -                    text: model.author
> -                }
> -            }
> -            height: units.gu(6)
>              leadingActions: songStackPage.line1 === i18n.tr("Playlist")
>                              ? playlistRemoveAction.item : null
>              multiselectable: true
>              reorderable: songStackPage.line1 === i18n.tr("Playlist")
> +            title.text: model.title

Order alphabetically?

> +            title.objectName: "songspage-tracktitle"
> +            subtitle.text: model.author
>              trailingActions: AddToQueueAndPlaylist {
>              }
>  


-- 
https://code.launchpad.net/~ahayzen/music-app/fix-1526274-use-layouts/+merge/281757
Your team Music App Developers is subscribed to branch lp:music-app.