[Merge] lp:~abreu-alexandre/webbrowser-app/text-color-for-theme-color-webapps into lp:webbrowser-app

Alberto Mardegan alberto.mardegan at canonical.com
Tue Apr 5 14:40:47 UTC 2016


Review: Needs Information

A few minor issues inline.

Diff comments:

> 
> === modified file 'src/app/webcontainer/WebApp.qml'
> --- src/app/webcontainer/WebApp.qml	2016-02-09 17:04:23 +0000
> +++ src/app/webcontainer/WebApp.qml	2016-04-04 17:16:24 +0000
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright 2013-2015 Canonical Ltd.
> + * Copyright 2013-2015-2016 Canonical Ltd.

2013-2016

>   *
>   * This file is part of webbrowser-app.
>   *
> 
> === modified file 'src/app/webcontainer/webapp-container-helper.cpp'
> --- src/app/webcontainer/webapp-container-helper.cpp	2015-05-27 16:19:45 +0000
> +++ src/app/webcontainer/webapp-container-helper.cpp	2016-04-04 17:16:24 +0000
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright 2014 Canonical Ltd.
> + * Copyright 2014, 2016 Canonical Ltd.

2014-2016

>   *
>   * This file is part of webbrowser-app.
>   *
> @@ -18,9 +18,11 @@
>  
>  #include "webapp-container-helper.h"
>  
> -#include <QMetaObject>
> -#include <QMetaProperty>
> -
> +#include <QColor>
> +#include <QtCore/QMetaObject>
> +#include <QtCore/QMetaProperty>
> +#include <QtCore/QRegExp>

I'd rather be consistent and either use #include <QtModule/Class> or #include <Class> everywhere (my preference goes for the latter).

> +#include <QDebug>
>  
>  WebappContainerHelper::WebappContainerHelper(QObject* parent)
>      : QObject(parent)
> @@ -38,15 +40,55 @@
>  
>  void WebappContainerHelper::browseToUrl(QObject* webview, const QUrl& url)
>  {
> -    if ( ! webview)
> +    if ( ! webview) {
>          return;
> +    }
>      const QMetaObject* metaobject = webview->metaObject();
>      int urlPropIdx = metaobject->indexOfProperty("url");
> -    if (urlPropIdx == -1)
> +    if (urlPropIdx == -1) {
>          return;
> +    }
>      metaobject->property(urlPropIdx).write(webview, QVariant(url));
>  }
>  
> -
> -
> -
> +QString WebappContainerHelper::rgbColorFromCSSColor(const QString& cssColor)
> +{
> +    QString color = cssColor.trimmed().toLower();
> +    if (color.isEmpty()) {
> +        return QString();
> +    }
> +    QString returnColorFormat = "%1,%2,%3";
> +    if (color.startsWith("rgb")) {
> +        QRegExp rgbColorRe("rgb\\s*\\(\\s*(\\d+)\\s*,\\s*(\\d+)\\s*,\\s*(\\d+)\\s*\\)");
> +        if (rgbColorRe.exactMatch(color)) {
> +            return returnColorFormat
> +                    .arg(rgbColorRe.cap(1).toInt())
> +                    .arg(rgbColorRe.cap(2).toInt())
> +                    .arg(rgbColorRe.cap(3).toInt());
> +        }
> +        return QString();
> +    } else if (color.startsWith("#")) {

This "else" block is not needed: the last "else" block will cover this case as well, given that QColor can parse "#rRgGbB" strings.

> +        QString hexColor = color.mid(1);
> +        if (hexColor.size() != 6) {
> +            hexColor = QString(6 - hexColor.size(), '0') + hexColor;
> +        }
> +        QRegExp rgbColorRe("((\\d|[a-f])(\\d|[a-f]))((\\d|[a-f])(\\d|[a-f]))((\\d|[a-f])(\\d|[a-f]))");
> +        if (rgbColorRe.exactMatch(hexColor)) {
> +            bool status = false;
> +            return returnColorFormat
> +                    .arg(rgbColorRe.cap(1).toUInt(&status, 16))
> +                    .arg(rgbColorRe.cap(4).toUInt(&status, 16))
> +                    .arg(rgbColorRe.cap(7).toUInt(&status, 16));
> +        }
> +        return QString();
> +    } else {
> +        QColor returnColor(color);
> +        return returnColor.isValid()
> +                ? returnColorFormat
> +                        .arg(returnColor.red())
> +                        .arg(returnColor.green())
> +                        .arg(returnColor.blue())
> +                        .toLower()
> +                : QString();
> +    }
> +}
> 
> === modified file 'src/app/webcontainer/webapp-container-helper.h'
> --- src/app/webcontainer/webapp-container-helper.h	2015-05-27 16:19:45 +0000
> +++ src/app/webcontainer/webapp-container-helper.h	2016-04-04 17:16:24 +0000
> @@ -31,6 +31,15 @@
>      WebappContainerHelper(QObject* parent = 0);
>      ~WebappContainerHelper();
>  
> +    /**
> +     * Expects a CSS color string http://www.w3schools.com/css/css_colors.asp
> +     * and returns a #rrggbb formatted string.

s/#rrggbb/rr,gg,bb/

> +     *
> +     * If the provided string is not a valid CSS color string, an empty string
> +     * is returned.
> +     */
> +    Q_INVOKABLE QString rgbColorFromCSSColor(const QString& cssColor);
> +
>  private Q_SLOTS:
>  
>      void browseToUrl(QObject* webview, const QUrl& url);


-- 
https://code.launchpad.net/~abreu-alexandre/webbrowser-app/text-color-for-theme-color-webapps/+merge/287058
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.



More information about the Ubuntu-reviews mailing list