You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "gbusch (via GitHub)" <gi...@apache.org> on 2023/02/12 15:02:28 UTC
[GitHub] [superset] gbusch opened a new pull request, #23064: feat: conditional coloring for big number chart
gbusch opened a new pull request, #23064:
URL: https://github.com/apache/superset/pull/23064
### SUMMARY
As suggested here: #21820
allow for conditional formatting (coloring) of Big Number charts.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<img width="1261" alt="Screenshot 2023-02-12 at 15 58 36" src="https://user-images.githubusercontent.com/16560193/218318705-20c14e5e-a091-4d57-a02d-2cce9324b155.png">
### TESTING INSTRUCTIONS
* select a "Big Number" chart
* add "conditional formatting" rules in the "Customize" tab
### ADDITIONAL INFORMATION
re-uses existing "ConditionalFormattingControl" from chart table and pivot table
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: conditional coloring for big number chart [superset]
Posted by "mtrentz (via GitHub)" <gi...@apache.org>.
mtrentz commented on PR #23064:
URL: https://github.com/apache/superset/pull/23064#issuecomment-2057114328
@rusackas opened this one #28038
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] gbusch commented on pull request #23064: feat: conditional coloring for big number chart
Posted by "gbusch (via GitHub)" <gi...@apache.org>.
gbusch commented on PR #23064:
URL: https://github.com/apache/superset/pull/23064#issuecomment-1509861876
Sorry, was busy and then almost forgot about it...
Colors are defined in this file:
superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx
I renamed the colors to make it clearer that they refer to the theme colors and added possibility for darker colors (from theme). Does this make sense?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #23064: feat: conditional coloring for big number chart
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #23064:
URL: https://github.com/apache/superset/pull/23064#issuecomment-1427070564
# [Codecov](https://codecov.io/gh/apache/superset/pull/23064?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#23064](https://codecov.io/gh/apache/superset/pull/23064?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7b840ae) into [master](https://codecov.io/gh/apache/superset/commit/17fbb2dbb2357417d81de01308264031606a661f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (17fbb2d) will **decrease** coverage by `0.02%`.
> The diff coverage is `22.72%`.
```diff
@@ Coverage Diff @@
## master #23064 +/- ##
==========================================
- Coverage 67.47% 67.46% -0.02%
==========================================
Files 1880 1880
Lines 72283 72303 +20
Branches 7881 7892 +11
==========================================
+ Hits 48772 48776 +4
- Misses 21486 21499 +13
- Partials 2025 2028 +3
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `53.88% <22.72%> (-0.02%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/23064?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...rts/src/BigNumber/BigNumberTotal/transformProps.ts](https://codecov.io/gh/apache/superset/pull/23064?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvQmlnTnVtYmVyL0JpZ051bWJlclRvdGFsL3RyYW5zZm9ybVByb3BzLnRz) | `0.00% <0.00%> (ø)` | |
| [...lugin-chart-echarts/src/BigNumber/BigNumberViz.tsx](https://codecov.io/gh/apache/superset/pull/23064?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvQmlnTnVtYmVyL0JpZ051bWJlclZpei50c3g=) | `0.00% <0.00%> (ø)` | |
| [...harts/src/BigNumber/BigNumberTotal/controlPanel.ts](https://codecov.io/gh/apache/superset/pull/23064?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvQmlnTnVtYmVyL0JpZ051bWJlclRvdGFsL2NvbnRyb2xQYW5lbC50cw==) | `30.00% <22.22%> (-70.00%)` | :arrow_down: |
| [...-ui-chart-controls/src/utils/getColorFormatters.ts](https://codecov.io/gh/apache/superset/pull/23064?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3V0aWxzL2dldENvbG9yRm9ybWF0dGVycy50cw==) | `100.00% <100.00%> (ø)` | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] justin-tomlinson commented on pull request #23064: feat: conditional coloring for big number chart
Posted by "justin-tomlinson (via GitHub)" <gi...@apache.org>.
justin-tomlinson commented on PR #23064:
URL: https://github.com/apache/superset/pull/23064#issuecomment-1447518246
Very useful improvement. It would be great if this was available across the Big Number and Big Number with trend line. Some additional improvements to functionality could be:
- Make the colour scheme be more configurable by using hex codes or RGB values
- Making the colour selection gradient optional. sometimes you want it to be a set colour when it meets the rule rather than be some gradient of the colour. I.e If measure >0.5 then Red, not a gradient of red.
This probably applies to the conditional formatting on the table chart too. -
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on a diff in pull request #23064: feat: conditional coloring for big number chart
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #23064:
URL: https://github.com/apache/superset/pull/23064#discussion_r1104830891
##########
superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/controlPanel.ts:
##########
@@ -89,6 +90,45 @@ export default {
},
},
],
+ [
+ {
+ name: 'conditional_formatting',
+ config: {
+ type: 'ConditionalFormattingControl',
+ renderTrigger: true,
+ label: t('Conditional Formatting'),
+ description: t('Apply conditional color formatting to metrics'),
Review Comment:
A bit pedantic, but should it just be "metric" in this 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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas merged pull request #23064: feat: conditional coloring for big number chart
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #23064:
URL: https://github.com/apache/superset/pull/23064
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] gbusch commented on a diff in pull request #23064: feat: conditional coloring for big number chart
Posted by "gbusch (via GitHub)" <gi...@apache.org>.
gbusch commented on code in PR #23064:
URL: https://github.com/apache/superset/pull/23064#discussion_r1167560181
##########
superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/controlPanel.ts:
##########
@@ -89,6 +90,45 @@ export default {
},
},
],
+ [
+ {
+ name: 'conditional_formatting',
+ config: {
+ type: 'ConditionalFormattingControl',
+ renderTrigger: true,
+ label: t('Conditional Formatting'),
+ description: t('Apply conditional color formatting to metrics'),
Review Comment:
true, done
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on a diff in pull request #23064: feat: conditional coloring for big number chart
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #23064:
URL: https://github.com/apache/superset/pull/23064#discussion_r1104833418
##########
superset-frontend/packages/superset-ui-chart-controls/test/utils/getColorFormatters.test.ts:
##########
@@ -188,6 +188,22 @@ describe('getColorFunction()', () => {
expect(colorFunction(150)).toBeUndefined();
});
+ it('getColorFunction BETWEEN_OR_EQUAL without opacity', () => {
+ const colorFunction = getColorFunction(
+ {
+ operator: COMPARATOR.BETWEEN_OR_EQUAL,
+ targetValueLeft: 50,
+ targetValueRight: 100,
+ colorScheme: '#FF0000',
+ column: 'count',
+ },
+ countValues,
+ false,
+ );
+ expect(colorFunction(50)).toEqual('#FF0000');
+ expect(colorFunction(100)).toEqual('#FF0000');
Review Comment:
Would it be worth adding an assertion in the middle of the range (e.g. 75) for good measure?
I'm also not sure if there's another test that asserts against values outside the range (e.g. 25 or 125 in this case). If not, that might be good to add in this test or a new one.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #23064: feat: conditional coloring for big number chart
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #23064:
URL: https://github.com/apache/superset/pull/23064#issuecomment-1428390855
@rusackas Ephemeral environment spinning up at http://34.222.110.51:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] feat: conditional coloring for big number chart [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #23064:
URL: https://github.com/apache/superset/pull/23064#issuecomment-1922404241
> Big Numbers doesn't get color formatted when its value is zero. Anyone else experience this?
>
> If I set a rule for "error when < 5" and the value is 0, nothing happens.
>
> If the value is 0.0001 then it colors.
Would you be willing to open a PR or an Issue about this? We haven't heard much outcry, and would love any help you can provide to set it on a course toward resolution.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on pull request #23064: feat: conditional coloring for big number chart
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #23064:
URL: https://github.com/apache/superset/pull/23064#issuecomment-1509495556
@gbusch curious about your input on any of the above comments... would love to see this PR get through! :)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] gbusch commented on a diff in pull request #23064: feat: conditional coloring for big number chart
Posted by "gbusch (via GitHub)" <gi...@apache.org>.
gbusch commented on code in PR #23064:
URL: https://github.com/apache/superset/pull/23064#discussion_r1167560254
##########
superset-frontend/packages/superset-ui-chart-controls/test/utils/getColorFormatters.test.ts:
##########
@@ -188,6 +188,22 @@ describe('getColorFunction()', () => {
expect(colorFunction(150)).toBeUndefined();
});
+ it('getColorFunction BETWEEN_OR_EQUAL without opacity', () => {
+ const colorFunction = getColorFunction(
+ {
+ operator: COMPARATOR.BETWEEN_OR_EQUAL,
+ targetValueLeft: 50,
+ targetValueRight: 100,
+ colorScheme: '#FF0000',
+ column: 'count',
+ },
+ countValues,
+ false,
+ );
+ expect(colorFunction(50)).toEqual('#FF0000');
+ expect(colorFunction(100)).toEqual('#FF0000');
Review Comment:
added tests, please resolve it 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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on pull request #23064: feat: conditional coloring for big number chart
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #23064:
URL: https://github.com/apache/superset/pull/23064#issuecomment-1430195960
Code looks good, and it seems to work well on the ephemeral environment. The only question I have is whether it's possible/reasonable to make the colors full opacity rather than the sort of pastel colors that they are now. I'm a little worried about contrast/legibility in particular.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] gbusch commented on a diff in pull request #23064: feat: conditional coloring for big number chart
Posted by "gbusch (via GitHub)" <gi...@apache.org>.
gbusch commented on code in PR #23064:
URL: https://github.com/apache/superset/pull/23064#discussion_r1167560492
##########
superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/controlPanel.ts:
##########
@@ -89,6 +90,45 @@ export default {
},
},
],
+ [
+ {
+ name: 'conditional_formatting',
+ config: {
+ type: 'ConditionalFormattingControl',
+ renderTrigger: true,
+ label: t('Conditional Formatting'),
+ description: t('Apply conditional color formatting to metrics'),
+ shouldMapStateToProps() {
+ return true;
+ },
+ mapStateToProps(explore, _, chart) {
+ const verboseMap = explore?.datasource?.hasOwnProperty(
+ 'verbose_map',
+ )
+ ? (explore?.datasource as Dataset)?.verbose_map
+ : explore?.datasource?.columns ?? {};
+ const { colnames, coltypes } =
+ chart?.queriesResponse?.[0] ?? {};
+ const numericColumns =
Review Comment:
might make sense but would prefer not to do it 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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] mtrentz commented on pull request #23064: feat: conditional coloring for big number chart
Posted by "mtrentz (via GitHub)" <gi...@apache.org>.
mtrentz commented on PR #23064:
URL: https://github.com/apache/superset/pull/23064#issuecomment-1717897520
Big Numbers doesn't get color formatted when its value is zero. Anyone else experience this?
If I set a rule for "error when < 5" and the value is 0, nothing happens.
If the value is 0.0001 then it colors.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] gbusch commented on pull request #23064: feat: conditional coloring for big number chart
Posted by "gbusch (via GitHub)" <gi...@apache.org>.
gbusch commented on PR #23064:
URL: https://github.com/apache/superset/pull/23064#issuecomment-1520550028
@rusackas What do you think?
Pipeline is red... Don't understand why, as the changes are rather small.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] kgabryje commented on pull request #23064: feat: conditional coloring for big number chart
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #23064:
URL: https://github.com/apache/superset/pull/23064#issuecomment-1521544709
> Pipeline is red... Don't understand why, as the changes are rather small.
The E2E tests are sometimes flaky... I restarted the CI, hopefully it passes this time
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on pull request #23064: feat: conditional coloring for big number chart
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #23064:
URL: https://github.com/apache/superset/pull/23064#issuecomment-1428388514
/testenv up
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on a diff in pull request #23064: feat: conditional coloring for big number chart
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #23064:
URL: https://github.com/apache/superset/pull/23064#discussion_r1104835263
##########
superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/controlPanel.ts:
##########
@@ -89,6 +90,45 @@ export default {
},
},
],
+ [
+ {
+ name: 'conditional_formatting',
+ config: {
+ type: 'ConditionalFormattingControl',
+ renderTrigger: true,
+ label: t('Conditional Formatting'),
+ description: t('Apply conditional color formatting to metrics'),
+ shouldMapStateToProps() {
+ return true;
+ },
+ mapStateToProps(explore, _, chart) {
+ const verboseMap = explore?.datasource?.hasOwnProperty(
+ 'verbose_map',
+ )
+ ? (explore?.datasource as Dataset)?.verbose_map
+ : explore?.datasource?.columns ?? {};
+ const { colnames, coltypes } =
+ chart?.queriesResponse?.[0] ?? {};
+ const numericColumns =
Review Comment:
Doesn't need to be in this PR, but I'm wondering if all this logic is now duplicated in 3+ places. If so, it might be worth centralizing/DRY-ing up.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org