You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by GitBox <gi...@apache.org> on 2020/10/21 13:30:35 UTC
[GitHub] [incubator-echarts] Ghostbird opened a new pull request #13469: Allow passing of DOM nodes to setContent
Ghostbird opened a new pull request #13469:
URL: https://github.com/apache/incubator-echarts/pull/13469
<!-- Please fill in the following information to help us review your PR more efficiently. -->
## Brief Information
This pull request is in the type of:
- [ ] bug fixing
- [x] new feature
- [ ] others
### What does this PR do?
Allows Tooltip to be set with array of DOM Nodes instead of HTML string.
### Fixed issues
- #13250
## Details
### Before: What was the problem?
When the tooltip is set, you could only pass static HTML content. I am developing an Angular app. Angular – like other web frameworks (e.g Vue, React) – has its own template mechanism to generate DOM nodes and retain control over their contents, to update them dynamically. However, I could not leverage my framework inside echarts tooltip.
### After: How is it fixed in this PR?
A non-framework approach is shown in the test file `tooltip-domnode.html`. That tooltip shows the continuously updating current date.
In my Angular project. I can return `this.viewContainerRef.createEmbeddedView(template, context).rootNodes` from the `EChartOption.Tooltip.Formatter` and my tooltip content will be updated in Angular's lifecycle mechanism.
## Usage
### Are there any API changes?
- [x] The API has been changed.
Previously this was the Formatter interface in Typescript:
```typescript
interface Formatter {
(params: Format | Format[], ticket: string, callback: (ticket: string, html: string) => void): string;
}
```
Now it is this:
```typescript
interface Formatter {
(params: Format | Format[], ticket: string, callback: (ticket: string, htmlOrDomNodes: string | any[]) => void): string | any[];
}
```
### Related test cases or examples to use the new APIs
- `tooltip-domnode.html`
## Others
### Merging options
- [x] Please squash the commits into a single one when merge.
### Other information
----------------------------------------------------------------
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
[GitHub] [incubator-echarts] Ghostbird commented on pull request #13469: Allow passing of DOM nodes to setContent
Posted by GitBox <gi...@apache.org>.
Ghostbird commented on pull request #13469:
URL: https://github.com/apache/incubator-echarts/pull/13469#issuecomment-714298136
Retargeted to `next` branch.
Didn't build for me unless I removed two invalid imports:
```diff
diff --git a/src/coord/cartesian/Grid.ts b/src/coord/cartesian/Grid.ts
index 5da228c2c..88c7d33ed 100644
--- a/src/coord/cartesian/Grid.ts
+++ b/src/coord/cartesian/Grid.ts
@@ -38,7 +38,7 @@ import CoordinateSystemManager from '../../CoordinateSystem';
import {ParsedModelFinder, SINGLE_REFERRING} from '../../util/model';
// Depends on GridModel, AxisModel, which performs preprocess.
-import GridModel, {defaultGridLayoutWithoutLabel, defaultGridLayoutWithLabel} from './GridModel';
+import GridModel from './GridModel';
import CartesianAxisModel from './AxisModel';
import GlobalModel from '../../model/Global';
import ExtensionAPI from '../../ExtensionAPI';
```
----------------------------------------------------------------
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
[GitHub] [incubator-echarts] Ghostbird edited a comment on pull request #13469: Allow passing of DOM nodes to setContent
Posted by GitBox <gi...@apache.org>.
Ghostbird edited a comment on pull request #13469:
URL: https://github.com/apache/incubator-echarts/pull/13469#issuecomment-714298136
Retargeted to `next` branch.
Didn't build for me unless I removed two invalid imports (outside the scope of this PR):
```diff
diff --git a/src/coord/cartesian/Grid.ts b/src/coord/cartesian/Grid.ts
index 5da228c2c..88c7d33ed 100644
--- a/src/coord/cartesian/Grid.ts
+++ b/src/coord/cartesian/Grid.ts
@@ -38,7 +38,7 @@ import CoordinateSystemManager from '../../CoordinateSystem';
import {ParsedModelFinder, SINGLE_REFERRING} from '../../util/model';
// Depends on GridModel, AxisModel, which performs preprocess.
-import GridModel, {defaultGridLayoutWithoutLabel, defaultGridLayoutWithLabel} from './GridModel';
+import GridModel from './GridModel';
import CartesianAxisModel from './AxisModel';
import GlobalModel from '../../model/Global';
import ExtensionAPI from '../../ExtensionAPI';
```
----------------------------------------------------------------
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
[GitHub] [incubator-echarts] Ghostbird commented on a change in pull request #13469: [5.0] Allow passing of DOM nodes to setContent
Posted by GitBox <gi...@apache.org>.
Ghostbird commented on a change in pull request #13469:
URL: https://github.com/apache/incubator-echarts/pull/13469#discussion_r511815890
##########
File path: src/component/tooltip/TooltipHTMLContent.ts
##########
@@ -375,20 +375,26 @@ class TooltipHTMLContent {
return;
}
+ const el = this.el;
+
if (isString(arrowPosition) && tooltipModel.get('trigger') === 'item'
&& !shouldTooltipConfine(tooltipModel)) {
content += assembleArrow(tooltipModel.get('backgroundColor'), borderColor, arrowPosition);
}
- if (isObject(content)) {
- if (this.el.children) {
- for (var child of Array.from(this.el.children)) {
- this.el.removeChild(child);
+ if (isString(content)) {
+ el.innerHTML = content == null ? '' : content;
+ }
+ else if (content) {
+ // Clear previous
+ el.innerHTML = '';
+ if (!isArray(content)) {
+ content = [content];
+ }
+ for (let i = 0; i < content.length; i++) {
Review comment:
Thanks for cleaning this up!
I'm curious, is a `for` loop with repeated indexing better than a `for … of` loop? I know that it is more memory efficient in compiled languages where arrays are implemented as contiguous areas of memory, but I had no idea the same was true for Javascript.
----------------------------------------------------------------
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
[GitHub] [incubator-echarts] Ghostbird commented on pull request #13469: Allow passing of DOM nodes to setContent
Posted by GitBox <gi...@apache.org>.
Ghostbird commented on pull request #13469:
URL: https://github.com/apache/incubator-echarts/pull/13469#issuecomment-714298846
Retargeted to `next` branch.
----------------------------------------------------------------
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
[GitHub] [incubator-echarts] pissang commented on pull request #13469: Allow passing of DOM nodes to setContent
Posted by GitBox <gi...@apache.org>.
pissang commented on pull request #13469:
URL: https://github.com/apache/incubator-echarts/pull/13469#issuecomment-714200917
Thanks for your contribution! Do you mind retargeting the PR to the `next` branch? It's our current developing branch and has been rewritten with typescript language.
----------------------------------------------------------------
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
[GitHub] [incubator-echarts] pissang merged pull request #13469: Allow passing of DOM nodes to setContent
Posted by GitBox <gi...@apache.org>.
pissang merged pull request #13469:
URL: https://github.com/apache/incubator-echarts/pull/13469
----------------------------------------------------------------
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
[GitHub] [incubator-echarts] Ghostbird removed a comment on pull request #13469: Allow passing of DOM nodes to setContent
Posted by GitBox <gi...@apache.org>.
Ghostbird removed a comment on pull request #13469:
URL: https://github.com/apache/incubator-echarts/pull/13469#issuecomment-714298136
Retargeted to `next` branch.
Didn't build for me unless I removed two invalid imports (outside the scope of this PR):
```diff
diff --git a/src/coord/cartesian/Grid.ts b/src/coord/cartesian/Grid.ts
index 5da228c2c..88c7d33ed 100644
--- a/src/coord/cartesian/Grid.ts
+++ b/src/coord/cartesian/Grid.ts
@@ -38,7 +38,7 @@ import CoordinateSystemManager from '../../CoordinateSystem';
import {ParsedModelFinder, SINGLE_REFERRING} from '../../util/model';
// Depends on GridModel, AxisModel, which performs preprocess.
-import GridModel, {defaultGridLayoutWithoutLabel, defaultGridLayoutWithLabel} from './GridModel';
+import GridModel from './GridModel';
import CartesianAxisModel from './AxisModel';
import GlobalModel from '../../model/Global';
import ExtensionAPI from '../../ExtensionAPI';
```
----------------------------------------------------------------
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