Skip to content

Cal heatmap controls migration #9355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rusackas merged 2 commits into from
Mar 24, 2020
Merged

Cal heatmap controls migration #9355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e376e7a
controls migrated
rusackas Mar 23, 2020
5c012d6
linting
rusackas Mar 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1055,7 +1055,6 @@ The filter-box configuration references column names (via the `column` key) and
| `x_axis_format` | _string_ | The **X Axis Format** widget |
| `x_axis_label` | _string_ | The **X Axis Label** widget |
| `x_axis_showminmax` | _boolean_ | The **X bounds** widget |
| `x_axis_time_format` | _N/A_ | _Deprecated?_ |
| `x_log_scale` | _N/A_ | _Deprecated?_ |
| `x_ticks_layout` | _string_ | The **X Tick Layout** widget |

Expand Down Expand Up @@ -1095,9 +1094,6 @@ Note the `y_axis_format` is defined under various section for some charts.
| `bar_stacked` | _N/A_ | |
| `cache_timeout` | _N/A_ | |
| `canvas_image_rendering` | _N/A_ | |
| `cell_padding` | _N/A_ | |
| `cell_radius` | _N/A_ | |
| `cell_size` | _N/A_ | |
| `charge` | _N/A_ | |
| `clustering_radius` | _N/A_ | |
| `code` | _N/A_ | |
Expand All @@ -1112,7 +1108,6 @@ Note the `y_axis_format` is defined under various section for some charts.
| `deck_slices` | _N/A_ | |
| `default_filters` | _N/A_ | |
| `dimension` | _N/A_ | |
| `domain_granularity` | _N/A_ | |
| `end_spatial` | _N/A_ | |
| `entity` | _N/A_ | |
| `equal_date_size` | _N/A_ | |
Expand Down Expand Up @@ -1211,7 +1206,6 @@ Note the `y_axis_format` is defined under various section for some charts.
| `show_druid_time_granularity` | _N/A_ | |
| `show_druid_time_origin` | _N/A_ | |
| `show_labels` | _N/A_ | |
| `show_metric_name` | _N/A_ | |
| `show_perc` | _N/A_ | |
| `show_sqla_time_column` | _N/A_ | |
| `show_sqla_time_granularity` | _N/A_ | |
Expand All @@ -1224,11 +1218,9 @@ Note the `y_axis_format` is defined under various section for some charts.
| `spatial` | _N/A_ | |
| `stacked_style` | _N/A_ | |
| `start_spatial` | _N/A_ | |
| `steps` | _N/A_ | |
| `stroke_color_picker` | _N/A_ | |
| `stroke_width` | _N/A_ | |
| `stroked` | _N/A_ | |
| `subdomain_granularity` | _N/A_ | |
| `subheader` | _N/A_ | |
| `table_filter` | _N/A_ | |
| `table_timestamp_format` | _N/A_ | |
Expand Down
132 changes: 124 additions & 8 deletions superset-frontend/src/explore/controlPanels/CalHeatmap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@
* under the License.
*/
import { t } from '@superset-ui/translation';
import {
// formatSelectOptionsForRange,
formatSelectOptions,
// mainMetric,
} from '../../modules/utils';
import * as v from '.././validators';
import { D3_TIME_FORMAT_OPTIONS, D3_FORMAT_DOCS } from '../controls';

