Merge lp:~osomon/webbrowser-app/locationBarController-show-hide into lp:webbrowser-app

Proposed by Olivier Tilloy
Status: Merged
Approved by: Olivier Tilloy
Approved revision: 973
Merged at revision: 1007
Proposed branch: lp:~osomon/webbrowser-app/locationBarController-show-hide
Merge into: lp:webbrowser-app
Diff against target: 162 lines (+51/-46)
4 files modified
debian/control (+1/-1)
src/app/ChromeController.qml (+39/-30)
src/app/webbrowser/Browser.qml (+9/-7)
src/app/webcontainer/WebApp.qml (+2/-8)
To merge this branch: bzr merge lp:~osomon/webbrowser-app/locationBarController-show-hide
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing
Alexandre Abreu (community) Approve
Review via email: mp+256679@code.launchpad.net

Commit message

Use the new Oxide APIs to better control visibility of the chrome.
This bumps the runtime dependency on liboxideqt-qmlplugin to 1.7.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alexandre Abreu (abreu-alexandre) wrote :

A few comments inline

Revision history for this message
Olivier Tilloy (osomon) wrote :

Answered inline.

Related to visibility/behavior of the chrome, I just filed bug #1453908. I don’t think its resolution should block merging this in (as the issue already exists with webbrowser-app trunk).

Revision history for this message
Alexandre Abreu (abreu-alexandre) :
review: Approve
969. By Olivier Tilloy

One ChromeController for all webviews is enough.

970. By Olivier Tilloy

Default to ModeShown on desktop, where we don’t want the chrome to auto-hide when scrolling anyway.

971. By Olivier Tilloy

Work around bug #1453908 by forcing ModeShown while loading until the load is committed or stopped.

972. By Olivier Tilloy

Merge the latest changes from trunk.

973. By Olivier Tilloy

Also set the default mode in the webapp container.

Revision history for this message
Olivier Tilloy (osomon) wrote :

