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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
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
53

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
50

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 (28 lines)

Diff 34031

View Options

app/qml/mpris.qml

Loading...