[Merge] lp:~michael-sheldon/webbrowser-app/link-share into lp:webbrowser-app
Olivier Tilloy
olivier.tilloy at canonical.com
Wed Jun 25 15:58:03 UTC 2014
Review: Needs Fixing
Seeing a functional issue here: if I clear the address bar, then dismiss the chrome (leaving the address bar empty), then show it again and long press on the empty address bar, I’m still getting the context menu with the share option, even though there doesn’t seem to be anything to share (in fact what happens is that the URL of the current view is being shared, despite the address bar being empty, but I think this is counter-intuitive).
On a related note, instead of disabling the share action when chrome.url is empty or on desktop, this condition should be tested in the onPressAndHold slot before showing the popup. Indeed, the implementation of PopupUtils.open(…) doesn’t check whether the popup is empty, and what happens on desktop is that an empty popover is being shown (and it looks ugly).
Diff comments:
> === modified file 'po/webbrowser-app.pot'
> --- po/webbrowser-app.pot 2014-06-18 08:33:20 +0000
> +++ po/webbrowser-app.pot 2014-06-23 12:00:30 +0000
> @@ -8,7 +8,7 @@
> msgstr ""
> "Project-Id-Version: webbrowser-app\n"
> "Report-Msgid-Bugs-To: \n"
> -"POT-Creation-Date: 2014-06-18 10:33+0200\n"
> +"POT-Creation-Date: 2014-06-23 12:57+0100\n"
> "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
> "Last-Translator: FULL NAME <EMAIL at ADDRESS>\n"
> "Language-Team: LANGUAGE <LL at li.org>\n"
> @@ -81,15 +81,15 @@
> msgid "Back to safety"
> msgstr ""
>
> -#: src/app/Chrome.qml:72 src/app/actions/Back.qml:23
> +#: src/app/Chrome.qml:75 src/app/actions/Back.qml:23
> msgid "Back"
> msgstr ""
>
> -#: src/app/Chrome.qml:91 src/app/actions/Forward.qml:23
> +#: src/app/Chrome.qml:94 src/app/actions/Forward.qml:23
> msgid "Forward"
> msgstr ""
>
> -#: src/app/Chrome.qml:126 src/app/webbrowser/ActivityView.qml:36
> +#: src/app/Chrome.qml:154 src/app/webbrowser/ActivityView.qml:36
> msgid "Activity"
> msgstr ""
>
> @@ -244,6 +244,10 @@
> msgid "Save image"
> msgstr ""
>
> +#: src/app/actions/ShareLink.qml:22
> +msgid "Share..."
> +msgstr ""
> +
> #: src/app/webbrowser/ActivityView.qml:54
> msgid "Bookmarks"
> msgstr ""
>
> === modified file 'src/app/AddressBar.qml'
> --- src/app/AddressBar.qml 2014-06-06 17:01:55 +0000
> +++ src/app/AddressBar.qml 2014-06-23 12:00:30 +0000
> @@ -29,6 +29,7 @@
> property bool loading
> signal requestReload()
> signal requestStop()
> + signal pressAndHold()
>
> height: textField.height
>
> @@ -120,6 +121,9 @@
> textField.forceActiveFocus()
> textField.selectAll()
> }
> + onPressAndHold: {
> + addressbar.pressAndHold()
> + }
> }
> }
>
>
> === modified file 'src/app/Chrome.qml'
> --- src/app/Chrome.qml 2014-05-22 12:38:25 +0000
> +++ src/app/Chrome.qml 2014-06-23 12:00:30 +0000
> @@ -18,6 +18,8 @@
>
> import QtQuick 2.0
> import Ubuntu.Components 0.1
> +import Ubuntu.Components.Popups 0.1
> +import "actions" as Actions
>
> Item {
> id: chrome
> @@ -25,6 +27,7 @@
> property alias url: addressBar.actualUrl
> signal urlValidated(url url)
> property alias addressBar: addressBar
> + property string title
> property alias loading: addressBar.loading
> property alias loadProgress: progressBar.value
> property alias canGoBack: backButton.enabled
> @@ -92,6 +95,30 @@
> onTriggered: chrome.goForwardClicked()
> }
>
> + Component {
> + id: addressBarPopover
> + ActionSelectionPopover {
> + actions: ActionList {
> + Actions.ShareLink {
> + enabled: chrome.url && shareLoader.status == Loader.Ready
> + onTriggered: shareLoader.item.shareLink(chrome.url, chrome.title)
> + }
> + }
> + }
> + }
> +
> + Item {
> + id: addressBarPopOverPositioner
> + anchors.bottom: addressBar.top
> + anchors.horizontalCenter: addressBar.horizontalCenter
> + visible: false
> + }
> +
> + Loader {
> + id: shareLoader
> + source: formFactor == "desktop" ? "" : "Share.qml"
> + }
> +
> AddressBar {
> id: addressBar
> objectName: "addressBar"
> @@ -107,6 +134,7 @@
> onValidated: chrome.urlValidated(requestedUrl)
> onRequestReload: chrome.requestReload()
> onRequestStop: chrome.requestStop()
> + onPressAndHold: PopupUtils.open(addressBarPopover, addressBarPopOverPositioner)
> visible: addressBarVisible
> }
>
>
> === added file 'src/app/ContentShareDialog.qml'
> --- src/app/ContentShareDialog.qml 1970-01-01 00:00:00 +0000
> +++ src/app/ContentShareDialog.qml 2014-06-23 12:00:30 +0000
> @@ -0,0 +1,50 @@
> +/*
> + * Copyright 2014 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.0
> +import Ubuntu.Components 0.1
> +import Ubuntu.Components.Popups 0.1
> +import Ubuntu.Content 0.1
> +
> +PopupBase {
> + id: shareDialog
> + anchors.fill: parent
> + property var activeTransfer
> + property var items: []
> + property alias contentType: peerPicker.contentType
> +
> + Rectangle {
> + anchors.fill: parent
> + ContentPeerPicker {
> + id: peerPicker
> + handler: ContentHandler.Share
> + visible: parent.visible
> +
> + onPeerSelected: {
> + activeTransfer = peer.request()
> + activeTransfer.items = shareDialog.items
> + activeTransfer.state = ContentTransfer.Charged
> + shareDialog.hide()
> + }
> +
> + onCancelPressed: {
> + shareDialog.hide()
> + }
> + }
> + }
> +}
>
> === modified file 'src/app/PanelLoader.qml'
> --- src/app/PanelLoader.qml 2014-06-11 07:03:29 +0000
> +++ src/app/PanelLoader.qml 2014-06-23 12:00:30 +0000
> @@ -75,6 +75,7 @@
> id: chrome
>
> anchors.fill: parent
> + title: currentWebview.title
>
> Connections {
> target: chromePanel
>
> === added file 'src/app/Share.qml'
> --- src/app/Share.qml 1970-01-01 00:00:00 +0000
> +++ src/app/Share.qml 2014-06-23 12:00:30 +0000
> @@ -0,0 +1,46 @@
> +/*
> + * Copyright 2014 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.0
> +import Ubuntu.Components 0.1
> +import Ubuntu.DownloadManager 0.1
> +import Ubuntu.Content 0.1
> +
> +Item {
> + id: shareItem
> +
> + ContentShareDialog {
> + id: shareDialog
> + }
> +
> + Component {
> + id: contentItemComponent
> + ContentItem { }
> + }
> +
> + function share(url, name, contentType) {
> + shareDialog.contentType = contentType
> + shareDialog.items.push(contentItemComponent.createObject(shareItem, {"url" : url, "name" : name}))
> + shareDialog.show()
> + }
> +
> + function shareLink(url, title) {
> + share(url, title, ContentType.Links)
> + }
> +
> +}
>
> === added file 'src/app/actions/ShareLink.qml'
> --- src/app/actions/ShareLink.qml 1970-01-01 00:00:00 +0000
> +++ src/app/actions/ShareLink.qml 2014-06-23 12:00:30 +0000
> @@ -0,0 +1,24 @@
> +/*
> + * Copyright 2014 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 Ubuntu.Components 0.1
> +
> +Action {
> + text: i18n.tr("Share...")
For consistency with the other contextual actions, the label should probably be "Share link".
> + iconName: "share"
> +}
>
--
https://code.launchpad.net/~michael-sheldon/webbrowser-app/link-share/+merge/223421
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.
More information about the Ubuntu-reviews
mailing list