Fix hover style bug.

src/util/graphic.js

@@ -48,6 +48,7 @@ var mathMax = Math.max;
 var mathMin = Math.min;
 
 var EMPTY_OBJ = {};
+var Z2_LIFT_VALUE = 1;
 
 /**
  * Extend shape with parameters
@@ -266,11 +267,12 @@ function cacheElementStl(el) {
 
     var hoverStyle = el.__hoverStl;
     if (!hoverStyle) {
-        el.__normalStl = null;
+        el.__cachedNormalStl = el.__cachedNormalZ2 = null;
         return;
     }
 
-    var normalStyle = el.__normalStl = {};
+    var normalStyle = el.__cachedNormalStl = {};
+    el.__cachedNormalZ2 = el.z2;
     var elStyle = el.style;
 
     for (var name in hoverStyle) {
@@ -308,9 +310,6 @@ function doSingleEnterHover(el) {
         targetStyle = elTarget.style;
     }
 
-    // Consider case: only `position: 'top'` is set on emphasis, then text
-    // color should be returned to `autoColor`, rather than remain '#fff'.
-    // So we should rollback then apply again after style merging.
     rollbackDefaultTextStyle(targetStyle);
 
     if (!useHoverLayer) {
@@ -345,7 +344,7 @@ function doSingleEnterHover(el) {
 
     if (!useHoverLayer) {
         el.dirty(false);
-        el.z2 += 1;
+        el.z2 += Z2_LIFT_VALUE;
     }
 }
 
@@ -356,32 +355,36 @@ function setDefaultHoverFillStroke(targetStyle, hoverStyle, prop) {
 }
 
 function doSingleLeaveHover(el) {
-    if (el.__highlighted) {
-        doSingleRestoreHoverStyle(el);
-        el.__highlighted = false;
+    var highlighted = el.__highlighted;
+
+    if (!highlighted) {
+        return;
     }
-}
 
-function doSingleRestoreHoverStyle(el) {
-    var highlighted = el.__highlighted;
+    el.__highlighted = false;
 
     if (highlighted === 'layer') {
         el.__zr && el.__zr.removeHover(el);
     }
     else if (highlighted) {
         var style = el.style;
-        var normalStl = el.__normalStl;
 
+        var normalStl = el.__cachedNormalStl;
         if (normalStl) {
             rollbackDefaultTextStyle(style);
-
             // Consider null/undefined value, should use
             // `setStyle` but not `extendFrom(stl, true)`.
             el.setStyle(normalStl);
-
             applyDefaultTextStyle(style);
-
-            el.z2 -= 1;
+            el.__cachedNormalStl = null;
+        }
+        // `__cachedNormalZ2` will not be reset if calling `setElementHoverStyle`
+        // when `el` is on emphasis state. So here by comparing with 1, we try
+        // hard to make the bug case rare.
+        var normalZ2 = el.__cachedNormalZ2;
+        if (el.z2 - normalZ2 === Z2_LIFT_VALUE) {
+            el.z2 = normalZ2;
+            el.__cachedNormalZ2 = null;
         }
     }
 }
@@ -395,7 +398,10 @@ function traverseCall(el, method) {
 }
 
 /**
- * Set hover style of element.
+ * Set hover style (namely "emphasis style") of element, based on the current
+ * style of the given `el`.
+ * This method should be called after all of the normal styles have been adopted
+ * to the `el`. See the reason on `setHoverStyle`.
  *
  * @param {module:zrender/Element} el Should not be `zrender/container/Group`.
  * @param {Object|boolean} [hoverStl] The specified hover style.
@@ -407,11 +413,29 @@ function traverseCall(el, method) {
  * @param {boolean} [opt.hoverSilentOnTouch=false] See `graphic.setAsHoverStyleTrigger`
  */
 export function setElementHoverStyle(el, hoverStl) {
+    // For performance consideration, it might be better to make the "hover style" only the
+    // difference properties from the "normal style", but not a entire copy of all styles.
     hoverStl = el.__hoverStl = hoverStl !== false && (hoverStl || {});
     el.__hoverStlDirty = true;
 
+    // FIXME
+    // It is not completely right to save "normal"/"emphasis" flag on elements.
+    // It probably should be saved on `data` of series. Consider the cases:
+    // (1) A highlighted elements are moved out of the view port and re-enter
+    // again by dataZoom.
+    // (2) call `setOption` and replace elements totally when they are highlighted.
     if (el.__highlighted) {
+        // Consider the case:
+        // The styles of a highlighted `el` is being updated. The new "emphasis style"
+        // should be adapted to the `el`. Notice here new "normal styles" should have
+        // been set outside and the cached "normal style" is out of date.
+        el.__cachedNormalStl = null;
+        // Do not clear `__cachedNormalZ2` here, because setting `z2` is not a constraint
+        // of this method. In most cases, `z2` is not set and hover style should be able
+        // to rollback. Of course, that would bring bug, but only in a rare case, see
+        // `doSingleLeaveHover` for details.
         doSingleLeaveHover(el);
+
         doSingleEnterHover(el);
     }
 }
