[GitHub] [incubator-echarts] plainheart commented on a change in pull request #12504: feat(tooltip): tooltip formatter trigger once

[GitBox <gi...@apache.org>]

plainheart commented on a change in pull request #12504:
URL: https://github.com/apache/incubator-echarts/pull/12504#discussion_r415752591



##########
File path: src/component/tooltip/TooltipView.js
##########
@@ -586,7 +610,8 @@ export default echarts.extendComponentView({
                 }
             }, this);
             this._ticket = asyncTicket;
-            html = formatter(params, asyncTicket, callback);
+            html = cacheHtml || formatter(params, asyncTicket, callback);
+            this._setTooltipCache(key, html);

Review comment:
       If `triggerOnce` is not enabled, `enableCache` will be false, so there is no need to call `_setTooltipCache`.
   We should wrapped this code line with `if`.

##########
File path: src/component/tooltip/TooltipView.js
##########
@@ -577,6 +591,16 @@ export default echarts.extendComponentView({
             html = formatUtil.formatTpl(formatter, params, true);
         }
         else if (typeof formatter === 'function') {
+            // formatter cache
+            var cacheHtml = null;
+            var key = null;

Review comment:
       Some varaibles have no need to be assigned an initial value `null`.
   ```suggestion
               var cacheHtml;
               var key;
   ```

##########
File path: src/component/tooltip/TooltipView.js
##########
@@ -577,6 +591,16 @@ export default echarts.extendComponentView({
             html = formatUtil.formatTpl(formatter, params, true);
         }
         else if (typeof formatter === 'function') {
+            // formatter cache
+            var cacheHtml = null;
+            var key = null;
+            var enableCache = tooltipModel.get('triggerOnce');
+            if (enableCache && params && params[0]) {
+                var p = params[0];
+                key = [p.componentIndex, p.seriesId, p.seriesName, p.dataIndex].join('_');
+                cacheHtml = this._getTooltipCache(key);
+            }
+
             var callback = bind(function (cbTicket, html) {
                 if (cbTicket === this._ticket) {
                     tooltipContent.setContent(html, markers, tooltipModel);

Review comment:
       The case of `callback` in `formatter` also needs to be handled. Add cache here ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org