[Merge] lp:~fboucault/webbrowser-app/downloads_emblem into lp:webbrowser-app/staging

Olivier Tilloy olivier.tilloy at canonical.com
Wed Mar 29 11:05:45 UTC 2017


Overall, LGTM. I have a few minor comments, please see inline.

Diff comments:

> === modified file 'src/app/webbrowser/Browser.qml'
> --- src/app/webbrowser/Browser.qml	2017-03-28 11:20:46 +0000
> +++ src/app/webbrowser/Browser.qml	2017-03-28 14:56:40 +0000
> @@ -40,6 +40,8 @@
>      currentWebview: tabsModel && tabsModel.currentTab ? tabsModel.currentTab.webview : null
>  
>      property bool incognito: false
> +    property bool downloadsViewOpened: downloadsViewLoader.status == Loader.Ready

Instead of a property, may I suggest a signal with the same name?

> +    property bool discardFinishedDowloads: false

Typo: missing 'n'

>  
>      property var tabsModel: TabsModel {
>          // These methods are required by the TabsBar component
> 
> === added file 'src/app/webbrowser/CountEmblem.qml'
> --- src/app/webbrowser/CountEmblem.qml	1970-01-01 00:00:00 +0000
> +++ src/app/webbrowser/CountEmblem.qml	2017-03-28 14:56:40 +0000
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright 2017 Canonical Ltd.
> + *
> + * This file is part of webbrowser-app.
> + *
> + * webbrowser-app is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 3.
> + *
> + * webbrowser-app is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +import QtQuick 2.4
> +import Ubuntu.Components 1.3
> +
> +Item {
> +    id: countEmblem
> +
> +    property int count: 0
> +    property bool pulsating: false
> +    property real maxWidth: parent.width

maxWidth can be marked readonly

> +
> +    implicitWidth: Math.min(maxWidth, Math.max(units.gu(2), countLabel.implicitWidth + units.gu(1)))
> +    implicitHeight: units.gu(2)
> +    visible: countEmblem.count > 0
> +
> +    UbuntuShape {
> +        anchors.fill: parent
> +        anchors.margins: -units.dp(1)
> +        backgroundColor: Qt.darker(backgroundShape.backgroundColor, 1.3)
> +        aspect: UbuntuShape.Flat
> +
> +        opacity: 0
> +        SequentialAnimation on opacity {
> +            running: countEmblem.pulsating
> +            alwaysRunToEnd: true
> +            loops: Animation.Infinite
> +            NumberAnimation { to: 1; duration: UbuntuAnimation.SlowDuration; easing: UbuntuAnimation.StandardEasingReverse }
> +            NumberAnimation { to: 0; duration: UbuntuAnimation.SlowDuration; easing: UbuntuAnimation.StandardEasing }
> +        }
> +    }
> +
> +    UbuntuShape {
> +        id: backgroundShape
> +        anchors.fill: parent
> +        backgroundColor: pulsating ? theme.palette.normal.activity : theme.palette.normal.positive
> +        aspect: UbuntuShape.Flat
> +    }
> +
> +    Label {
> +        id: countLabel
> +        text: countEmblem.count
> +        anchors.centerIn: parent
> +        width: countEmblem.maxWidth - units.gu(1)
> +        horizontalAlignment: Text.AlignHCenter
> +        elide: Text.ElideRight
> +        color: pulsating ? theme.palette.normal.activityText : theme.palette.normal.positiveText
> +        textSize: Label.Small
> +    }
> +}
> 
> === modified file 'src/app/webbrowser/NavigationBar.qml'
> --- src/app/webbrowser/NavigationBar.qml	2017-02-03 15:11:00 +0000
> +++ src/app/webbrowser/NavigationBar.qml	2017-03-28 14:56:40 +0000
> @@ -206,6 +207,18 @@
>                          internal.openDrawer.opened = true
>                      }
>                  }
> +
> +                CountEmblem {
> +                    anchors {
> +                        right: parent.right
> +                        top: parent.top
> +                        topMargin: units.gu(0.5)
> +                    }
> +                    property var downloadsInProgress: root.downloadManager ? root.downloadManager.downloadsInProgress : []
> +                    property var downloadsFinished: root.downloadManager ? root.downloadManager.downloadsFinished : []

Those two properties can be marked readonly.

> +                    count: downloadsInProgress.length + downloadsFinished.length
> +                    pulsating: downloadsInProgress.length > 0
> +                }
>              }
>          }
>      }
> 
> === modified file 'src/app/webbrowser/webbrowser-app.qml'
> --- src/app/webbrowser/webbrowser-app.qml	2017-02-23 09:53:09 +0000
> +++ src/app/webbrowser/webbrowser-app.qml	2017-03-28 14:56:40 +0000
> @@ -85,11 +85,26 @@
>          window.requestActivate()
>      }
>  
> +    function updateDiscardFinishedDowloads() {

Typo: missing 'n'

> +        for (var i = allWindows.length - 1; i >= 0; --i) {
> +            var window = allWindows[i]
> +            if (window.downloadsViewOpened) {
> +                discardFinishedDowloads = true;
> +                return;
> +            }
> +        }
> +        discardFinishedDowloads = false;
> +    }
> +
> +    property bool discardFinishedDowloads: false
> +
>      property var windowFactory: Component {
>          BrowserWindow {
>              id: window
>  
>              property alias incognito: browser.incognito
> +            property alias downloadsViewOpened: browser.downloadsViewOpened
> +            onDownloadsViewOpenedChanged: webbrowserapp.updateDiscardFinishedDowloads()
>              readonly property alias model: browser.tabsModel
>              readonly property var tabsModel: browser.tabsModel
>  


-- 
https://code.launchpad.net/~fboucault/webbrowser-app/downloads_emblem/+merge/319588
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app/staging.



More information about the Ubuntu-reviews mailing list