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/26 22:17:19 UTC
[GitHub] [incubator-echarts] WillPower98 opened a new pull request #13489: Fix 13413
WillPower98 opened a new pull request #13489:
URL: https://github.com/apache/incubator-echarts/pull/13489
This is a second version of a pull request I made yesterday.
## Brief Information
This pull request is in the type of:
new feature
### What does this PR do?
adds a new feature to change the size of the tooltip for the developer
### After: How is it fixed in this PR?
export function getTooltipMarker(color: ColorString, size?: string, extraCssText?: string): TooltipMarker;
### Are there any API changes?
- [x ] The API has been changed.
const marker = getTooltipMarker({
color: colorStr,
type: markerType,
renderMode,
markerId: markerId,
size: "10px"
});
### Related test cases or examples to use the new APIs
the size can be adjusted in src/tooltip/toolTipMarkUp.ts
----------------------------------------------------------------
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] susiwen8 edited a comment on pull request #13489: feat(tooltip): added marker options. close #13413
Posted by GitBox <gi...@apache.org>.
susiwen8 edited a comment on pull request #13489:
URL: https://github.com/apache/incubator-echarts/pull/13489#issuecomment-717645383
Hi @WillPower98 package-lock.json also doesn''t need to commit.
Please be follow code style guide, run `npm rum lint` before commit
test case still missing
----------------------------------------------------------------
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] WillPower98 commented on pull request #13489: feat(tooltip): added marker options. close #13413
Posted by GitBox <gi...@apache.org>.
WillPower98 commented on pull request #13489:
URL: https://github.com/apache/incubator-echarts/pull/13489#issuecomment-717620340
Hey I made changes is it possible u can merge. ty.
----------------------------------------------------------------
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] echarts-bot[bot] commented on pull request #13489: Fix 13413
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #13489:
URL: https://github.com/apache/incubator-echarts/pull/13489#issuecomment-716854195
Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout [the coding standard](https://echarts.apache.org/en/coding-standard.html) and Wiki about [How to make a pull request](https://github.com/apache/incubator-echarts/wiki/How-to-make-a-pull-request).
----------------------------------------------------------------
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] chfw commented on a change in pull request #13489: feat(tooltip): added marker options. close #13413
Posted by GitBox <gi...@apache.org>.
chfw commented on a change in pull request #13489:
URL: https://github.com/apache/incubator-echarts/pull/13489#discussion_r546970820
##########
File path: src/util/format.ts
##########
@@ -202,19 +202,26 @@ interface GetTooltipMarkerOpt {
// id name for marker. If only one marker is in a rich text, this can be omitted.
// By default: 'markerX'
markerId?: string;
+ size?: string;
}
// Only support color string
-export function getTooltipMarker(color: ColorString, extraCssText?: string): TooltipMarker;
+export function getTooltipMarker(color: ColorString, size?: string, extraCssText?: string): TooltipMarker;
export function getTooltipMarker(opt: GetTooltipMarkerOpt): TooltipMarker;
export function getTooltipMarker(inOpt: ColorString | GetTooltipMarkerOpt, extraCssText?: string): TooltipMarker {
const opt = zrUtil.isString(inOpt) ? {
color: inOpt,
- extraCssText: extraCssText
+ extraCssText: extraCssText,
+ size: inOpt
} : (inOpt || {}) as GetTooltipMarkerOpt;
const color = opt.color;
const type = opt.type;
extraCssText = opt.extraCssText;
const renderMode = opt.renderMode || 'html';
+ let size = opt.size;
+
+ if (!size) {
+ size = '10px';
+ }
Review comment:
can the initialization of 'size' be done in the consistent way as `renderMode`?
----------------------------------------------------------------
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] chfw commented on a change in pull request #13489: feat(tooltip): added marker options. close #13413
Posted by GitBox <gi...@apache.org>.
chfw commented on a change in pull request #13489:
URL: https://github.com/apache/incubator-echarts/pull/13489#discussion_r546971653
##########
File path: src/util/format.ts
##########
@@ -227,7 +234,7 @@ export function getTooltipMarker(inOpt: ColorString | GetTooltipMarkerOpt, extra
// Only support string
+ encodeHTML(color) + ';' + (extraCssText || '') + '"></span>'
: '<span style="display:inline-block;margin-right:4px;'
- + 'border-radius:10px;width:10px;height:10px;background-color:'
+ + 'border-radius:' + size + ';width:' + size + ';height:' + size + ';background-color:'
Review comment:
you may want to use ` symbol so that you can use ${size} instead of '+ size + '. 1 character less to type.
----------------------------------------------------------------
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] WillPower98 commented on a change in pull request #13489: feat(tooltip): added marker options. close #13413
Posted by GitBox <gi...@apache.org>.
WillPower98 commented on a change in pull request #13489:
URL: https://github.com/apache/incubator-echarts/pull/13489#discussion_r513144768
##########
File path: src/util/format.ts
##########
@@ -202,19 +202,26 @@ interface GetTooltipMarkerOpt {
// id name for marker. If only one marker is in a rich text, this can be omitted.
// By default: 'markerX'
markerId?: string;
+ size?: string;
}
// Only support color string
-export function getTooltipMarker(color: ColorString, extraCssText?: string): TooltipMarker;
+export function getTooltipMarker(color: ColorString, size?: string, extraCssText?: string): TooltipMarker;
export function getTooltipMarker(opt: GetTooltipMarkerOpt): TooltipMarker;
export function getTooltipMarker(inOpt: ColorString | GetTooltipMarkerOpt, extraCssText?: string): TooltipMarker {
const opt = zrUtil.isString(inOpt) ? {
color: inOpt,
- extraCssText: extraCssText
+ extraCssText: extraCssText,
+ size: inOpt
Review comment:
ok.
----------------------------------------------------------------
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] susiwen8 commented on pull request #13489: feat(tooltip): added marker options. close #13413
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on pull request #13489:
URL: https://github.com/apache/incubator-echarts/pull/13489#issuecomment-716927867
Please add test case.
----------------------------------------------------------------
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] susiwen8 commented on pull request #13489: feat(tooltip): added marker options. close #13413
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on pull request #13489:
URL: https://github.com/apache/incubator-echarts/pull/13489#issuecomment-716921621
Thanks for it, but please do not commit anything in `dist` folder.
----------------------------------------------------------------
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] WillPower98 commented on a change in pull request #13489: feat(tooltip): added marker options. close #13413
Posted by GitBox <gi...@apache.org>.
WillPower98 commented on a change in pull request #13489:
URL: https://github.com/apache/incubator-echarts/pull/13489#discussion_r512375016
##########
File path: src/util/format.ts
##########
@@ -227,7 +233,7 @@ export function getTooltipMarker(inOpt: ColorString | GetTooltipMarkerOpt, extra
// Only support string
+ encodeHTML(color) + ';' + (extraCssText || '') + '"></span>'
: '<span style="display:inline-block;margin-right:4px;'
- + 'border-radius:10px;width:10px;height:10px;background-color:'
+ + 'border-radius:' + size + ';width:' + size + ';height:' + size + ';background-color:'
Review comment:
im not sure if I should do the same for the subitem.
----------------------------------------------------------------
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] susiwen8 commented on a change in pull request #13489: feat(tooltip): added marker options. close #13413
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on a change in pull request #13489:
URL: https://github.com/apache/incubator-echarts/pull/13489#discussion_r513131659
##########
File path: src/util/format.ts
##########
@@ -202,19 +202,26 @@ interface GetTooltipMarkerOpt {
// id name for marker. If only one marker is in a rich text, this can be omitted.
// By default: 'markerX'
markerId?: string;
+ size?: string;
}
// Only support color string
-export function getTooltipMarker(color: ColorString, extraCssText?: string): TooltipMarker;
+export function getTooltipMarker(color: ColorString, size?: string, extraCssText?: string): TooltipMarker;
export function getTooltipMarker(opt: GetTooltipMarkerOpt): TooltipMarker;
export function getTooltipMarker(inOpt: ColorString | GetTooltipMarkerOpt, extraCssText?: string): TooltipMarker {
const opt = zrUtil.isString(inOpt) ? {
color: inOpt,
- extraCssText: extraCssText
+ extraCssText: extraCssText,
+ size: inOpt
Review comment:
`getTooltipMarker ` was called in [here](https://github.com/apache/incubator-echarts/blob/next/src/component/tooltip/tooltipMarkup.ts#L484)
So you might want add an attribute `size` for `getTooltipMarker`
```js
const marker = getTooltipMarker({
color: colorStr,
type: markerType,
renderMode,
markerId: markerId,
size: size
});
```
You should track all the way up to find the function that starts to build marker and pass `size` at that function
----------------------------------------------------------------
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] susiwen8 commented on pull request #13489: feat(tooltip): added marker options. close #13413
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on pull request #13489:
URL: https://github.com/apache/incubator-echarts/pull/13489#issuecomment-717645383
Hi @WillPower98 package-lock.json also doesn''t need to commit.
Please be follow code style guide, run `npm rum lint` before commit
----------------------------------------------------------------
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] WillPower98 commented on a change in pull request #13489: feat(tooltip): added marker options. close #13413
Posted by GitBox <gi...@apache.org>.
WillPower98 commented on a change in pull request #13489:
URL: https://github.com/apache/incubator-echarts/pull/13489#discussion_r512374321
##########
File path: src/util/format.ts
##########
@@ -215,6 +216,11 @@ export function getTooltipMarker(inOpt: ColorString | GetTooltipMarkerOpt, extra
const type = opt.type;
extraCssText = opt.extraCssText;
const renderMode = opt.renderMode || 'html';
+ let size = opt.size;
Review comment:
i will add this as a parameter.
----------------------------------------------------------------
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] susiwen8 commented on a change in pull request #13489: feat(tooltip): added marker options. close #13413
Posted by GitBox <gi...@apache.org>.
susiwen8 commented on a change in pull request #13489:
URL: https://github.com/apache/incubator-echarts/pull/13489#discussion_r512369355
##########
File path: src/util/format.ts
##########
@@ -215,6 +216,11 @@ export function getTooltipMarker(inOpt: ColorString | GetTooltipMarkerOpt, extra
const type = opt.type;
extraCssText = opt.extraCssText;
const renderMode = opt.renderMode || 'html';
+ let size = opt.size;
Review comment:
Where does `size` pass into function, Did you forget to commit `tooltipMarkup.ts`?
##########
File path: src/component/tooltip/tooltipMarkup.ts
##########
@@ -485,7 +485,8 @@ export class TooltipMarkupStyleCreator {
color: colorStr,
type: markerType,
renderMode,
- markerId: markerId
+ markerId: markerId,
+ size: "10px"
Review comment:
lint: please use `'` single quote
##########
File path: src/util/format.ts
##########
@@ -227,7 +233,7 @@ export function getTooltipMarker(inOpt: ColorString | GetTooltipMarkerOpt, extra
// Only support string
+ encodeHTML(color) + ';' + (extraCssText || '') + '"></span>'
: '<span style="display:inline-block;margin-right:4px;'
- + 'border-radius:10px;width:10px;height:10px;background-color:'
+ + 'border-radius:' + size + ';width:' + size + ';height:' + size + ';background-color:'
Review comment:
There are two `rendermode` for `tooltip`: `html` and `canvas`. You should also change code in `canvas` mode. just below it.
----------------------------------------------------------------
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