broulik added inline comments. INLINE COMMENTS
> AboutPage.qml:102 > + Layout.maximumWidth: Units.iconSizes.medium > + source: "https://www.gravatar.com/avatar/"; + > Qt.md5(modelData.emailAddress) + "?d=404&s=" + Units.iconSizes.medium > + fallback: "user" Not a huge fan of this, privacy-wise, also QtQuick doesn't do any caching on its own > LinkButton.qml:35 > + */ > +QQC2.Label { > + id: control This thing is missing `Accessible` and keyboard handling, can you make this style `AbstractButton` or something instead? > LinkButton.qml:44 > + > + font: control.font > + color: enabled ? Theme.linkColor : Theme.textColor That binds to itself? > LinkButton.qml:57 > + > + onContainsMouseChanged: { > + control.font.underline = containsMouse && control.enabled Can this be done using a binding? > UrlButton.qml:38 > + text: url > + visible: text.length>0 > + Coding style, spaces > UrlButton.qml:41 > + acceptedButtons: Qt.LeftButton | Qt.RightButton > + onClicked: { > + if (mouse.button === Qt.RightButton) Context menus open on //press//, not click > UrlButton.qml:42 > + onClicked: { > + if (mouse.button === Qt.RightButton) > + menu.popup() Coding style, braces > UrlButton.qml:51 > + QQC2.MenuItem { > + text: i18n("Copy link address") > + onClicked: app.copyTextToClipboard(button.url) Add `edit-copy` icon > settings.cpp:136 > +{ > + return tr("<ul><li>KDE Frameworks %1</li><li>Qt %2 (built against > %3)</li><li>The <em>%4</em> windowing system</li></ul>").arg( > + QStringLiteral(KIRIGAMI2_VERSION_STRING), Can we have this return a `QStringList` instead instead of assuming this will be rendered as html list? Or is that "not public api"? > settings.h:66 > > + Q_PROPERTY(QString information READ information CONSTANT) > + Add some docs, also `information` isn't very descriptive name, also `@since` REPOSITORY R169 Kirigami BRANCH master REVISION DETAIL https://phabricator.kde.org/D17216 To: apol, #kirigami, mart, broulik Cc: plasma-devel, dkardarakos, apol, davidedmundson, mart, hein
