D17216: Move the about page from Discover to Kirigami
Kai Uwe Broulik
noreply at phabricator.kde.org
Wed Nov 28 14:53:49 GMT 2018
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20181128/8fd4bc43/attachment-0001.html>
More information about the Plasma-devel
mailing list