You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/04/16 23:03:50 UTC
[GitHub] [incubator-superset] ktmud opened a new pull request #9562: Add raw
number/integer option to d3 number formats
ktmud opened a new pull request #9562: Add raw number/integer option to d3 number formats
URL: https://github.com/apache/incubator-superset/pull/9562
### CATEGORY
- [x] Enhancement (new features, refinement)
### SUMMARY
@graceguo-supercat mentioned we don't even have an integer type in number formatting. So let's add one. Also add a "original number" type in case users want to see the raw number.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
### TEST PLAN
1. Go to any chart with number formatting (e.g. Big Number).
2. Make sure the supported formats work as expected.
### ADDITIONAL INFORMATION
- [x] Changes UI
### REVIEWERS
@graceguo-supercat @kristw @evans
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on issue #9562: Add raw
number/integer option to d3 number formats
Posted by GitBox <gi...@apache.org>.
ktmud commented on issue #9562: Add raw number/integer option to d3 number formats
URL: https://github.com/apache/incubator-superset/pull/9562#issuecomment-615390210
I agree the formatters should probably have a localized name and the `superset-ui` registries seem to be the perfect place for it.
Should we hold off this change before that refactor happens or merge it first since it's a fairly small change?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on issue #9562: Add raw
number/integer option to d3 number formats
Posted by GitBox <gi...@apache.org>.
villebro commented on issue #9562: Add raw number/integer option to d3 number formats
URL: https://github.com/apache/incubator-superset/pull/9562#issuecomment-615207017
Not something directly related to this PR, but [durationFormatter](https://github.com/apache-superset/superset-ui/pull/209) expects parameters as a JSON object. It would be great to have support for specifying custom options for any available formatter in the registry, instead of having to hard code them in `setupFormatters.js`. So something where one can choose a formatter, and then pass a string containing options, which for d3 would be a regular d3 format string, and for duration formatter would be a string that is parsed into a JSON object.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw commented on a change in pull request
#9562: Add raw number/integer option to d3 number formats
Posted by GitBox <gi...@apache.org>.
kristw commented on a change in pull request #9562: Add raw number/integer option to d3 number formats
URL: https://github.com/apache/incubator-superset/pull/9562#discussion_r409913915
##########
File path: superset-frontend/src/explore/controls.jsx
##########
@@ -83,6 +83,8 @@ export const PRIMARY_COLOR = { r: 0, g: 122, b: 135, a: 1 };
// input choices & options
export const D3_FORMAT_OPTIONS = [
['SMART_NUMBER', 'Adaptative formating'],
+ [' ', 'Original number'],
+ [',d', ',d (12345.432 => 12,345)'],
Review comment:
How about changing the description
`,d (12345.432 => 12,345)` to `Integer (12345.432 => 12,345)`
(optional) There are also constants in the package.
```ts
import { NumberFormats } from '@superset-ui/number-format';
NumberFormats.INTEGER
NumberFormats.SIGNED_INTEGER
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] rusackas commented on issue #9562: Add raw
number/integer option to d3 number formats
Posted by GitBox <gi...@apache.org>.
rusackas commented on issue #9562: Add raw number/integer option to d3 number formats
URL: https://github.com/apache/incubator-superset/pull/9562#issuecomment-615326816
Just a heads-up, this is one of those oddities in `controls.jsx` that's fairly pervasive, and will need to be addressed in some way. In this case, probably moving it to `superset-ui` to be exported/imported from there.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] graceguo-supercat commented on issue #9562:
Add raw number/integer option to d3 number formats
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on issue #9562: Add raw number/integer option to d3 number formats
URL: https://github.com/apache/incubator-superset/pull/9562#issuecomment-615511736
I would vote for `merge it first`. refactor to superset-ui registries seems a long term project.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] kristw commented on issue #9562: Add raw
number/integer option to d3 number formats
Posted by GitBox <gi...@apache.org>.
kristw commented on issue #9562: Add raw number/integer option to d3 number formats
URL: https://github.com/apache/incubator-superset/pull/9562#issuecomment-615512864
@graceguo-supercat +1
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on a change in pull request
#9562: Add raw number/integer option to d3 number formats
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #9562: Add raw number/integer option to d3 number formats
URL: https://github.com/apache/incubator-superset/pull/9562#discussion_r409926324
##########
File path: superset-frontend/src/explore/controls.jsx
##########
@@ -83,6 +83,8 @@ export const PRIMARY_COLOR = { r: 0, g: 122, b: 135, a: 1 };
// input choices & options
export const D3_FORMAT_OPTIONS = [
['SMART_NUMBER', 'Adaptative formating'],
+ [' ', 'Original number'],
+ [',d', ',d (12345.432 => 12,345)'],
Review comment:
Some languages uses "," for decimal points, so it's probably better for i18n by being more specific.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org