[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