Differential D12552

Use title, album and artist in UI
ClosedPublic
Actions

Authored by nicolasfella on Apr 26 2018, 9:53 PM.

Details

Reviewers
apol
mtijink
Group Reviewers
KDE Connect
Commits
R224:1934585c412f: Use title, album and artist in UI
Summary

Depends on D12546

Test Plan

Look at it

Diff Detail

Repository
R224 KDE Connect
Branch
requeststatus
Lint
No Linters Available
Unit
No Unit Test Coverage
nicolasfella created this revision.Apr 26 2018, 9:53 PM
Restricted Application added a project: KDE Connect. · View Herald TranscriptApr 26 2018, 9:54 PM
nicolasfella requested review of this revision.Apr 26 2018, 9:54 PM
apol added a subscriber: apol.Apr 27 2018, 10:06 AM
apol added inline comments.
app/qml/mpris.qml
52 ↗(On Diff #33179)

text: i18n("%1 - %2", ...)

mtijink added a subscriber: mtijink.Apr 27 2018, 12:08 PM

What if the remote doesn't send title, artist, etc. but only nowPlaying?

nicolasfella updated this revision to Diff 33209.Apr 27 2018, 3:49 PM
  • Show nowPlaying when no title available
  • i18n
apol requested changes to this revision.Apr 27 2018, 10:19 PM
apol added inline comments.
app/qml/mpris.qml
49

This is weird. When there's a title but no artist you'll se both. Use a visible: !otherThing.visible.

This revision now requires changes to proceed.Apr 27 2018, 10:19 PM
nicolasfella updated this revision to Diff 33226.Apr 27 2018, 11:14 PM
  • Merge branch 'master' into arcpatch-D12552
  • Handle more cases
nicolasfella updated this revision to Diff 33227.Apr 27 2018, 11:15 PM
  • Unrelated change
nicolasfella added a comment.Apr 27 2018, 11:19 PM

Turns out there are more cases to handle:

  • No title available -> show nowplaying
  • Title availabe, no artist/album available -> show title
  • title and either album or artist -> show title and either album or artist
  • title, album and artist available -> show title and artist - album

Any hints on simplifying that logic is welcome

mtijink added a comment.Apr 30 2018, 6:36 PM
In D12552#255007, @nicolasfella wrote:

Turns out there are more cases to handle:

  • No title available -> show nowplaying
  • Title availabe, no artist/album available -> show title
  • title and either album or artist -> show title and either album or artist
  • title, album and artist available -> show title and artist - album

    Any hints on simplifying that logic is welcome

I think this would work fine:
First label: title or, if not available, nowPlaying
Second label: artist - album or artist (if album isn't available). And use visible: text.length > 0.

nicolasfella updated this revision to Diff 33849.May 8 2018, 9:53 PM
  • Use length > 0
nicolasfella added a comment.May 8 2018, 9:54 PM

There are cases, e.g. PBI, when there is a album but no artist

nicolasfella updated this revision to Diff 33852.May 8 2018, 10:42 PM
  • Enable word wrap
mtijink accepted this revision.May 9 2018, 1:27 PM
Restricted Application added a subscriber: kdeconnect. · View Herald TranscriptMay 9 2018, 1:27 PM
This revision was not accepted when it landed; it landed in state Needs Review.May 12 2018, 4:05 PM
Closed by commit R224:1934585c412f: Use title, album and artist in UI (authored by nicolasfella). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mapp/qml/mpris.qml (12 lines)

Diff 33209

View Options

app/qml/mpris.qml

Loading...