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 2022/09/08 19:05:11 UTC
[GitHub] [echarts] maddhruv opened a new pull request, #17636: fix: add width, height, x,y to CustomSeriesRenderItemParamsCoordSys type
maddhruv opened a new pull request, #17636:
URL: https://github.com/apache/echarts/pull/17636
<!-- Please fill in the following information to help us review your PR more efficiently. -->
## Brief Information
Adding more params -- `width`, `height`, `x`, `y` to `CustomSeriesRenderItemParamsCoordSys` type, which can be useful when creating custom rectangle shapes with echarts.
```js
const rectShape = echarts.graphic.clipRectByRect(
{
x: start[0],
y: start[1] - height / 2,
width: end[0] - start[0],
height: height,
},
{
x: params.coordSys.x ?? 0,
y: params.coordSys.y ?? 0,
width: params.coordSys.width ?? 0,
height: params.coordSys.height ?? 0,
}
);
```
This pull request is in the type of:
- [ ] bug fixing
- [ ] new feature
- [x] others
### What does this PR do?
Adding more params to `CustomSeriesRenderItemParamsCoordSys` type, which can be useful when creating custom rectangle shapes with echarts.
### Fixed issues
<!--
- #xxxx: ...
-->
## Details
### Before: What was the problem?
TypeScript check fails while creating custom rectangles using `coorsSys`.
### After: How does it behave after the fixing?
Fixing the type interface
## Document Info
One of the following should be checked.
- [ ] This PR doesn't relate to document changes
- [ ] The document should be updated later
- [ ] The document changes have been made in apache/echarts-doc#xxx
## Misc
### ZRender Changes
- [ ] This PR depends on ZRender changes (ecomfe/zrender#xxx).
### Related test cases or examples to use the new APIs
N.A.
## Others
### Merging options
- [ ] Please squash the commits into a single one when merging.
### 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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] maddhruv commented on pull request #17636: fix: add width, height, x,y to CustomSeriesRenderItemParamsCoordSys type
Posted by GitBox <gi...@apache.org>.
maddhruv commented on PR #17636:
URL: https://github.com/apache/echarts/pull/17636#issuecomment-1250738049
Yeah @Ovilia I can totally do this
```
interface CustomSeriesRenderItemParamsCoordSys {
type: string;
[key: string]: unknown;
}
```
But I would still have a problem with `echarts.graphic.clipRectByRect` where it accepts the `RectLike` to be of `number` so I would either have to typecast my values there, to make this work -
```
const rect = echarts.graphic.clipRectByRect(
{x: 1},
{x: (params.coordSys.x ?? 0) as number}
)
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] echarts-bot[bot] commented on pull request #17636: fix: add width, height, x,y to CustomSeriesRenderItemParamsCoordSys type
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on PR #17636:
URL: https://github.com/apache/echarts/pull/17636#issuecomment-1241115014
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/echarts/wiki/How-to-make-a-pull-request).
⚠️ MISSING DOCUMENT INFO: Please make sure one of the document options are checked in this PR's description. Search "Document Info" in the description of this PR. This should be done either by the author or the reviewers of the PR.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] Ovilia commented on pull request #17636: fix: add width, height, x,y to CustomSeriesRenderItemParamsCoordSys type
Posted by GitBox <gi...@apache.org>.
Ovilia commented on PR #17636:
URL: https://github.com/apache/echarts/pull/17636#issuecomment-1246296217
The type of `params.coordSys` is not wrong. Custom series can be used in different coordinates, `width, height, x, y` exist only when using it with cartesian coordinate system.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] Ovilia closed pull request #17636: fix: add width, height, x,y to CustomSeriesRenderItemParamsCoordSys type
Posted by GitBox <gi...@apache.org>.
Ovilia closed pull request #17636: fix: add width, height, x,y to CustomSeriesRenderItemParamsCoordSys type
URL: https://github.com/apache/echarts/pull/17636
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] Ovilia commented on pull request #17636: fix: add width, height, x,y to CustomSeriesRenderItemParamsCoordSys type
Posted by GitBox <gi...@apache.org>.
Ovilia commented on PR #17636:
URL: https://github.com/apache/echarts/pull/17636#issuecomment-1251907781
@maddhruv Yes, this would be inevitable.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] maddhruv commented on pull request #17636: fix: add width, height, x,y to CustomSeriesRenderItemParamsCoordSys type
Posted by GitBox <gi...@apache.org>.
maddhruv commented on PR #17636:
URL: https://github.com/apache/echarts/pull/17636#issuecomment-1241729557
> You are probably using bmap
I don't think I am
I am using this chart example - https://echarts.apache.org/examples/en/editor.html?c=custom-profile check Line 44
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] jjansen72 commented on pull request #17636: fix: add width, height, x,y to CustomSeriesRenderItemParamsCoordSys type
Posted by GitBox <gi...@apache.org>.
jjansen72 commented on PR #17636:
URL: https://github.com/apache/echarts/pull/17636#issuecomment-1245130378
> `CustomSeriesRenderItemParamsCoordSys` does not contain those properties. You are probably using bmap so the type should be changed [with bmap](https://github.com/apache/echarts/blob/master/extension-src/bmap/BMapCoordSys.ts#L94). But this file uses `@ts-nocheck`, so changing this typing won't help. So I would like to suggest converting types in your own code and leave this PR closed. Thanks for your contribution!
I'm confused as well. The example (https://echarts.apache.org/examples/en/editor.html?c=custom-profile) is working, but the type of `params.coordSys` seems to be wrong. Could you give us an example of how to change the types? Or could it be, that the example is outdated?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] maddhruv commented on pull request #17636: fix: add width, height, x,y to CustomSeriesRenderItemParamsCoordSys type
Posted by GitBox <gi...@apache.org>.
maddhruv commented on PR #17636:
URL: https://github.com/apache/echarts/pull/17636#issuecomment-1246460613
> exist only when using it with cartesian coordinate system.
That is why I have introduced them as optional properties, as they might be undefined in other cases.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] Ovilia commented on pull request #17636: fix: add width, height, x,y to CustomSeriesRenderItemParamsCoordSys type
Posted by GitBox <gi...@apache.org>.
Ovilia commented on PR #17636:
URL: https://github.com/apache/echarts/pull/17636#issuecomment-1250568519
@maddhruv `x, y, width, height` is only for the cartesian coordinate system. Just as it's not reasonable to reveal child property in parent class, they should be included in `CustomSeriesRenderItemParamsCoordSys`. I think we could add `[key: string]: unknown;` to fix the typing problem 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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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] [echarts] infacto commented on pull request #17636: fix: add width, height, x,y to CustomSeriesRenderItemParamsCoordSys type
Posted by "infacto (via GitHub)" <gi...@apache.org>.
infacto commented on PR #17636:
URL: https://github.com/apache/echarts/pull/17636#issuecomment-1419159890
Why not just extend the type with the required properties (x,y,width,height)? Exported from ECharts.
Quest: Create the [custom example](https://echarts.apache.org/examples/en/editor.html?c=custom-profile) in TypeScript too. And provide the necessary types. No need to create custom types for things coming from this library. Also avoid `any` / `unknown` type or unknown additional property like `{ [key: string]: unknown }`. TS !== TS if not used correctly.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
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