ubuntu-sdk-team team mailing list archive

[Albert Astals Cid <albert.astals@xxxxxxxxxxxxx>]

Albert Astals Cid has proposed merging lp:~aacid/ubuntu-ui-toolkit/fix_popup_focus_restore into lp:ubuntu-ui-toolkit/staging.

Commit message:
Fix saving/restoring of previous item focus for popups

You can't save the focus after the popup/dialog has been created,
at that stage the dialog has focus already,
you need to do it before creating it

Requested reviews:
  ubuntu-sdk-build-bot (ubuntu-sdk-build-bot): continuous-integration
  Ubuntu SDK team (ubuntu-sdk-team)

For more details, see:
https://code.launchpad.net/~aacid/ubuntu-ui-toolkit/fix_popup_focus_restore/+merge/314156
-- 
Your team Ubuntu SDK team is requested to review the proposed merge of lp:~aacid/ubuntu-ui-toolkit/fix_popup_focus_restore into lp:ubuntu-ui-toolkit/staging.

=== modified file 'src/imports/Components/Popups/1.3/PopupBase.qml'
--- src/imports/Components/Popups/1.3/PopupBase.qml	2016-04-13 19:32:20 +0000
+++ src/imports/Components/Popups/1.3/PopupBase.qml	2017-01-05 14:23:46 +0000
@@ -119,6 +119,14 @@
 
     /*!
       \internal
+      The function saves the active focus for later.
+      */
+    function __setPreviousActiveFocusItem(item) {
+        stateWrapper.prevFocus = item;
+    }
+
+    /*!
+      \internal
       Foreground component excluded from InverseMouseArea
       */
     property Item __foreground
@@ -177,30 +185,14 @@
     /*! \internal */
     onParentChanged: stateWrapper.rootItem = QuickUtils.rootItem(popupBase)
     Component.onCompleted: {
-        stateWrapper.saveActiveFocus();
         stateWrapper.rootItem = QuickUtils.rootItem(popupBase);
     }
 
     Item {
         id: stateWrapper
         property Item rootItem: QuickUtils.rootItem(popupBase)
-
-        property bool windowIsValid: typeof window != "undefined"
         property Item prevFocus
 
-        function saveActiveFocus() {
-            // 'window' context property is exposed to QML after component completion
-            // before rendering is complete, therefore a simple 'if (window)' check is
-            // not enough.
-            if (windowIsValid) {
-                prevFocus = window.activeFocusItem;
-                windowIsValidChanged.disconnect(saveActiveFocus);
-            } else {
-                // connect the function so we can save the original focus item
-                windowIsValidChanged.connect(saveActiveFocus);
-            }
-        }
-
         function restoreActiveFocus() {
             if (prevFocus) {
                 if (prevFocus.hasOwnProperty("requestFocus")) {

=== modified file 'src/imports/Components/Popups/1.3/popupUtils.js'
--- src/imports/Components/Popups/1.3/popupUtils.js	2016-09-15 15:53:27 +0000
+++ src/imports/Components/Popups/1.3/popupUtils.js	2017-01-05 14:23:46 +0000
@@ -66,6 +66,8 @@
     }
 
     var popupObject;
+    // If there's an active item, save it so we can restore it later
+    var prevFocusItem = (typeof window !== "undefined") ? window.activeFocusItem : null;
     if (params !== undefined) {
         popupObject = popupComponent.createObject(rootObject, params);
     } else {
@@ -75,8 +77,11 @@
         print(popupComponent.errorString().slice(0, -1));
         print("PopupUtils.open(): Failed to create the popup object.");
         return;
-    } else if (popupObject.hasOwnProperty("caller") && caller)
+    } else if (popupObject.hasOwnProperty("caller") && caller) {
         popupObject.caller = caller;
+    } else if (popupObject.hasOwnProperty("__setPreviousActiveFocusItem")) {
+        popupObject.__setPreviousActiveFocusItem(prevFocusItem);
+    }
 
     // if caller is specified, connect its cleanup to the popup's close
     // so popups will be removed together with the caller.

=== modified file 'tests/unit/visual/tst_popups_dialog.13.qml'
--- tests/unit/visual/tst_popups_dialog.13.qml	2016-06-15 13:46:51 +0000
+++ tests/unit/visual/tst_popups_dialog.13.qml	2017-01-05 14:23:46 +0000
@@ -47,6 +47,21 @@
             keyClick(Qt.Key_Escape);
             tryCompare(test, "dialogDestroyed", true, 500, "Dialog not destroyed");
         }
+
+        function test_focus_restore_ondismiss_dialog() {
+            pressMe.forceActiveFocus();
+
+            tryCompare(window, "activeFocusItem", pressMe);
+
+            var dlg = PopupUtils.open(dialog);
+            waitForRendering(dlg);
+
+            tryCompare(window, "activeFocusItem", dlg.button);
+
+            keyClick(Qt.Key_Escape);
+
+            tryCompare(window, "activeFocusItem", pressMe);
+        }
     }
 
     Component {
@@ -54,10 +69,13 @@
         Dialog {
             id: ahojDialog
             title: "Ahoj"
+            property alias button: closeButton
 
             Button {
+                id: closeButton
                 text: "Close"
                 onClicked: PopupUtils.close(ahojDialog)
+                focus: true
             }
         }
     }