Tweak inital state for playlist
ClosedPublic

Authored by januz on Jul 14 2018, 11:06 PM.

Details

Summary

This patch tweaks the text and design for the playlist when it's empty so it's a bit clearer.

Diff Detail

Repository
R255 Elisa
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
januz requested review of this revision.Jul 14 2018, 11:06 PM
januz created this revision.
januz edited the summary of this revision. (Show Details)Jul 14 2018, 11:07 PM
januz added a reviewer: Elisa.
januz added a project: Elisa.
astippich requested changes to this revision.Jul 15 2018, 8:00 AM
astippich added a subscriber: astippich.
astippich added inline comments.
src/qml/MediaPlayListView.qml
167

Anchors on layout gives a warning. You should be able to remove it or use the Layout.fillWidth/Height properties

195

small nitpick here: it's imho not really a title, so I would write
"Your playlist is empty."

This revision now requires changes to proceed.Jul 15 2018, 8:00 AM
januz updated this revision to Diff 40369.Aug 24 2018, 2:30 PM

Thanks for the ping! This was mostly done but I had forgotten about it

  • Change text
  • Use layout properties instead of anchors
  • Center the empty list text (a little hackish)
astippich requested changes to this revision.Aug 25 2018, 10:04 AM
astippich added inline comments.
src/qml/MediaPlayListView.qml
170

Unfortunately, now this creates a binding loop and gives a warning, because the parent height also depends on the margin.
Before you had

Item { Layout.fillHeight: true }

This revision now requires changes to proceed.Aug 25 2018, 10:04 AM

ping. would be a shame to let this one go to waste so close to merging

If no-one objects, I will accept and land this diff soon, and fix the error afterwards

If no-one objects, I will accept and land this diff soon, and fix the error afterwards

+1

januz added a comment.Oct 8 2018, 6:51 PM

Hi guys, sorry I've been dropping the ball on this one. I had reverted this some weeks ago but couldn't find another way to center it. And then I didn't have time to get back on it.
+1 for landing it now

src/qml/MediaPlayListView.qml
170

True, but I can't think of any other way to center this item vertically otherwise. If I use the previous property it ends up too low (because it's taking into account the height of the entire sidebar).

astippich accepted this revision.Oct 10 2018, 6:31 PM

Hi guys, sorry I've been dropping the ball on this one. I had reverted this some weeks ago but couldn't find another way to center it. And then I didn't have time to get back on it.
+1 for landing it now

I think it is still an improvement even without centering it, so I will remove the binding loop to the solution you provided before.

This revision is now accepted and ready to land.Oct 10 2018, 6:31 PM
This revision was automatically updated to reflect the committed changes.