ngraham added a comment.
Getting there! In D19077#417120 <https://phabricator.kde.org/D19077#417120>, @filipf wrote: > In D19077#417105 <https://phabricator.kde.org/D19077#417105>, @ngraham wrote: > > > This introduces a new warning: > > WARNING: viewBackgroundColor is deprecated, use backgroundColor with colorSet: Theme.View instead > > > I would use the new value, but it doesn't provide the same color as the old one. So that means it breaks the magical hack and I can't hit the sweet spot anymore by fiddling with opacity. I'm not thrilled by the need to introduce new warnings in order to implement a hack. It's all a pretty stinky code smell, to be honest. :) Maybe @davidedmundson has some ideas. Also, here are some more inline comments: INLINE COMMENTS > main.qml:81 > + Label { > + text: description + i18n(", by ") + authorName + " (" + license > + ")" > + Layout.fillWidth: true This is a word puzzle and will lead to bad translations because translators won't be able to figure out what `", by "` refers to. The string should be like this instead: `i18n("%1, by %2 (%3)", description, authorName, license)` Of course if there's a chance any of those strings might be unset or empty, you'll need to handle that in the code or else the empty string will mess up the `i18n()` call. > main.qml:86 > > - Text { > - color: palette.text > - text: description > - font.bold: true > - font.pointSize: 13 > - } > - Text { > - color: palette.text > - text: version > - font.bold: true > - font.pointSize: 10 > - Layout.alignment: Qt.AlignVCenter | Qt.AlignRight > - } > - Text { > - color: palette.text > - text: authorName > - font.pointSize: 10 > - } > - Text { > - color: palette.text > - text: license > - font.pointSize: 7 > - Layout.alignment: Qt.AlignVCenter | Qt.AlignRight > - } > - Text { > - color: palette.text > - text: website > - font.pointSize: 7 > - } > - Text { > - color: palette.text > - text: email > - font.pointSize: 7 > - Layout.alignment: Qt.AlignVCenter | Qt.AlignRight > - } > + Label { > + id: url This could use a `visible: website !== ""` so it doesn't appear and look weird when that piece of metadata isn't available. > main.qml:95 > + > + Label { > + id: mail ^^ Ditto REPOSITORY R123 SDDM Configuration Panel (KCM) REVISION DETAIL https://phabricator.kde.org/D19077 To: filipf, #plasma, #vdg, ngraham Cc: GB_2, mmustac, davidedmundson, abetts, rooty, plasma-devel, jraleigh, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