Updated with further changes, would you mind giving it another round of review?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/control'
--- debian/control 2015-05-06 21:50:57 +0000
+++ debian/control 2015-05-11 19:26:41 +0000
@@ -33,7 +33,7 @@
33Depends: ${misc:Depends},33Depends: ${misc:Depends},
34 ${shlibs:Depends},34 ${shlibs:Depends},
35 fonts-liberation,35 fonts-liberation,
36 liboxideqt-qmlplugin (>= 1.5),36 liboxideqt-qmlplugin (>= 1.7),
37 libqt5sql5-sqlite,37 libqt5sql5-sqlite,
38 qml-module-qt-labs-folderlistmodel,38 qml-module-qt-labs-folderlistmodel,
39 qml-module-qt-labs-settings,39 qml-module-qt-labs-settings,
4040
=== modified file 'src/app/ChromeController.qml'
--- src/app/ChromeController.qml 2015-03-30 18:29:58 +0000
+++ src/app/ChromeController.qml 2015-05-11 19:26:41 +0000
@@ -17,48 +17,57 @@
17 */17 */
1818
19import QtQuick 2.019import QtQuick 2.0
20import com.canonical.Oxide 1.5 as Oxide20import com.canonical.Oxide 1.7 as Oxide
2121
22Item {22Item {
23 visible: false23 visible: false
2424
25 property var webview25 property var webview
26 property bool forceHide: false26 property bool forceHide: false
2727 property int defaultMode: Oxide.LocationBarController.ModeAuto
28 readonly property int mode: {28
29 if (forceHide || webview.fullscreen) {29 onForceHideChanged: {
30 return Oxide.LocationBarController.ModeHidden30 if (!webview) {
31 } else if (internal.forceShow) {31 return
32 return Oxide.LocationBarController.ModeShown32 }
33 } else {33 webview.locationBarController.animated = false
34 return Oxide.LocationBarController.ModeAuto34 if (forceHide) {
35 }35 webview.locationBarController.mode = Oxide.LocationBarController.ModeHidden
36 }36 } else if (!webview.fullscreen) {
3737 webview.locationBarController.mode = defaultMode
38 // Work around the lack of a show() method on the location bar controller38 if (webview.locationBarController.mode == Oxide.LocationBarController.ModeAuto) {
39 // (https://launchpad.net/bugs/1422920) by forcing its mode to ModeShown39 webview.locationBarController.show(false)
40 // for long enough (500ms) to allow the animation to be committed.40 }
41 QtObject {41 }
42 id: internal42 webview.locationBarController.animated = true
43 property bool forceShow: false43 }
44 }44
45 Timer {
46 id: delayedResetMode
47 interval: 500
48 onTriggered: internal.forceShow = false
49 }
50 Connections {45 Connections {
51 target: webview46 target: webview
52 onFullscreenChanged: {47 onFullscreenChanged: {
53 if (!webview.fullscreen) {48 if (webview.fullscreen) {
54 internal.forceShow = true49 webview.locationBarController.mode = Oxide.LocationBarController.ModeHidden
55 delayedResetMode.restart()50 } else if (!forceHide) {
51 webview.locationBarController.mode = defaultMode
52 if (webview.locationBarController.mode == Oxide.LocationBarController.ModeAuto) {
53 webview.locationBarController.show(true)
54 }
56 }55 }
57 }56 }
58 onLoadingChanged: {57 onLoadingChanged: {
59 if (webview.loading && !webview.fullscreen) {58 if (webview.loading && !webview.fullscreen && !forceHide &&
60 internal.forceShow = true59 (webview.locationBarController.mode == Oxide.LocationBarController.ModeAuto)) {
61 delayedResetMode.restart()60 webview.locationBarController.show(true)
61 }
62 }
63 onLoadEvent: {
64 // When loading, force ModeShown until the load is committed or stopped,
65 // to work around https://launchpad.net/bugs/1453908.
66 if (event.type == Oxide.LoadEvent.TypeStarted) {
67 webview.locationBarController.mode = Oxide.LocationBarController.ModeShown
68 } else if ((event.type == Oxide.LoadEvent.TypeCommitted) ||
69 (event.type == Oxide.LoadEvent.TypeStopped)) {
70 webview.locationBarController.mode = defaultMode
62 }71 }
63 }72 }
64 }73 }
6574
=== modified file 'src/app/webbrowser/Browser.qml'
--- src/app/webbrowser/Browser.qml 2015-05-06 21:51:16 +0000
+++ src/app/webbrowser/Browser.qml 2015-05-11 19:26:41 +0000
@@ -256,6 +256,14 @@
256 ]256 ]
257 }257 }
258258
259 ChromeController {
260 id: chromeController
261 webview: browser.currentWebview
262 forceHide: recentView.visible
263 defaultMode: (formFactor == "desktop") ? Oxide.LocationBarController.ModeShown
264 : Oxide.LocationBarController.ModeAuto
265 }
266
259 Suggestions {267 Suggestions {
260 id: suggestionsList268 id: suggestionsList
261 opacity: ((chrome.state == "shown") && chrome.activeFocus && (count > 0) && !chrome.drawerOpen) ? 1.0 : 0.0269 opacity: ((chrome.state == "shown") && chrome.activeFocus && (count > 0) && !chrome.drawerOpen) ? 1.0 : 0.0
@@ -588,15 +596,9 @@
588596
589 enabled: visible && !bottomEdgeHandle.dragging && !recentView.visible597 enabled: visible && !bottomEdgeHandle.dragging && !recentView.visible
590598
591 ChromeController {
592 id: chromeController
593 webview: webviewimpl
594 forceHide: recentView.visible
595 }
596
597 locationBarController {599 locationBarController {
598 height: webviewimpl.visible ? chrome.height : 0600 height: webviewimpl.visible ? chrome.height : 0
599 mode: chromeController.mode601 mode: chromeController.defaultMode
600 }602 }
601603
602 //experimental.preferences.developerExtrasEnabled: developerExtrasEnabled604 //experimental.preferences.developerExtrasEnabled: developerExtrasEnabled
603605
=== modified file 'src/app/webcontainer/WebApp.qml'
--- src/app/webcontainer/WebApp.qml 2015-04-08 13:20:57 +0000
+++ src/app/webcontainer/WebApp.qml 2015-05-11 19:26:41 +0000
@@ -143,16 +143,10 @@
143 }143 }
144144
145 ChromeController {145 ChromeController {
146 id: oxideChromeController
147 webview: webapp.currentWebview146 webview: webapp.currentWebview
148 forceHide: webapp.chromeless147 forceHide: webapp.chromeless
149 }148 defaultMode: (formFactor == "desktop") ? Oxide.LocationBarController.ModeShown
150149 : Oxide.LocationBarController.ModeAuto
151 Binding {
152 when: webapp.currentWebview
153 target: webapp.currentWebview ? webapp.currentWebview.locationBarController : null
154 property: 'mode'
155 value: oxideChromeController.mode
156 }150 }
157 }151 }
158152

Subscribers

People subscribed via source and target branches

to status/vote changes: