100pah commented on a change in pull request #9171: fix #8009 & #5969, symbol
symbolSize and opacity setting for category itemStyle in graph
URL: https://github.com/apache/incubator-echarts/pull/9171#discussion_r227323477
##########
File path: src/chart/graph/categoryVisual.js
##########
@@ -47,14 +60,30 @@ export default function (ecModel) {
if (typeof category === 'string') {
category = categoryNameIdxMap['ec-' + category];
}
- if (!data.getItemVisual(idx, 'color', true)) {
- data.setItemVisual(
- idx, 'color',
- categoriesData.getItemVisual(category, 'color')
- );
+
+ var itemStyleList = ['color', 'opacity'];
+
+ for (var i = 0; i < itemStyleList.length; i++) {
+ if (!data.getItemVisual(idx, itemStyleList[i], true)) {
+ data.setItemVisual(
+ idx, itemStyleList[i],
+ categoriesData.getItemVisual(category,
itemStyleList[i])
+ );
+ }
+ }
+
+ var symbolStyleList = ['symbol', 'symbolSize',
'symbolKeepAspect'];
+
+ for (var i = 0; i < symbolStyleList.length; i++) {
+ if (!data.getItemVisual(idx, symbolStyleList[i],
true)) {
+ data.setItemVisual(
+ idx, symbolStyleList[i],
+ categoriesData.getItemVisual(category,
symbolStyleList[i])
+ );
+ }
}
}
});
}
});
-}
\ No newline at end of file
+}
Review comment:
It is a little complicated about this issue.
First of all, there is a "programming contract" about when and how to use
the properies in option to render a chart in echarts:
Tow cases:
**(A)**
Basically, properties are retrieved from model (essentially, from "option"
in model) directly when rendering in xxxView.
The model cascade mechanism ensure that get the right property.
The "cascade" means, for example:
+ itemModel - seriesModel (by default)
+ itemModel - categoryModel - seriesModel (in graph)
+ itemModel - levelModel - seriesModel (in tree, treemap, ...）
+ ... (other cases)
**(B)**
If a properties is available to be modified by other components (e.g.,
`legend`, `visualMap`),
it will be retrieved in the "visual encoding stage", and be saved to data
(by `data.setVisual() / data.setItemVisual()`). When chart is finally being
rendered in xxxView, it will
be retrieved from data (by `data.getItemVisual()`).
---
The "contract" is a little complicated and a little fuzzy. Why not just
trace them all via `data.set/getItemVisual`? I think there is two reasons:
(1) A chart needs many properties. If we retrieve them all and save to
`data` and finally retrieve from `data` and
then render, it brings some redundancy of logic and code.
(2) Save obj by the unit of `data item` probably cause performance issue
(mainly, GC issue) in large data.
---
In this case, we have these properties to be considered:
`color`, `opacity`, `symbolSize`, `symbol`, `symbolOffset`, `symbolRotate`,
`symbolKeepAspect`.
Probably we have to trade them in different way:
(1) Follow to the contract (B) above:
`color` (implemented)
`opacity` (not implemented yet, can be implemented in `categoryVisual` like
this PR did.）
`symbolSize` (not implemented yet, can be implemented in `categoryVisual`
like this PR did.)
(2) Follow to the contract (A) above:
`symbolOffset` (implemented)
`symbolRotate` (implemented)
`symbol` (not implemented yet)
(3)
`symbolKeepAspect` (?)
I am not sure about this property. Currently it has been implemented follow
the (A), but
probably it should not be able to modified by other components I think. What
your opinion on it ?
@Ovilia
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
