[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