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/03/24 08:41:27 UTC
[GitHub] [incubator-superset] villebro opened a new pull request #9360:
Migrate unique Heatmap controls
villebro opened a new pull request #9360: Migrate unique Heatmap controls
URL: https://github.com/apache/incubator-superset/pull/9360
### CATEGORY
Choose one
- [ ] Bug Fix
- [ ] Enhancement (new features, refinement)
- [x] Refactor
- [ ] Add tests
- [ ] Build / Development Environment
- [ ] Documentation
### SUMMARY
Move Heatmap specific controls from `controls.js` to `Heatmap.js`.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### REVIEWERS
@rusackas @kristw
----------------------------------------------------------------
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 #9360: Migrate
unique Heatmap controls
Posted by GitBox <gi...@apache.org>.
kristw commented on issue #9360: Migrate unique Heatmap controls
URL: https://github.com/apache/incubator-superset/pull/9360#issuecomment-603404030
@villebro I am proposing each chart maintain their own copy of the control config to keep them independent. There are no value added for histogram and heatmap sharing the same `normalized` field when it is not clear if it indicates the same behavior on both charts.
----------------------------------------------------------------
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 #9360: Migrate
unique Heatmap controls
Posted by GitBox <gi...@apache.org>.
rusackas commented on issue #9360: Migrate unique Heatmap controls
URL: https://github.com/apache/incubator-superset/pull/9360#issuecomment-603447485
> @villebro I am proposing each chart maintain their own copy of the control config to keep them independent. There are no value added for histogram and heatmap sharing the same `normalized` field when it is not clear if it indicates the same behavior on both charts.
Following up here to mirror a Slack discussion... we can keep this pass to pulling out the singletons, but in another pass, we can start copying multiple-use controls into individual plugins, and removing the overrides that plugins are layering on. I propose continuing our per-plugin singleton-removal pass, and then doing a per-control pass next to start chewing away at the remaining list.
----------------------------------------------------------------
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 merged pull request #9360: Migrate
unique Heatmap controls
Posted by GitBox <gi...@apache.org>.
villebro merged pull request #9360: Migrate unique Heatmap controls
URL: https://github.com/apache/incubator-superset/pull/9360
----------------------------------------------------------------
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 #9360: Migrate
unique Heatmap controls
Posted by GitBox <gi...@apache.org>.
villebro commented on issue #9360: Migrate unique Heatmap controls
URL: https://github.com/apache/incubator-superset/pull/9360#issuecomment-603118350
@kristw @rusackas There seems to be a bunch of controls that are used in two plugins but aren't really general purpose plugins, e.g. `normalized`, `show_values`, `global_opacity` etc. I propose we move these into the most appropriate "parent" plugin, an import it from there where needed. For example, `normalized` should probably be defined in `Histogram`, and can then be imported and customized in `Heatmap`.
----------------------------------------------------------------
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 a change in pull request
#9360: Migrate unique Heatmap controls
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #9360: Migrate unique Heatmap controls
URL: https://github.com/apache/incubator-superset/pull/9360#discussion_r397003410
##########
File path: superset-frontend/src/explore/controlPanels/Heatmap.js
##########
@@ -18,6 +18,14 @@
*/
import { t } from '@superset-ui/translation';
import { nonEmpty } from '../validators';
+import { formatSelectOptionsForRange } from '../../modules/utils';
+
+const sortAxisChoices = [
+ ['alpha_asc', t('Axis ascending')],
+ ['alpha_desc', t('Axis descending')],
+ ['value_asc', t('Metric ascending')],
+ ['value_desc', t('Metric descending')],
+];
Review comment:
Note the added i18n and `sum(value)` -> `Metric`.
----------------------------------------------------------------
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 #9360: Migrate
unique Heatmap controls
Posted by GitBox <gi...@apache.org>.
villebro commented on issue #9360: Migrate unique Heatmap controls
URL: https://github.com/apache/incubator-superset/pull/9360#issuecomment-603453177
Ok, I'm cool with that, singleton pass it is 👍
----------------------------------------------------------------
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