Mattflaschen has submitted this change and it was merged.

Change subject: Fix issue with styleArrow and cleanup position and attach code:
......................................................................


Fix issue with styleArrow and cleanup position and attach code:

* Don't attach in createGuider(), since we're going to do so in show()
anyway.
* Convert string position to numeric position as early as possible,
createGuider and (since the latter is also public) getFlippedPosition.
* Fix incorrect behavior when 0 passed to getFlippedPosition.
* Minor code cleanup.

Change-Id: Icf53c4e06f50f9c3765ee13b5e45bc03fc30ee0b
---
M README.md
M guiders-1.2.8.js
2 files changed, 34 insertions(+), 25 deletions(-)

Approvals:
  Mattflaschen: Verified
  Spage: Looks good to me, approved



diff --git a/README.md b/README.md
index 5fddb53..ac64b41 100644
--- a/README.md
+++ b/README.md
@@ -41,7 +41,12 @@
 });
 ~~~~
 
-Note that if you can use `initGuider()` instead of `createGuider()`. The main 
difference is that `initGuider()` initializes a guider step without actually 
creating it. This is useful when you have steps where the guider being 
`attachTo`'d may ore may not be in the DOM yet. If you use `createGuider()` you 
would get an error like: `base is null [Break On This Error] var top = base top;
+Guiders are cached in memory after creation, but are positioned every time 
they are shown, and can be repositioned mwith `guiders.reposition()` if needed. 
 You should choose between the following two methods for setting up a guider.
+
+- `initGuider()` stores the information needed to create the guider, but 
defers the work of creating it.  This work will generally occur when the guider 
is first shown.
+- `createGuider()` creates the guider, doing the necessary work immediately.
+
+In either case, call `.show()` when ready to show the guider.
 
 The parameters for creating guiders are:
 
@@ -122,4 +127,4 @@
 License
 -------
 
-Released under the [Apache License 
2.0](http://www.apache.org/licenses/LICENSE-2.0.html).
\ No newline at end of file
+Released under the [Apache License 
2.0](http://www.apache.org/licenses/LICENSE-2.0.html).
diff --git a/guiders-1.2.8.js b/guiders-1.2.8.js
index fc7125a..ccc7812 100644
--- a/guiders-1.2.8.js
+++ b/guiders-1.2.8.js
@@ -83,7 +83,10 @@
     onHide: null,
     onShow: null,
     overlay: false,
-    position: 0, // 1-12 follows an analog clock, 0 means centered
+
+    // 1-12 follows an analog clock, 0 means centered.  You can also use the 
string positions
+    // listed below at guiders._offsetNameMapping, such as "topRight".
+    position: 0,
     shouldSkip: null, //function handler that allows you to skip this function 
if returns true.
     title: "Sample title goes here",
     width: 400,
@@ -170,6 +173,8 @@
   guiders._guiders = {}; //stores created guiders indexed by id
   guiders._lastCreatedGuiderID = null;
   guiders._nextButtonTitle = "Next";
+
+  // See position above in guiders._defaultSettings
   guiders._offsetNameMapping = {
     "topLeft": 11,
     "top": 12,
@@ -303,7 +308,9 @@
    * for various scenarios, such as handling right-to-left languages and 
flipping a position
    * if the original one would go off screen.
    *
-   * @param {string|number} position position as in guider initialization 
object
+   * It accepts both string (e.g. "top") and numeric (e.g. 12) positions.
+   *
+   * @param {string|number} position position as in guider settings object
    * @param {Object} options how to flip
    * @param {boolean} options.vertical true to flip vertical (optional, 
defaults false)
    * @param {boolean} options.horizontal true to flip vertical (optional, 
defaults false)
@@ -321,6 +328,14 @@
       position = guiders._offsetNameMapping[position];
     }
 
+    position = Number( position );
+
+    if ( position === 0 ) {
+      // Don't change center position.
+      return position;
+    }
+
+    // This math is all based on the analog clock model used for guiders 
positioning.
     if (options.horizontal && !options.vertical) {
       position = TOP_CLOCK - position;
     } else if ( options.vertical && !options.horizontal ) {
@@ -359,13 +374,6 @@
     // topMarginOfBody corrects positioning if body has a top margin set on it.
     var topMarginOfBody = $("body").outerHeight(true) - 
$("body").outerHeight(false);
     top -= topMarginOfBody;
-
-    // Now, take into account how the guider should be positioned relative to 
the attachTo element.
-    // e.g. top left, bottom center, etc.
-    if (guiders._offsetNameMapping[guider.position]) {
-      // As an alternative to the clock model, you can also use keywords to 
position the guider.
-      guider.position = guiders._offsetNameMapping[guider.position];
-    }
 
     var attachToHeight = attachTo.innerHeight();
     var attachToWidth = attachTo.innerWidth();
@@ -495,7 +503,9 @@
   };
 
   guiders._styleArrow = function(myGuider) {
-    var position = myGuider.position || 0;
+    var position = myGuider.position || 0,
+        arrowPosition;
+
     if (!position) {
       return;
     }
@@ -536,8 +546,8 @@
       11: ["left", arrowOffset],
       12: ["left", myWidth/2 - arrowOffset]
     };
-    var position = positionMap[myGuider.position];
-    myGuiderArrow.css(position[0], position[1] + "px");
+    arrowPosition = positionMap[myGuider.position];
+    myGuiderArrow.css(arrowPosition[0], arrowPosition[1] + "px");
     // TODO: experiment with pulsing
     //myGuiderArrow.css(position[0], position[1] + 
"px").stop().pulse({backgroundPosition:["7px 0","0 
0"],right:["-35px","-42px"]}, {times: 10, duration: 'slow'});
   };
@@ -632,14 +642,7 @@
 
   /**
    * This stores the guider but does no work on it.
-   *
-   * The main problem with createGuider() is that it needs _attach() to work. 
If
-   * you try to _attachTo something that doesn't exist yet, the guider will
-   * suffer a fatal javscript error and never initialize.
-   *
-   * A secondary problem is createGuider() code is expensive on the CPU/time.
-   * This prevents more than one guider from being created at a time (it defers
-   * creation to a user-is-idle time.
+   * It is an alternative to createGuider() that defers the actual setup work.
    */
   guiders.initGuider = function(passedSettings) {
     if (passedSettings === null || passedSettings === undefined) {
@@ -682,9 +685,10 @@
     guiderElement.appendTo("body");
     guiderElement.attr("id", myGuider.id);
 
-    // Ensure myGuider.attachTo is a jQuery element.
-    if (typeof myGuider.attachTo !== "undefined" && myGuider !== null) {
-      guiders._attach(myGuider);
+    // If a string form (e.g. "top") was passed, convert it to numeric (e.g. 
12)
+    // As an alternative to the clock model, you can also use keywords to 
position the myGuider.
+    if (guiders._offsetNameMapping[myGuider.position]) {
+      myGuider.position = guiders._offsetNameMapping[myGuider.position];
     }
 
     guiders._initializeOverlay();

-- 
To view, visit https://gerrit.wikimedia.org/r/54107
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Icf53c4e06f50f9c3765ee13b5e45bc03fc30ee0b
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/GuidedTour/guiders
Gerrit-Branch: master
Gerrit-Owner: Mattflaschen <[email protected]>
Gerrit-Reviewer: Mattflaschen <[email protected]>
Gerrit-Reviewer: Ori.livneh <[email protected]>
Gerrit-Reviewer: Spage <[email protected]>
Gerrit-Reviewer: Swalling <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to