export default {
requiresTime: true,
Expand All @@ -25,7 +32,43 @@ export default {
label: t('Query'),
expanded: true,
controlSetRows: [
['domain_granularity', 'subdomain_granularity'],
[
{
name: 'domain_granularity',
config: {
type: 'SelectControl',
label: t('Domain'),
default: 'month',
choices: formatSelectOptions([
'hour',
'day',
'week',
'month',
'year',
]),
description: t('The time unit used for the grouping of blocks'),
},
},
{
name: 'subdomain_granularity',
config: {
type: 'SelectControl',
label: t('Subdomain'),
default: 'day',
choices: formatSelectOptions([
'min',
'hour',
'day',
'week',
'month',
]),
description: t(
'The time unit for each block. Should be a smaller unit than ' +
'domain_granularity. Should be larger or equal to Time Grain',
),
},
},
],
['metrics'],
['adhoc_filters'],
],
Expand All @@ -35,21 +78,94 @@ export default {
expanded: true,
controlSetRows: [
['linear_color_scheme'],
['cell_size', 'cell_padding'],
['cell_radius', 'steps'],
['y_axis_format', 'x_axis_time_format'],
[
{
name: 'cell_size',
config: {
type: 'TextControl',
isInt: true,
default: 10,
validators: [v.integer],
renderTrigger: true,
label: t('Cell Size'),
description: t('The size of the square cell, in pixels'),
},
},
{
name: 'cell_padding',
config: {
type: 'TextControl',
isInt: true,
validators: [v.integer],
renderTrigger: true,
default: 2,
label: t('Cell Padding'),
description: t('The distance between cells, in pixels'),
},
},
],
[
{
name: 'cell_radius',
config: {
type: 'TextControl',
isInt: true,
validators: [v.integer],
renderTrigger: true,
default: 0,
label: t('Cell Radius'),
description: t('The pixel radius'),
},
},
{
name: 'steps',
config: {
type: 'TextControl',
isInt: true,
validators: [v.integer],
renderTrigger: true,
default: 10,
label: t('Color Steps'),
description: t('The number color "steps"'),
},
},
],
[
'y_axis_format',
{
name: 'x_axis_time_format',
Copy link
Member Author

@rusackas rusackas Mar 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kristw it appears I can change this one to use x_axis_format with a couple config overrides. This would get rid of x_axis_time_format entirely, which appears to be a deprecated practice.

If that's a good idea, I can make the change here, and a quick PR in superset-ui-plugins to match.

However, I'm not sure if this leads to any painful migrations for existing charts/dashboards. Thoughts?

Copy link
Contributor

@kristw kristw Mar 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Umm, as much as I wanna merge them, this will cause db:migration for existing charts/dashboards so perhaps let's hold on to it.

Copy link
Member Author

@rusackas rusackas Mar 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok... I'll try to keep track of that, as it may be worth addressing this and other similar migrations... maybe we can get a handful of them in a batch. Heads up to @villebro or @willbarrett that this may come up in discussion.

Copy link
Member

@villebro villebro Mar 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rusackas I like the idea of doing a migration batch to clean up duplicate controls. Let's hold off on the changes for now and put them in a google sheet or similar so we can do one or two migrations when the control migration is otherwise done.

config: {
type: 'SelectControl',
freeForm: true,
label: t('Time Format'),
renderTrigger: true,
default: 'smart_date',
choices: D3_TIME_FORMAT_OPTIONS,
description: D3_FORMAT_DOCS,
},
},
],
['show_legend', 'show_values'],
['show_metric_name', null],
[
{
name: 'show_metric_name',
config: {
type: 'CheckboxControl',
label: t('Show Metric Names'),
renderTrigger: true,
default: true,
description: t('Whether to display the metric name as a title'),
},
},
null,
],
],
},
],
controlOverrides: {
y_axis_format: {
label: t('Number Format'),
},
x_axis_time_format: {
label: t('Time Format'),
},
show_values: {
default: false,
},
Expand Down
82 changes: 3 additions & 79 deletions superset-frontend/src/explore/controls.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,6 @@ const sequentialSchemeRegistry = getSequentialSchemeRegistry();

const PRIMARY_COLOR = { r: 0, g: 122, b: 135, a: 1 };

const D3_FORMAT_DOCS = 'D3 format syntax: https://github.com/d3/d3-format';

// input choices & options
const D3_FORMAT_OPTIONS = [
['SMART_NUMBER', 'Adaptative formating'],
Expand All @@ -100,6 +98,9 @@ const ROW_LIMIT_OPTIONS = [10, 50, 100, 250, 500, 1000, 5000, 10000, 50000];

const SERIES_LIMITS = [0, 5, 10, 25, 50, 100, 500];

export const D3_FORMAT_DOCS =
'D3 format syntax: https://github.com/d3/d3-format';

export const D3_TIME_FORMAT_OPTIONS = [
['smart_date', 'Adaptative formating'],
['%d/%m/%Y', '%d/%m/%Y | 14/01/2019'],
Expand Down Expand Up @@ -894,25 +895,6 @@ export const controls = {
),
},

domain_granularity: {
type: 'SelectControl',
label: t('Domain'),
default: 'month',
choices: formatSelectOptions(['hour', 'day', 'week', 'month', 'year']),
description: t('The time unit used for the grouping of blocks'),
},

subdomain_granularity: {
type: 'SelectControl',
label: t('Subdomain'),
default: 'day',
choices: formatSelectOptions(['min', 'hour', 'day', 'week', 'month']),
description: t(
'The time unit for each block. Should be a smaller unit than ' +
'domain_granularity. Should be larger or equal to Time Grain',
),
},

link_length: {
type: 'SelectControl',
renderTrigger: true,
Expand Down Expand Up @@ -1155,46 +1137,6 @@ export const controls = {
),
},

cell_size: {
type: 'TextControl',
isInt: true,
default: 10,
validators: [v.integer],
renderTrigger: true,
label: t('Cell Size'),
description: t('The size of the square cell, in pixels'),
},

cell_padding: {
type: 'TextControl',
isInt: true,
validators: [v.integer],
renderTrigger: true,
default: 2,
label: t('Cell Padding'),
description: t('The distance between cells, in pixels'),
},

cell_radius: {
type: 'TextControl',
isInt: true,
validators: [v.integer],
renderTrigger: true,
default: 0,
label: t('Cell Radius'),
description: t('The pixel radius'),
},

steps: {
type: 'TextControl',
isInt: true,
validators: [v.integer],
renderTrigger: true,
default: 10,
label: t('Color Steps'),
description: t('The number color "steps"'),
},

grid_size: {
type: 'TextControl',
label: t('Grid Size'),
Expand Down Expand Up @@ -1348,16 +1290,6 @@ export const controls = {
description: D3_FORMAT_DOCS,
},

x_axis_time_format: {
type: 'SelectControl',
freeForm: true,
label: t('X Axis Format'),
renderTrigger: true,
default: 'smart_date',
choices: D3_TIME_FORMAT_OPTIONS,
description: D3_FORMAT_DOCS,
},

y_axis_format: {
type: 'SelectControl',
freeForm: true,
Expand Down Expand Up @@ -1724,14 +1656,6 @@ export const controls = {
description: t('Whether to display the numerical values within the cells'),
},

show_metric_name: {
type: 'CheckboxControl',
label: t('Show Metric Names'),
renderTrigger: true,
default: true,
description: t('Whether to display the metric name as a title'),
},

show_trend_line: {
type: 'CheckboxControl',
label: t('Show Trend Line'),
Expand Down