ubuntu-touch-coreapps-reviewers team mailing list archive

[Andrew Hayzen <ahayzen@xxxxxxxxx>]

Review: Needs Fixing

Awesome, thanks for doing this.

I have on minor inline comment that assists with the styling after moving to the new ListItem component. I also agree with Victor that we'll likely wait for the new SDK scrollbars to see which is best, or a mixture of both. As when testing this you can get it into states where the header covers the letters on the right side, and on smaller screens (eg when in landscape) the fastscroll component does not fit in the vertical space.


1) This label should probably have 
anchors {
    leftMargin: units.gu(1)
    verticalCenter: parent.verticalCenter
}
so that it is vertically centred and not against the left edge

Diff comments:

> 
> === modified file 'app/ui/Songs.qml'
> --- app/ui/Songs.qml	2015-10-18 18:16:05 +0000
> +++ app/ui/Songs.qml	2016-01-02 22:15:54 +0000
> @@ -90,6 +95,18 @@
>              }
>          }
>  
> +        section.property: "title"
> +        section.criteria: ViewSection.FirstCharacter
> +        section.delegate: ListItem {
> +            Label {
> +                text: section

This label should probably have 
anchors {
    leftMargin: units.gu(1)
    verticalCenter: parent.verticalCenter
}
so that it is vertically centred and not against the left edge

> +            }
> +        }
> +
> +        function getSectionText(index) {
> +            return model.get(index).title.substring(0,1)
> +        }
> +
>          delegate: MusicListItem {
>              id: track
>              objectName: "tracksPageListItem" + index


-- 
https://code.launchpad.net/~mcintire-evan/music-app/songs-fastscroll/+merge/281471
Your team Music App Developers is subscribed to branch lp:music-app.