You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by su...@apache.org on 2019/01/02 14:30:33 UTC
[incubator-echarts] branch release updated: Fix hover style bug.
This is an automated email from the ASF dual-hosted git repository.
sushuang pushed a commit to branch release
in repository https://gitbox.apache.org/repos/asf/incubator-echarts.git
The following commit(s) were added to refs/heads/release by this push:
new 679033d Fix hover style bug.
679033d is described below
commit 679033d118ac312683906f1159edb5d424324460
Author: sushuang <su...@gmail.com>
AuthorDate: Wed Jan 2 22:30:01 2019 +0800
Fix hover style bug.
---
src/util/graphic.js | 139 ++++++++++++++++++++++++++++++++++++++++-----------
test/hoverStyle.html | 104 +++++++++++++++++++++++++++++++++++++-
2 files changed, 214 insertions(+), 29 deletions(-)
diff --git a/src/util/graphic.js b/src/util/graphic.js
index f44b844..b9abeeb 100644
--- a/src/util/graphic.js
+++ b/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) {
diff --git a/test/hoverStyle.html b/test/hoverStyle.html
index 83b9b78..a34f6c2 100644
--- a/test/hoverStyle.html
+++ b/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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org