@@ -460,12 +484,29 @@ function leaveEmphasis() {
 }
 
 /**
- * Set hover style of element.
+ * Set hover style (namely "emphasis style") of element,
+ * based on the current style of the given `el`.
+ *
+ * (1)
+ * **CONSTRAINTS** for this method:
+ * <A> This method MUST be called after all of the normal styles having been adopted
+ * to the `el`.
+ * <B> The input `hoverStyle` (that is, "emphasis style") MUST be the subset of the
+ * "normal style" having been set to the el.
+ * <C> `color` MUST be one of the "normal styles" (because color might be lifted as
+ * a default hover style).
  *
- * [Caveat]:
- * This method can be called repeatly and achieve the same result.
+ * The reason: this method treat the current style of the `el` as the "normal style"
+ * and cache them when enter/update the "emphasis style". Consider the case: the `el`
+ * is in "emphasis" state and `setOption`/`dispatchAction` trigger the style updating
+ * logic, where the el should shift from the original emphasis style to the new
+ * "emphasis style" and should be able to "downplay" back to the new "normal style".
  *
- * [Usage]:
+ * Indeed, it is error-prone to make a interface has so many constraints, but I have
+ * not found a better solution yet to fit the backward compatibility, performance and
+ * the current programming style.
+ *
+ * (2)
  * Call the method for a "root" element once. Do not call it for each descendants.
  * If the descendants elemenets of a group has itself hover style different from the
  * root group, we can simply mount the style on `el.hoverStyle` for them, but should
@@ -520,6 +561,7 @@ export function setAsHoverStyleTrigger(el, opt) {
 }
 
 /**
+ * See more info in `setTextStyleCommon`.
  * @param {Object|module:zrender/graphic/Style} normalStyle
  * @param {Object} emphasisStyle
  * @param {module:echarts/model/Model} normalModel
@@ -592,6 +634,7 @@ export function setLabelStyle(
 
 /**
  * Set basic textStyle properties.
+ * See more info in `setTextStyleCommon`.
  * @param {Object|module:zrender/graphic/Style} textStyle
  * @param {module:echarts/model/Model} model
  * @param {Object} [specifiedTextStyle] Can be overrided by settings in model.
@@ -610,6 +653,7 @@ export function setTextStyle(
 
 /**
  * Set text option in the style.
+ * See more info in `setTextStyleCommon`.
  * @deprecated
  * @param {Object} textStyle
  * @param {module:echarts/model/Model} labelModel
@@ -632,7 +676,23 @@ export function setText(textStyle, labelModel, defaultColor) {
 }
 
 /**
- * {
+ * The uniform entry of set text style, that is, retrieve style definitions
+ * from `model` and set to `textStyle` object.
+ *
+ * Never in merge mode, but in overwrite mode, that is, all of the text style
+ * properties will be set. (Consider the states of normal and emphasis and
+ * default value can be adopted, merge would make the logic too complicated
+ * to manage.)
+ *
+ * The `textStyle` object can either be a plain object or an instance of
+ * `zrender/src/graphic/Style`, and either be the style of normal or emphasis.
+ * After this mothod called, the `textStyle` object can then be used in
+ * `el.setStyle(textStyle)` or `el.hoverStyle = textStyle`.
+ *
+ * Default value will be adopted and `insideRollbackOpt` will be created.
+ * See `applyDefaultTextStyle` `rollbackDefaultTextStyle` for more details.
+ *
+ * opt: {
  *      disableBox: boolean, Whether diable drawing box of block (outer most).
  *      isRectText: boolean,
  *      autoColor: string, specify a color when color is 'auto',
@@ -813,14 +873,27 @@ function getAutoColor(color, opt) {
     return color !== 'auto' ? color : (opt && opt.autoColor) ? opt.autoColor : null;
 }
 
-// When text position is `inside` and `textFill` not specified, we
-// provide a mechanism to auto make text border for better view. But
-// text position changing when hovering or being emphasis should be
-// considered, where the `insideRollback` enables to restore the style.
+/**
+ * Give some default value to the input `textStyle` object, based on the current settings
+ * in this `textStyle` object.
+ *
+ * The Scenario:
+ * when text position is `inside` and `textFill` is not specified, we show
+ * text border by default for better view. But it should be considered that text position
+ * might be changed when hovering or being emphasis, where the `insideRollback` is used to
+ * restore the style.
+ *
+ * Usage (& NOTICE):
+ * When a style object (eithor plain object or instance of `zrender/src/graphic/Style`) is
+ * about to be modified on its text related properties, `rollbackDefaultTextStyle` should
+ * be called before the modification and `applyDefaultTextStyle` should be called after that.
+ * (For the case that all of the text related properties is reset, like `setTextStyleCommon`
+ * does, `rollbackDefaultTextStyle` is not needed to be called).
+ */
 function applyDefaultTextStyle(textStyle) {
     var opt = textStyle.insideRollbackOpt;
 
-    // Only insideRollbackOpt create (setTextStyleCommon used),
+    // Only `insideRollbackOpt` created (in `setTextStyleCommon`),
     // applyDefaultTextStyle works.
     if (!opt || textStyle.textFill != null) {
         return;
@@ -864,6 +937,16 @@ function applyDefaultTextStyle(textStyle) {
     }
 }
 
+/**
+ * Consider the case: in a scatter,
+ * label: {
+ *     normal: {position: 'inside'},
+ *     emphasis: {position: 'top'}
+ * }
+ * In the normal state, the `textFill` will be set as '#fff' for pretty view (see
+ * `applyDefaultTextStyle`), but when switching to emphasis state, the `textFill`
+ * should be retured to 'autoColor', but not keep '#fff'.
+ */
 function rollbackDefaultTextStyle(style) {
     var insideRollback = style.insideRollback;
     if (insideRollback) {

test/hoverStyle.html

@@ -53,6 +53,10 @@ under the License.
         <br>
 
         <div id="info"></div>
+
+        <div id="mainb1"></div>
+        <div id="mainb2"></div>
+
         <div id="main0"></div>
         <div id="main1"></div>
         <div id="main2"></div>
@@ -61,6 +65,8 @@ under the License.
         <div id="main5"></div>
 
 
+
+
         <script>
             var USE_HOVER_LAYER_KEY = '__EC_TEST_USE_HOVER_LAYER_KEY___';
 
@@ -102,6 +108,10 @@ under the License.
 
             var originalCreate = testHelper.create;
             testHelper.create = function (echarts, dom, opt) {
+                if (opt.option.hoverLayerThreshold === void 0) {
+                    throw new Error('"hoverLayerThreshold" should be set');
+                }
+
                 var buttons = opt.buttons || [];
                 opt.buttons = buttons = genHoverLayerBtns().concat(buttons);
                 var chart = originalCreate.call(this, echarts, dom, opt);
@@ -126,6 +136,99 @@ under the License.
 
 
 
+        <script>
+            require(['echarts'], function (echarts) {
+                var option = {
+                    hoverLayerThreshold: hoverLayerThreshold,
+                    xAxis: {
+                    },
+                    yAxis: {
+                    },
+                    grid: {
+                        right: 120
+                    },
+                    visualMap: {
+                        min: 100,
+                        max: 800,
+                        top: 10,
+                        right: 10,
+                        splitNumber: 3,
+                        type: 'piecewise',
+                        // demension: 1,
+                        // hoverLink: true,
+                        inRange: {
+                            color: ['blue', 'red']
+                        },
+                        outOfRange: {
+                            color: 'black'
+                        }
+                    },
+                    series: [{
+                        type: 'scatter',
+                        symbolSize: 30,
+                        data: [
+                            [12, 331],
+                            [55, 131],
+                            [55, 531]
+                        ]
+                    }]
+                };
+
+                testHelper.create(echarts, 'mainb1', {
+                    title: [
+                        'visualMap should be normal (and hoverLink should be normal)',
+                        '(inRange: has color, outOfRange: "black")'
+                    ],
+                    option: option,
+                    height: 200
+                });
+            });
+        </script>
+
+
+
+
+
+
+
+        <script>
+            require(['echarts'], function (echarts) {
+                var option = {
+                    hoverLayerThreshold: hoverLayerThreshold,
+                    xAxis: {
+                    },
+                    yAxis: {
+                    },
+                    series: [{
+                        type: 'scatter',
+                        symbolSize: 30,
+                        data: [
+                            [12, 131],
+                            [55, 431],
+                            [55, 331]
+                        ]
+                    }]
+                };
+
+                var chart = testHelper.create(echarts, 'mainb2', {
+                    title: [
+                        'setOption onclick and hover animation should be normal:',
+                        'click any symbol, all symbols should become blue'
+                    ],
+                    option: option,
+                    height: 200
+                });
+
+                chart.on('click', function () {
+                    chart.setOption({
+                        color: ['blue']
+                    });
+                });
+            });
+        </script>
+
+
+
 
 
         <script>
@@ -491,6 +594,5 @@ under the License.
 
 
 
-
     </body>
 </html>
\ No newline at end of file