apache
feat(plugins): Make comparison values on BigNumberPeriodOverPeriod toggleable
SUMMARY
There are various scenarios where it is preferred to only show a subset of the three comparison values: previous period value, period difference, and period percent change. This change allows for hiding unwanted values.
(This is my first contribution on the frontend side so please let me know if I missed something obvious!)
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
https://github.com/apache/superset/assets/52328004/72d594b8-114f-49db-a571-0b4b31aa48bf
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: CHART_PLUGINS_EXPERIMENTAL
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in SIP-59)
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
@Antonio-RiveroMartnez Ephemeral environment spinning up at http://35.93.65.110:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@eschutho any chance this gets reviewed and merged? We'd very much love the feature!
@fontclos Ephemeral environment creation is currently limited to committers.
@rusackas Ephemeral environment spinning up at http://35.92.2.227:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.
Thanks for the contribution @mkramer5454! This looks good to me. Can you write a few tests here: superset-frontend/packages/superset-ui-core/test/time-comparison/getComparisonInfo.test.ts
And maybe one small UI improvement, when "Add color for positive/negative" is selected, there's an extra dialog that shows up. I'd suggest to keep the two fields together and minimize any jumpiness, we just move the "Add color" field to the bottom below the new checkboxes. Also cc @yousoph and @kasiazjc on any other design suggestions.
Hi @mkramer5454 Sorry for not getting back to you on this PR! We're actually looking at doing something similar for the table chart, and have come up with some designs for it. Would you be interested in updating your PR to match the new designs?
@mkramer5454 this looks great! Let me spin up a test environment. Thanks! In the meantime do you want to look at some of the linting/test errors?
@eschutho Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments
@eschutho Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments
@eschutho Ephemeral environment spinning up at http://35.95.139.123:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
Looks like the feature flag wasn't picked up. I updated the PR description with FEATURE_CHART_PLUGINS_EXPERIMENTAL=true per the docs.
Looks like the feature flag wasn't picked up. I updated the PR description with FEATURE_CHART_PLUGINS_EXPERIMENTAL=true per the docs.
Thanks @mkramer5454!
@eschutho Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments
@eschutho Ephemeral environment spinning up at http://44.242.197.170:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.