[Merge] lp:~abreu-alexandre/webbrowser-app/context-menu-to-overlay-webviews into lp:webbrowser-app
Alberto Mardegan
alberto.mardegan at canonical.com
Thu Feb 4 07:50:20 UTC 2016
Review: Needs Fixing
A couple of minor inline comments.
Diff comments:
>
> === modified file 'src/app/webcontainer/PopupWindowOverlay.qml'
> --- src/app/webcontainer/PopupWindowOverlay.qml 2015-12-01 20:11:50 +0000
> +++ src/app/webcontainer/PopupWindowOverlay.qml 2016-02-03 14:56:25 +0000
> @@ -30,7 +30,8 @@
> property alias currentWebview: popupWebview
> property alias request: popupWebview.request
> property alias url: popupWebview.url
> -
> + property bool wide: false
Since you have the "wide" property set to false in WebappWebview, I don't see a reason not to have a property alias here.
> +
> signal webviewUrlChanged(url webviewUrl)
>
> Rectangle {
>
> === modified file 'src/app/webcontainer/WebappContainerWebview.qml'
> --- src/app/webcontainer/WebappContainerWebview.qml 2016-01-08 16:09:01 +0000
> +++ src/app/webcontainer/WebappContainerWebview.qml 2016-02-03 14:56:25 +0000
> @@ -44,7 +44,12 @@
> signal samlRequestUrlPatternReceived(string urlPattern)
> signal themeColorMetaInformationDetected(string theme_color)
>
> - onWideChanged: if (webappContainerWebViewLoader.item) webappContainerWebViewLoader.item.wide = wide
> + onWideChanged: {
> + if (webappContainerWebViewLoader.item) {
> + webappContainerWebViewLoader.item.wide = wide
> + popupController.wide = wide
There is no need to change this: since you are using a property binding below, popupController.wide should always have the right value.
> + }
> + }
>
> PopupWindowController {
> id: popupController
--
https://code.launchpad.net/~abreu-alexandre/webbrowser-app/context-menu-to-overlay-webviews/+merge/282489
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.
More information about the Ubuntu-reviews
mailing list