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