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 2021/05/06 04:11:13 UTC
[GitHub] [echarts] dougalg opened a new pull request #14871: fix(types) Export cbs and their parameter types
dougalg opened a new pull request #14871:
URL: https://github.com/apache/echarts/pull/14871
<!-- Please fill in the following information to help us review your PR more efficiently. -->
## Brief Information
This pull request is in the type of:
- [x] bug fixing
- [ ] new feature
- [ ] others
### What does this PR do?
<!-- USE ONCE SENTENCE TO DESCRIBE WHAT THIS PR DOES. -->
Adds type exports for several callbacks and their parameters
### Fixed issues
<!--
- #xxxx: ...
-->
- #14277: Export types for formatter callback params
## Details
### Before: What was the problem?
<!-- DESCRIBE THE BUG OR REQUIREMENT HERE. -->
<!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
It was difficult to implement properly types callback functions
### After: How is it fixed in this PR?
<!-- THE RESULT AFTER FIXING AND A SIMPLE EXPLANATION ABOUT HOW IT IS FIXED. -->
<!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
It should now be possible to implement properly typed callbacks
## Misc
<!-- ADD RELATED ISSUE ID WHEN APPLICABLE -->
- [ ] The API has been changed (apache/echarts-doc#xxx).
- [ ] This PR depends on ZRender changes (ecomfe/zrender#xxx).
### Related test cases or examples to use the new APIs
As far as I could tell, there are no tests needed for type exports, but if I missed them, please let me know, and I will try to add them.
## Others
### Merging options
- [ ] 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] [echarts] pissang commented on a change in pull request #14871: fix(types) Export cbs and their parameter types
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #14871:
URL: https://github.com/apache/echarts/pull/14871#discussion_r627892338
##########
File path: src/util/types.ts
##########
@@ -1235,7 +1235,7 @@ type TooltipBoxLayoutOption = Pick<
/**
* Position relative to the hoverred element. Only available when trigger is item.
*/
-interface PositionCallback {
+export interface PositionCallback {
Review comment:
PositionCallback can be renamed to TooltipPositionCallback to avoid potential conflicts when exported
--
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] [echarts] pissang commented on a change in pull request #14871: fix(types) Export cbs and their parameter types
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #14871:
URL: https://github.com/apache/echarts/pull/14871#discussion_r627082925
##########
File path: src/export/callback.ts
##########
@@ -0,0 +1,25 @@
+import type {
Review comment:
Needs Apache License in the header.
IMO these callback types can be exported in the `option`.
--
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] [echarts] dougalg commented on pull request #14871: fix(types) Export cbs and their parameter types
Posted by GitBox <gi...@apache.org>.
dougalg commented on pull request #14871:
URL: https://github.com/apache/echarts/pull/14871#issuecomment-834053631
@pissang I believe I have addressed your comments. I also noticed some inconsistencies in my naming patterns and renamed a couple of the exports to be more consistent. Thank you very much
--
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] [echarts] dougalg commented on a change in pull request #14871: fix(types) Export cbs and their parameter types
Posted by GitBox <gi...@apache.org>.
dougalg commented on a change in pull request #14871:
URL: https://github.com/apache/echarts/pull/14871#discussion_r627390199
##########
File path: src/export/callback.ts
##########
@@ -0,0 +1,25 @@
+import type {
Review comment:
Thanks for the feedback! I was considering putting them in there, but I initially wasn't sure how many exports I'd be adding and I didn't want to bloat that file too much. But I think this doesn't add too much bloat, although there are some more callbacks I didn't include as I wasn't sure exactly how they should be exported.
--
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] [echarts] pissang merged pull request #14871: fix(types) Export cbs and their parameter types
Posted by GitBox <gi...@apache.org>.
pissang merged pull request #14871:
URL: https://github.com/apache/echarts/pull/14871
--
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] [echarts] pissang commented on a change in pull request #14871: fix(types) Export cbs and their parameter types
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #14871:
URL: https://github.com/apache/echarts/pull/14871#discussion_r627893080
##########
File path: src/export/option.ts
##########
@@ -234,4 +250,17 @@ export interface EChartsOption extends ECBasicOption {
options?: EChartsOption[];
baseOption?: EChartsOption;
-}
\ No newline at end of file
+}
+
+export {
+ TooltipFormatterCallback as TooltipComponentFormatterCallback,
+ TopLevelFormatterParams as TooltipComponentFormatterParams,
+ LabelFormatterCallback,
+ CallbackDataParams as DefaultLabelFormatterParams,
+ AnimationDurationCallback,
+ AnimationDelayCallback,
+ AnimationDelayCallbackParam as AnimationDelayCallbackParams,
+ LabelLayoutOptionCallbackParams,
+ LabelLayoutOptionCallback,
+ PositionCallback
Review comment:
`PositionCallback` is only used in the tooltip. So it can also be TooltipComponentPositionCallback
--
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] [echarts] pissang commented on a change in pull request #14871: fix(types) Export cbs and their parameter types
Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #14871:
URL: https://github.com/apache/echarts/pull/14871#discussion_r627082925
##########
File path: src/export/callback.ts
##########
@@ -0,0 +1,25 @@
+import type {
Review comment:
Needs Apache License in the header.
--
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] [echarts] echarts-bot[bot] commented on pull request #14871: fix(types) Export cbs and their parameter types
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #14871:
URL: https://github.com/apache/echarts/pull/14871#issuecomment-842790294
Congratulations! Your PR has been merged. Thanks for your contribution! 👍
--
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] [echarts] echarts-bot[bot] commented on pull request #14871: fix(types) Export cbs and their parameter types
Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #14871:
URL: https://github.com/apache/echarts/pull/14871#issuecomment-833209875
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).
--
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] [echarts] dougalg commented on pull request #14871: fix(types) Export cbs and their parameter types
Posted by GitBox <gi...@apache.org>.
dougalg commented on pull request #14871:
URL: https://github.com/apache/echarts/pull/14871#issuecomment-834014221
@pissang I have moved this PR to "awaiting review" from WIP, whenever you (or others) are ready to do a final/next review.
Thank you again!
--
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