From eb77a181b6eadf34319f6956d9a1ce168607a961 Mon Sep 17 00:00:00 2001 From: Jun Tan Date: Mon, 20 Aug 2018 16:33:13 -0500 Subject: [PATCH 01/10] fix tooltip bug: columns in aggregate AND used as sort now show in tooltip --- packages/perspective-viewer-highcharts/src/js/config.js | 8 +++----- packages/perspective-viewer-highcharts/src/js/tooltip.js | 9 +++++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/perspective-viewer-highcharts/src/js/config.js b/packages/perspective-viewer-highcharts/src/js/config.js index e28b93730b..47b55c78e3 100644 --- a/packages/perspective-viewer-highcharts/src/js/config.js +++ b/packages/perspective-viewer-highcharts/src/js/config.js @@ -118,7 +118,7 @@ export function default_config(aggregates, mode) { const that = this, config = that._view._config; - const axis_titles = get_axis_titles(config.aggregate, config.sort); + const axis_titles = get_axis_titles(config.aggregate); const pivot_titles = get_pivot_titles(config.row_pivot, config.column_pivot); return { @@ -248,14 +248,12 @@ export function default_config(aggregates, mode) { } } -function get_axis_titles(aggs, sort) { - const sort_titles = sort.map(row => row[0]); +function get_axis_titles(aggs) { let titles = []; for (let i = 0; i < aggs.length; i++) { let axis_title = aggs[i].column; - if(!sort_titles.includes(axis_title)) - titles.push(aggs[i].column); + titles.push(axis_title); } return titles; } diff --git a/packages/perspective-viewer-highcharts/src/js/tooltip.js b/packages/perspective-viewer-highcharts/src/js/tooltip.js index 3e95e88712..105009801e 100644 --- a/packages/perspective-viewer-highcharts/src/js/tooltip.js +++ b/packages/perspective-viewer-highcharts/src/js/tooltip.js @@ -101,10 +101,19 @@ export function format_tooltip(context, type, schema, axis_titles, pivot_titles) function collate_single_value(title, raw_value, schema) { const type = get_axis_type(title, schema); const formatted_value = format_value(raw_value, type); + + /* columns in aggregate AND in sort need to show up, but + * columns not in aggregate but NOT in sort need to hide */ + if (formatted_value === 'NaN' || formatted_value === null || formatted_value === undefined) + return ''; + return `${ title }: ${ formatted_value }
`; } function collate_multiple_values(titles, values) { + if (values.length <= 0) + return ''; + let output = []; for (let i = 0; i < titles.length; i++) { output.push(`${ titles[i] }: ${ values[i] }
`) From f94bdf9074a7c312c8d09f23ca69b02451cd0a10 Mon Sep 17 00:00:00 2001 From: Jun Tan Date: Wed, 22 Aug 2018 16:50:00 -0500 Subject: [PATCH 02/10] issue #200 WIP: working multiple inputs, pre-refactor --- .../src/html/computed_column.html | 9 +- .../perspective-viewer/src/js/computation.js | 3 +- .../src/js/computed_column.js | 243 +++++++++++------- packages/perspective-viewer/src/js/view.js | 16 +- .../src/less/computed_column.less | 55 ++-- .../perspective-viewer/src/less/default.less | 6 + packages/perspective-viewer/src/less/row.less | 2 +- .../perspective-viewer/src/less/view.less | 5 + packages/perspective/src/js/perspective.js | 4 +- 9 files changed, 211 insertions(+), 132 deletions(-) diff --git a/packages/perspective-viewer/src/html/computed_column.html b/packages/perspective-viewer/src/html/computed_column.html index 8cd120a53b..076e8f204c 100644 --- a/packages/perspective-viewer/src/html/computed_column.html +++ b/packages/perspective-viewer/src/html/computed_column.html @@ -14,21 +14,22 @@
-
+
-
+
-
-
+
+ +
diff --git a/packages/perspective-viewer/src/js/computation.js b/packages/perspective-viewer/src/js/computation.js index 980cfa03a8..d1e447c7c3 100644 --- a/packages/perspective-viewer/src/js/computation.js +++ b/packages/perspective-viewer/src/js/computation.js @@ -8,10 +8,11 @@ */ export default class Computation { - constructor(name, input_type, return_type, func) { + constructor(name, input_type, return_type, func, num_params = 1) { this.name = name; this.input_type = input_type; this.return_type = return_type; this.func = func.toString(); + this.num_params = num_params; } } \ No newline at end of file diff --git a/packages/perspective-viewer/src/js/computed_column.js b/packages/perspective-viewer/src/js/computed_column.js index 100fdd05f1..2f4c500efe 100644 --- a/packages/perspective-viewer/src/js/computed_column.js +++ b/packages/perspective-viewer/src/js/computed_column.js @@ -25,60 +25,6 @@ polyfill({}); * */ -// Called on end of drag operation by releasing the mouse -function column_undrag() { - this._input_column.classList.remove('dropping'); -} - -// Called when the column leaves the target -function column_dragleave(event) { - let src = event.relatedTarget; - while (src && src !== this._input_column) { - src = src.parentElement; - } - if (src === null) { - this._input_column.classList.remove('dropping'); - this._drop_target_hover.removeAttribute('drop-target'); - } -} - -// Called when column is held over the target -function column_dragover(event) { - event.preventDefault(); - event.dataTransfer.dropEffect = 'move'; - - this._clear_error_messages(); - - if (event.currentTarget.className !== 'dropping') { - //event.currentTarget.classList.remove('dropped'); - event.currentTarget.classList.add('dropping'); - } - if (!this._drop_target_hover.hasAttribute('drop-target')) { - this._drop_target_hover.setAttribute('drop-target', true); - } - - const input_column = this._input_column; - - if(input_column.children.length === 0) { - // drop_target_hover is the blue box - input_column.parentNode.insertBefore(this._drop_target_hover, input_column.nextSibling); - } -} - -// Called when the column is dropped on the target -function column_drop(event) { - event.preventDefault(); - event.currentTarget.classList.remove('dropping'); - - // column must match return type of computation - const data = JSON.parse(event.dataTransfer.getData('text')); - if (!data) return; - - const column_name = data[0]; - const column_type = data[3]; - - this._set_input_column(event, column_name, column_type); -} // Computations const hour_of_day = function (val) { @@ -117,7 +63,7 @@ class ComputedColumn extends HTMLElement { edit: false, column_name: undefined, computation: undefined, - input_column: undefined, + input_column: [], }; this.computations = { @@ -128,7 +74,9 @@ class ComputedColumn extends HTMLElement { 'day_bucket': new Computation('day_bucket', 'date', 'date', day_bucket), 'uppercase': new Computation('uppercase', 'string', 'string', x => x.toUpperCase()), 'lowercase': new Computation('lowercase', 'string', 'string', x => x.toLowerCase()), - 'length': new Computation('length', 'string', 'integer', x => x.length) + 'length': new Computation('length', 'string', 'integer', x => x.length), + 'add': new Computation('add', 'integer', 'integer', (a, b) => a + b, 2), + 'multiply': new Computation('multiply', 'integer', 'integer', (a, b) => a * b, 2), }; this.type_markers = { @@ -145,6 +93,7 @@ class ComputedColumn extends HTMLElement { this._register_computations(); this._register_callbacks(); this._update_computation(null); + this._register_inputs(); } _register_computations() { @@ -155,7 +104,99 @@ class ComputedColumn extends HTMLElement { iterate = false; } } + + _register_inputs() { + this._input_columns.innerHTML = ''; + const state = this._get_state(); + const computation = state.computation; + const input_type = computation.input_type; + + this._set_state('input_column', []); + + for (let i = 0; i < computation.num_params; i++) { + this._input_columns.innerHTML += + `
+ + Param ${i+1} +
+
`; + } + + for (let column of this._input_columns.children) { + column.addEventListener('drop', this.column_drop.bind(this)); + column.addEventListener('dragend', this.column_undrag.bind(this)); + column.addEventListener('dragover', this.column_dragover.bind(this)); + column.addEventListener('dragleave', this.column_dragleave.bind(this)); + } + + this._clear_column_name(); + } + // utils + + // Called on end of drag operation by releasing the mouse + column_undrag(event) { + event.currentTarget.remove('dropping'); + const state = this._get_state(); + const refresh = this._input_columns.children.length < state.computation.num_params; + if (refresh) + this._register_inputs(); + } + + // Called when the column leaves the target + column_dragleave(event) { + const src = event.currentTarget; + if (src !== null && src.nodeName !== 'SPAN') { + const drop_target_hover = src.querySelector('.psp-cc-computation__drop-target-hover'); + src.classList.remove('dropping'); + if(drop_target_hover) + drop_target_hover.removeAttribute('drop-target'); + } + } + + // Called when column is held over the target + column_dragover(event) { + event.preventDefault(); + event.dataTransfer.dropEffect = 'move'; + + const input_column = event.currentTarget; + const drop_target_hover = input_column.querySelector('.psp-cc-computation__drop-target-hover'); + + this._clear_error_messages(); + + if (input_column.className !== 'dropping') { + //event.currentTarget.classList.remove('dropped'); + input_column.classList.add('dropping'); + } + if (drop_target_hover && !drop_target_hover.hasAttribute('drop-target')) { + drop_target_hover.setAttribute('drop-target', true); + } + + if(input_column.children.length === 2) { + // drop_target_hover is the blue box + input_column.parentNode.insertBefore(drop_target_hover, input_column.nextSibling); + } + } + + // Called when the column is dropped on the target + column_drop(event) { + event.preventDefault(); + event.currentTarget.classList.remove('dropping'); + + // column must match return type of computation + const data = JSON.parse(event.dataTransfer.getData('text')); + if (!data) return; + + const column_name = data[0]; + const column_type = data[3]; + + this._set_input_column(event, column_name, column_type); + } + + // state _get_state() { return this.state; } @@ -179,25 +220,21 @@ class ComputedColumn extends HTMLElement { _clear_state() { this.classList.remove('edit'); this._column_name_input.innerText = ''; - this._input_column.innerHTML = ''; - this._input_column.classList.remove('dropped'); + this._input_columns.innerHTML = ''; + + for (let child of this._input_columns.children) + child.classList.remove('dropped'); + this.state = { edit: false, column_name: undefined, computation: undefined, - input_column: undefined, + input_column: [], name_edited: false }; - this._update_computation(null); - this._clear_error_messages(); - } - _clear_input_column() { - this._input_column.classList.remove('dropped'); - var input_type = this._get_state().computation.input_type; - this._input_column.innerHTML = `Param 1`; - this._set_state('input_column', undefined); - this._auto_name(); + this._update_computation(); + this._clear_error_messages(); } _close_computed_column() { @@ -228,56 +265,81 @@ class ComputedColumn extends HTMLElement { if (state.name_edited) { return; } - if (state.input_column) { - this._column_name_input.innerText = `${state.computation.name}(${state.input_column.name})`; + if (state.input_column.length > 0) { + let names = []; + for (let column of state.input_column) + names.push(column.name); + this._column_name_input.innerText = `${state.computation.name}(${names.join(', ')})`; } else { this._column_name_input.innerText = ''; } this._set_column_name(); } + _clear_column_name() { + const input = this._column_name_input; + input.innerText = ''; + this._set_state('name_edited', false); + this._set_column_name(); + } + _set_input_column(event, name, type) { - const computation = this._get_state().computation; + const state = this._get_state(); + const computation = state.computation; const computation_type = computation.input_type; + const inputs = state.input_column; - this._input_column.innerHTML = ''; - this._set_state('input_column', ''); + const target = event.currentTarget; + const index = Number.parseInt(target.getAttribute('data-index')); - if(type !== computation_type) { - this._clear_input_column(); + if (type !== computation_type) { + this._register_inputs(); this._set_error_message( `Input column type (${type}) must match computation input type (${computation_type}).`, this._input_column_error_message); - this._input_column.classList.remove('dropped'); + target.classList.remove('dropped'); return; } - this._input_column.classList.add('dropped'); + target.classList.add('dropped'); + + const drop_target_hover = target.querySelector('.psp-cc-computation__drop-target-hover'); + if (drop_target_hover) + drop_target_hover.removeAttribute('drop-target'); - this._drop_target_hover.removeAttribute('drop-target'); + target.innerHTML = ''; const input_column = { name: name, type: type, }; - this._set_state('input_column', input_column); + if (inputs[index]) { + inputs[index] = input_column; + } else { + inputs.push(input_column); + } + + this._set_state('input_column', inputs); this._auto_name(); this.dispatchEvent(new CustomEvent('perspective-computed-column-update', { - detail: input_column + detail: { + target, + input_column + } })); } // computation _update_computation(event, computation_name) { const state = this._get_state(); - const has_input_column = state.input_column !== undefined; + //const has_input_column = state.input_column.length > 0; const select = this._computation_selector; if (!computation_name) { computation_name = select[select.selectedIndex].value; - } else if (event === null) { + } else if (event === null || event === undefined) { select.value = computation_name; } @@ -293,11 +355,13 @@ class ComputedColumn extends HTMLElement { this._computation_type.innerHTML = `${this.type_markers[return_type]}`; this._set_state('computation', computation); - this._auto_name(); + this._clear_column_name(); - if((!has_input_column) || (event !== null && state.input_column.type !== input_type)) { - this._clear_input_column(); - } + this._register_inputs(); +/* + if((has_input_column) || (event !== null && state.input_column.type !== input_type)) { + + }*/ this._clear_error_messages(); } @@ -353,8 +417,7 @@ class ComputedColumn extends HTMLElement { this._column_name_input = this.querySelector('#psp-cc-name'); this._computation_selector = this.querySelector('#psp-cc-computation__select'); this._computation_type = this.querySelector('#psp-cc-computation__type'); - this._input_column = this.querySelector('#psp-cc-computation__input-column'); - this._drop_target_hover = this.querySelector('#psp-cc-computation__drop-target-hover'); + this._input_columns = this.querySelector('#psp-cc-computation-inputs'); //this._delete_button = this.querySelector('#psp-cc-button-delete'); this._save_button = this.querySelector('#psp-cc-button-save'); this._input_column_error_message = this.querySelector('#psp-cc__error--input'); @@ -363,10 +426,6 @@ class ComputedColumn extends HTMLElement { _register_callbacks() { this._close_button.addEventListener('click', this._close_computed_column.bind(this)); - this._input_column.addEventListener('drop', column_drop.bind(this)); - this._input_column.addEventListener('dragend', column_undrag.bind(this)); - this._input_column.addEventListener('dragover', column_dragover.bind(this)); - this._input_column.addEventListener('dragleave', column_dragleave.bind(this)); this._computation_selector.addEventListener('change', this._update_computation.bind(this)); this._column_name_input.addEventListener('keyup', event => { this._set_state('name_edited', this._column_name_input.innerText && this._column_name_input.innerText.length > 0); diff --git a/packages/perspective-viewer/src/js/view.js b/packages/perspective-viewer/src/js/view.js index e8bca42c54..cddfe9b52c 100755 --- a/packages/perspective-viewer/src/js/view.js +++ b/packages/perspective-viewer/src/js/view.js @@ -232,8 +232,8 @@ function column_visibility_clicked(ev) { } } else { // check if we're manipulating computed column input - if(ev.path[1].id === 'psp-cc-computation__input-column') { - this._computed_column._clear_input_column(); + if(ev.path[1].classList.contains('psp-cc-computation__input-column')) { + this._computed_column._register_inputs(); this._update_column_view(); return; } @@ -854,8 +854,8 @@ class ViewPrivate extends HTMLElement { } _set_computed_column_input(event) { - this._computed_column_input_column.appendChild( - new_row.call(this, event.detail.name, event.detail.type)); + event.detail.target.appendChild( + new_row.call(this, event.detail.input_column.name, event.detail.input_column.type)); this._update_column_view(); } @@ -868,13 +868,13 @@ class ViewPrivate extends HTMLElement { if(cols.includes(computed_column_name) && !data.edit) { computed_column_name += ` ${(Math.round(Math.random() * 100))}`; } - + const params = [{ computation: data.computation, column: computed_column_name, func: data.computation.func, - inputs: [data.input_column.name], - input_type: data.input_column.type, + inputs: data.input_column.map(col => col.name), + input_type: data.computation.input_type, type: data.computation.return_type, }]; @@ -910,7 +910,7 @@ class ViewPrivate extends HTMLElement { this._side_panel_actions = this.querySelector('#side_panel__actions'); this._add_computed_column = this.querySelector('#add-computed-column'); this._computed_column = this.querySelector('perspective-computed-column'); - this._computed_column_input_column = this._computed_column.querySelector('#psp-cc-computation__input-column'); + this._computed_column_inputs = this._computed_column.querySelector('#psp-cc-computation-inputs'); this._inner_drop_target = this.querySelector('#drop_target_inner'); this._drop_target = this.querySelector('#drop_target'); this._config_button = this.querySelector('#config_button'); diff --git a/packages/perspective-viewer/src/less/computed_column.less b/packages/perspective-viewer/src/less/computed_column.less index e2607ed56d..15291c55e0 100644 --- a/packages/perspective-viewer/src/less/computed_column.less +++ b/packages/perspective-viewer/src/less/computed_column.less @@ -121,13 +121,35 @@ perspective-computed-column { -moz-appearance: none; appearance: none; background: #fff; - border-bottom: 1px solid #ccc; + border-bottom: 1px solid @border-color; font-size: 12px; padding: 3px 4px; flex-basis: 500px; } } + #psp-cc-name { + background-color: #eee; + border-bottom: 1px solid @border-color; + font-size: 12px; + color: #333; + padding-left: 2px; + width: 100%; + + &:empty:before{ + content: "New Column"; + color: @border-color; + } + + * { + display: inline; + } + + br { + display: none; + } + } + .column_row() { font-family: monospace; display: flex; @@ -170,7 +192,11 @@ perspective-computed-column { } } - #psp-cc-computation__input-column { + #psp-cc-computation-inputs { + + } + + .psp-cc-computation__input-column { display: flex; align-items: center; background-color: rgba(255,255,255,0.3); @@ -209,7 +235,7 @@ perspective-computed-column { } } - #psp-cc-computation { + .psp-cc__content { align-items: center; display: flex; margin-top: 5px; @@ -249,34 +275,13 @@ perspective-computed-column { color: #666; } - #psp-cc-name { - background-color: #eee; - border-bottom: none; - font-size: 12px; - color: #333; - padding-left: 2px; - - * { - display: inline; - } - - br { - display: none; - } - } - - #psp-cc-name:empty:before{ - content: "New Column"; - color: #ccc; - } - #psp-cc-computation__drop-target-hover { display: none; } .psp-cc__button { border-radius: 0; - border-bottom: 1px solid #ccc; + border-bottom: 1px solid @border-color; border-left: 0; border-right: 0; border-top: 0; diff --git a/packages/perspective-viewer/src/less/default.less b/packages/perspective-viewer/src/less/default.less index 7039dd35a6..26321982a7 100644 --- a/packages/perspective-viewer/src/less/default.less +++ b/packages/perspective-viewer/src/less/default.less @@ -121,14 +121,20 @@ perspective-viewer { } #app.columns_horizontal { + #side_panel { + max-width: none; + } + #columns_container { flex-direction: row-reverse; } + #active_columns, #inactive_columns { display: flex; flex-direction: column; flex: 1 !important; } + #active_columns { padding-right: 8px; perspective-row { diff --git a/packages/perspective-viewer/src/less/row.less b/packages/perspective-viewer/src/less/row.less index 384fcea5ba..b7ae1221c5 100644 --- a/packages/perspective-viewer/src/less/row.less +++ b/packages/perspective-viewer/src/less/row.less @@ -75,7 +75,7 @@ perspective-row { margin-left: 5px; &:before { - content: "\01F527" + //content: "\01F527" } } diff --git a/packages/perspective-viewer/src/less/view.less b/packages/perspective-viewer/src/less/view.less index e3c769df54..1150a8b505 100644 --- a/packages/perspective-viewer/src/less/view.less +++ b/packages/perspective-viewer/src/less/view.less @@ -78,6 +78,10 @@ perspective-viewer { min-height: 20px; } + #side_panel { + max-width: 30%; + } + #active_columns, #inactive_columns { list-style: none; padding: 5px; @@ -211,6 +215,7 @@ perspective-viewer { align-items: center; justify-content: center; } + #top_panel { display: flex; flex-wrap: wrap; diff --git a/packages/perspective/src/js/perspective.js b/packages/perspective/src/js/perspective.js index c646e3d83e..68ab91265d 100644 --- a/packages/perspective/src/js/perspective.js +++ b/packages/perspective/src/js/perspective.js @@ -901,6 +901,8 @@ table.prototype._calculate_computed = function(tbl, computed_defs) { let inputs = coldef['inputs']; let type = coldef['type'] || 'string'; + console.log(coldef, name, func, inputs, type); + let dtype; switch (type) { case 'integer': @@ -1014,7 +1016,7 @@ table.prototype._computed_schema = function() { const column = {}; column.type = column_type; - column.input_column = computed[i].inputs[0]; // edit to support multiple input columns + column.input_column = computed[i].inputs; // edit to support multiple input columns column.input_type = computed[i].input_type; column.computation = computed[i].computation; From 62b26769a70c9996146d72401b60c68ac3de27ca Mon Sep 17 00:00:00 2001 From: Jun Tan Date: Wed, 22 Aug 2018 17:18:56 -0500 Subject: [PATCH 03/10] #200: refactor to externally defined State class for computed columns --- .../src/js/computed_column.js | 139 +++++++----------- .../Computation.js} | 0 .../src/js/computed_column/State.js | 23 +++ packages/perspective-viewer/src/js/view.js | 18 +-- .../src/less/material.dark.less | 1 + .../perspective-viewer/src/less/material.less | 2 +- 6 files changed, 88 insertions(+), 95 deletions(-) rename packages/perspective-viewer/src/js/{computation.js => computed_column/Computation.js} (100%) create mode 100644 packages/perspective-viewer/src/js/computed_column/State.js diff --git a/packages/perspective-viewer/src/js/computed_column.js b/packages/perspective-viewer/src/js/computed_column.js index 2f4c500efe..6a77968a95 100644 --- a/packages/perspective-viewer/src/js/computed_column.js +++ b/packages/perspective-viewer/src/js/computed_column.js @@ -11,7 +11,8 @@ import { values } from 'underscore'; import {polyfill} from "mobile-drag-drop"; import {bindTemplate} from './utils.js'; -import Computation from './computation.js'; +import State from './computed_column/State.js'; +import Computation from './computed_column/Computation.js'; import template from '../html/computed_column.html'; @@ -59,12 +60,7 @@ class ComputedColumn extends HTMLElement { constructor() { super(); - this.state = { - edit: false, - column_name: undefined, - computation: undefined, - input_column: [], - }; + this.state = new State(); this.computations = { 'hour_of_day': new Computation('hour_of_day', 'date', 'integer', hour_of_day), @@ -107,11 +103,10 @@ class ComputedColumn extends HTMLElement { _register_inputs() { this._input_columns.innerHTML = ''; - const state = this._get_state(); - const computation = state.computation; + const computation = this.state.computation; const input_type = computation.input_type; - this._set_state('input_column', []); + this.state.input_column = []; for (let i = 0; i < computation.num_params; i++) { this._input_columns.innerHTML += @@ -135,13 +130,12 @@ class ComputedColumn extends HTMLElement { this._clear_column_name(); } - // utils + // Drag & Drop // Called on end of drag operation by releasing the mouse column_undrag(event) { event.currentTarget.remove('dropping'); - const state = this._get_state(); - const refresh = this._input_columns.children.length < state.computation.num_params; + const refresh = this._input_columns.children.length < this.state.computation.num_params; if (refresh) this._register_inputs(); } @@ -196,53 +190,18 @@ class ComputedColumn extends HTMLElement { this._set_input_column(event, column_name, column_type); } - // state - _get_state() { - return this.state; - } - - _set_state(key, val) { - this.state[key] = val; - } - - _apply_state() { - const state = this._get_state(); - this._update_computation(null, state.computation.name); - this._set_input_column(null, state.input_column.name, state.input_column.type); - this._column_name_input.innerText = state.column_name; - } + /* _apply_state() { + this._update_computation(null, this.state.computation.name); + this._set_input_column(null, this.state.input_column.name, this.state.input_column.type); + this._column_name_input.innerText = this.state.column_name; + } */ + // State validation _is_valid_state() { - const vals = values(this._get_state()); + const vals = values(this.state); return !vals.includes(null) && !vals.includes(undefined) && !vals.includes(''); } - _clear_state() { - this.classList.remove('edit'); - this._column_name_input.innerText = ''; - this._input_columns.innerHTML = ''; - - for (let child of this._input_columns.children) - child.classList.remove('dropped'); - - this.state = { - edit: false, - column_name: undefined, - computation: undefined, - input_column: [], - name_edited: false - }; - - this._update_computation(); - this._clear_error_messages(); - } - - _close_computed_column() { - this.style.display = 'none'; - this._side_panel_actions.style.display = 'flex'; - this._clear_state(); - } - // error handling _set_error_message(message, target) { target.innerText = message; @@ -256,20 +215,19 @@ class ComputedColumn extends HTMLElement { // column_name _set_column_name() { const input = this._column_name_input; - this._set_state('column_name', input.innerText); + this.state['column_name'] = input.innerText; this._clear_error_messages(); } _auto_name() { - const state = this._get_state(); - if (state.name_edited) { + if (this.state.name_edited) { return; } - if (state.input_column.length > 0) { + if (this.state.input_column.length > 0) { let names = []; - for (let column of state.input_column) + for (let column of this.state.input_column) names.push(column.name); - this._column_name_input.innerText = `${state.computation.name}(${names.join(', ')})`; + this._column_name_input.innerText = `${this.state.computation.name}(${names.join(', ')})`; } else { this._column_name_input.innerText = ''; } @@ -277,17 +235,17 @@ class ComputedColumn extends HTMLElement { } _clear_column_name() { + // TODO: clean up data flow const input = this._column_name_input; input.innerText = ''; - this._set_state('name_edited', false); + this.state['name_edited'] = false; this._set_column_name(); } _set_input_column(event, name, type) { - const state = this._get_state(); - const computation = state.computation; + const computation = this.state.computation; const computation_type = computation.input_type; - const inputs = state.input_column; + const inputs = this.state.input_column; const target = event.currentTarget; const index = Number.parseInt(target.getAttribute('data-index')); @@ -320,7 +278,7 @@ class ComputedColumn extends HTMLElement { inputs.push(input_column); } - this._set_state('input_column', inputs); + this.state['input_column'] = inputs; this._auto_name(); this.dispatchEvent(new CustomEvent('perspective-computed-column-update', { @@ -333,8 +291,6 @@ class ComputedColumn extends HTMLElement { // computation _update_computation(event, computation_name) { - const state = this._get_state(); - //const has_input_column = state.input_column.length > 0; const select = this._computation_selector; if (!computation_name) { @@ -349,38 +305,33 @@ class ComputedColumn extends HTMLElement { throw 'Undefined computation could not be set.'; } - const input_type = computation.input_type; const return_type = computation.return_type; this._computation_type.innerHTML = `${this.type_markers[return_type]}`; - this._set_state('computation', computation); + this.state['computation'] = computation; this._clear_column_name(); this._register_inputs(); -/* - if((has_input_column) || (event !== null && state.input_column.type !== input_type)) { - - }*/ this._clear_error_messages(); } - // edit + /* edit _edit_computed_column(data) { - this._set_state('computation', data.computation.name); - this._set_state('column_name', data.column_name); - this._set_state('input_column', { + this.state['computation'] = data.computation.name) + this.state('column_name', data.column_name); + this.state['input_column'] = { name: data.input_column, type: data.input_type - }); - this._set_state('edit', true); - this._set_state('name_edited', data.column_name !== `${data.computation.name}(${data.input_column})`); + }; + this.state['edit'] = true; + this.state['name_edited'] = data.column_name !== `${data.computation.name}(${data.input_column})`; this._apply_state(); //this.classList.add('edit'); } - // delete - cannot be used without corresponding engine API + delete - cannot be used without corresponding engine API _delete_computed_column() { const state = this._get_state(); if (!state.edit) return; @@ -393,7 +344,7 @@ class ComputedColumn extends HTMLElement { this.dispatchEvent(event); this._clear_state(); - } + } */ // save _save_computed_column() { @@ -402,7 +353,7 @@ class ComputedColumn extends HTMLElement { return; } - const computed_column = this._get_state(); + const computed_column = this.state; const event = new CustomEvent('perspective-computed-column-save', { detail: computed_column @@ -411,6 +362,24 @@ class ComputedColumn extends HTMLElement { this.dispatchEvent(event); } + // close + _close_computed_column() { + this.style.display = 'none'; + this._side_panel_actions.style.display = 'flex'; + + this.classList.remove('edit'); + this._column_name_input.innerText = ''; + this._input_columns.innerHTML = ''; + + for (let child of this._input_columns.children) + child.classList.remove('dropped'); + + this.state = new State(); + + this._update_computation(); + this._clear_error_messages(); + } + _register_ids() { this._side_panel_actions = document.querySelector('#side_panel__actions'); this._close_button = this.querySelector('#psp-cc__close'); @@ -428,7 +397,7 @@ class ComputedColumn extends HTMLElement { this._close_button.addEventListener('click', this._close_computed_column.bind(this)); this._computation_selector.addEventListener('change', this._update_computation.bind(this)); this._column_name_input.addEventListener('keyup', event => { - this._set_state('name_edited', this._column_name_input.innerText && this._column_name_input.innerText.length > 0); + this.state['name_edited'] = this._column_name_input.innerText && this._column_name_input.innerText.length > 0; this._set_column_name(event); }); //this._delete_button.addEventListener('click', this._delete_computed_column.bind(this)); diff --git a/packages/perspective-viewer/src/js/computation.js b/packages/perspective-viewer/src/js/computed_column/Computation.js similarity index 100% rename from packages/perspective-viewer/src/js/computation.js rename to packages/perspective-viewer/src/js/computed_column/Computation.js diff --git a/packages/perspective-viewer/src/js/computed_column/State.js b/packages/perspective-viewer/src/js/computed_column/State.js new file mode 100644 index 0000000000..3725157365 --- /dev/null +++ b/packages/perspective-viewer/src/js/computed_column/State.js @@ -0,0 +1,23 @@ +/****************************************************************************** + * + * Copyright (c) 2017, the Perspective Authors. + * + * This file is part of the Perspective library, distributed under the terms of + * the Apache License 2.0. The full license can be found in the LICENSE file. + * + */ + +export default class State { + constructor() { + return { + errors: { + input_column: undefined, + save: undefined, + }, + column_name: undefined, + computation: undefined, + input_columns: [], + name_edited: false, + } + } +} \ No newline at end of file diff --git a/packages/perspective-viewer/src/js/view.js b/packages/perspective-viewer/src/js/view.js index cddfe9b52c..bcf0f6f3c1 100755 --- a/packages/perspective-viewer/src/js/view.js +++ b/packages/perspective-viewer/src/js/view.js @@ -843,19 +843,22 @@ class ViewPrivate extends HTMLElement { } } + // Computed columns _open_computed_column(event) { - const data = event.detail; + //const data = event.detail; event.stopImmediatePropagation(); - if (event.type === 'perspective-computed-column-edit') { + /*if (event.type === 'perspective-computed-column-edit') { this._computed_column._edit_computed_column(data); - } + }*/ this._computed_column.style.display = 'flex'; this._side_panel_actions.style.display = 'none'; } _set_computed_column_input(event) { event.detail.target.appendChild( - new_row.call(this, event.detail.input_column.name, event.detail.input_column.type)); + new_row.call(this, + event.detail.input_column.name, + event.detail.input_column.type)); this._update_column_view(); } @@ -868,7 +871,7 @@ class ViewPrivate extends HTMLElement { if(cols.includes(computed_column_name) && !data.edit) { computed_column_name += ` ${(Math.round(Math.random() * 100))}`; } - + const params = [{ computation: data.computation, column: computed_column_name, @@ -882,10 +885,7 @@ class ViewPrivate extends HTMLElement { loadTable.call(this, table).then(() => { this._update_column_view(); //this.dispatchEvent(new Event('perspective-view-update')); - - this._computed_column.style.display = 'none'; - this._side_panel_actions.style.display = 'flex'; - this._computed_column._clear_state(); + this._computed_column._close_computed_column(); }); }); } diff --git a/packages/perspective-viewer/src/less/material.dark.less b/packages/perspective-viewer/src/less/material.dark.less index 97a55f92ad..3879d9b69a 100644 --- a/packages/perspective-viewer/src/less/material.dark.less +++ b/packages/perspective-viewer/src/less/material.dark.less @@ -269,6 +269,7 @@ perspective-viewer { } #psp-cc-computation__input-column { + background: none; border-bottom-color: @coolgrey600; &.dropping { diff --git a/packages/perspective-viewer/src/less/material.less b/packages/perspective-viewer/src/less/material.less index 42626c9715..86896227db 100644 --- a/packages/perspective-viewer/src/less/material.less +++ b/packages/perspective-viewer/src/less/material.less @@ -394,7 +394,7 @@ perspective-viewer #app { perspective-computed-column { font-family: @material-sans-serif-fonts !important; - #psp-cc-computation__input-column { + .psp-cc-computation__input-column { background: none; perspective-row { margin-left: -24px; From 66a20271a3bd2f96e250f2b80e383b5b54298a7d Mon Sep 17 00:00:00 2001 From: Jun Tan Date: Fri, 24 Aug 2018 16:37:17 -0500 Subject: [PATCH 04/10] #200: fix broken tests, refactor semantics of input column singular + plural --- .../src/js/computed_column.js | 147 ++++++++++-------- packages/perspective-viewer/src/js/row.js | 2 +- packages/perspective-viewer/src/js/view.js | 8 +- .../test/js/computed_column_tests.js | 47 +++--- .../test/results/results.json | 15 +- packages/perspective/src/js/perspective.js | 17 +- packages/perspective/test/js/constructors.js | 2 +- 7 files changed, 128 insertions(+), 110 deletions(-) diff --git a/packages/perspective-viewer/src/js/computed_column.js b/packages/perspective-viewer/src/js/computed_column.js index 6a77968a95..7056cf0d95 100644 --- a/packages/perspective-viewer/src/js/computed_column.js +++ b/packages/perspective-viewer/src/js/computed_column.js @@ -72,7 +72,12 @@ class ComputedColumn extends HTMLElement { 'lowercase': new Computation('lowercase', 'string', 'string', x => x.toLowerCase()), 'length': new Computation('length', 'string', 'integer', x => x.length), 'add': new Computation('add', 'integer', 'integer', (a, b) => a + b, 2), + 'subtract': new Computation('subtract', 'integer', 'integer', (a, b) => a - b, 2), 'multiply': new Computation('multiply', 'integer', 'integer', (a, b) => a * b, 2), + 'divide': new Computation('divide', 'integer', 'integer', (a, b) => a / b, 2), + 'percent_a_of_b': new Computation('percent_a_of_b', 'integer', 'integer', (a, b) => ((a / b) * 100), 2), + 'concat_space': new Computation('concat_space', 'string', 'string', (a, b) => a + ' ' + b, 2), + 'concat_comma': new Computation('concat_comma', 'string', 'string', (a, b) => a + ', ' + b, 2), }; this.type_markers = { @@ -96,17 +101,19 @@ class ComputedColumn extends HTMLElement { this._computation_selector.innerHTML = ""; let iterate = true; for (let comp of Object.keys(this.computations)) { - this._computation_selector.innerHTML += ``; + this._computation_selector.innerHTML += + ``; iterate = false; } } + // Generate input column holders, reset input column state _register_inputs() { this._input_columns.innerHTML = ''; const computation = this.state.computation; const input_type = computation.input_type; - this.state.input_column = []; + this.state.input_columns = []; for (let i = 0; i < computation.num_params; i++) { this._input_columns.innerHTML += @@ -121,62 +128,40 @@ class ComputedColumn extends HTMLElement { } for (let column of this._input_columns.children) { - column.addEventListener('drop', this.column_drop.bind(this)); - column.addEventListener('dragend', this.column_undrag.bind(this)); - column.addEventListener('dragover', this.column_dragover.bind(this)); - column.addEventListener('dragleave', this.column_dragleave.bind(this)); + column.addEventListener('drop', this.drop_column.bind(this)); + column.addEventListener('dragend', this.remove_column.bind(this)); + column.addEventListener('dragover', this.hover_column.bind(this)); + column.addEventListener('dragleave', this.pass_column.bind(this)); } this._clear_column_name(); } // Drag & Drop - - // Called on end of drag operation by releasing the mouse - column_undrag(event) { - event.currentTarget.remove('dropping'); - const refresh = this._input_columns.children.length < this.state.computation.num_params; - if (refresh) - this._register_inputs(); - } - - // Called when the column leaves the target - column_dragleave(event) { - const src = event.currentTarget; - if (src !== null && src.nodeName !== 'SPAN') { - const drop_target_hover = src.querySelector('.psp-cc-computation__drop-target-hover'); - src.classList.remove('dropping'); - if(drop_target_hover) - drop_target_hover.removeAttribute('drop-target'); - } - } - - // Called when column is held over the target - column_dragover(event) { + hover_column(event) { event.preventDefault(); event.dataTransfer.dropEffect = 'move'; - const input_column = event.currentTarget; - const drop_target_hover = input_column.querySelector('.psp-cc-computation__drop-target-hover'); + const drop_target = event.currentTarget; + const drop_target_hover = drop_target.querySelector('.psp-cc-computation__drop-target-hover'); this._clear_error_messages(); - if (input_column.className !== 'dropping') { + if (drop_target.className !== 'dropping') { //event.currentTarget.classList.remove('dropped'); - input_column.classList.add('dropping'); + drop_target.classList.add('dropping'); } if (drop_target_hover && !drop_target_hover.hasAttribute('drop-target')) { - drop_target_hover.setAttribute('drop-target', true); + drop_target_hover.setAttribute('drop-target', 'true'); } - if(input_column.children.length === 2) { + if(drop_target.children.length === 2) { // drop_target_hover is the blue box - input_column.parentNode.insertBefore(drop_target_hover, input_column.nextSibling); + drop_target.parentNode.insertBefore(drop_target_hover, drop_target.nextSibling); } } - // Called when the column is dropped on the target - column_drop(event) { + drop_column(event) { event.preventDefault(); event.currentTarget.classList.remove('dropping'); @@ -190,11 +175,42 @@ class ComputedColumn extends HTMLElement { this._set_input_column(event, column_name, column_type); } - /* _apply_state() { - this._update_computation(null, this.state.computation.name); - this._set_input_column(null, this.state.input_column.name, this.state.input_column.type); - this._column_name_input.innerText = this.state.column_name; - } */ + // Called when a column is dragged out of the computed column UI + remove_column(event) { + event.currentTarget.remove('dropping'); + const refresh = this._input_columns.children.length < this.state.computation.num_params; + if (refresh) + this._register_inputs(); + } + + // Called when the column passes over and then leaves the drop target + pass_column(event) { + const src = event.currentTarget; + if (src !== null && src.nodeName !== 'SPAN') { + const drop_target_hover = src.querySelector('.psp-cc-computation__drop-target-hover'); + src.classList.remove('dropping'); + if(drop_target_hover) + drop_target_hover.removeAttribute('drop-target'); + } + } + + // When state changes are made manually, apply them to the UI + _apply_state(columns, computation, name) { + this._update_computation(null, computation.name); + this.state['input_columns'] = columns; + const inputs = this._input_columns.children; + + for (let i = 0; i < this.state['input_columns'].length; i++) { + this._set_input_column( + { currentTarget: inputs[i] }, + this.state['input_columns'][i].name, + this.state['input_columns'][i].type); + } + + this._column_name_input.innerText = name; + this._set_column_name(); + this.state['name_edited'] = true; + } // State validation _is_valid_state() { @@ -203,11 +219,15 @@ class ComputedColumn extends HTMLElement { } // error handling - _set_error_message(message, target) { - target.innerText = message; + _set_error_message(type, target) { + target.innerText = this.state.errors[type]; } _clear_error_messages() { + this.state['errors'] = { + input_column: undefined, + save: undefined, + }; this._input_column_error_message.innerText = ''; this._save_error_message.innerText = ''; } @@ -219,13 +239,13 @@ class ComputedColumn extends HTMLElement { this._clear_error_messages(); } - _auto_name() { + _auto_column_name() { if (this.state.name_edited) { return; } - if (this.state.input_column.length > 0) { + if (this.state.input_columns.length > 0) { let names = []; - for (let column of this.state.input_column) + for (let column of this.state.input_columns) names.push(column.name); this._column_name_input.innerText = `${this.state.computation.name}(${names.join(', ')})`; } else { @@ -235,7 +255,6 @@ class ComputedColumn extends HTMLElement { } _clear_column_name() { - // TODO: clean up data flow const input = this._column_name_input; input.innerText = ''; this.state['name_edited'] = false; @@ -245,16 +264,15 @@ class ComputedColumn extends HTMLElement { _set_input_column(event, name, type) { const computation = this.state.computation; const computation_type = computation.input_type; - const inputs = this.state.input_column; + const inputs = this.state.input_columns; const target = event.currentTarget; const index = Number.parseInt(target.getAttribute('data-index')); if (type !== computation_type) { this._register_inputs(); - this._set_error_message( - `Input column type (${type}) must match computation input type (${computation_type}).`, - this._input_column_error_message); + this.state.errors.input_column = `Input column type (${type}) must match computation input type (${computation_type}).`; + this._set_error_message('input_column', this._input_column_error_message); target.classList.remove('dropped'); return; } @@ -267,24 +285,24 @@ class ComputedColumn extends HTMLElement { target.innerHTML = ''; - const input_column = { + const column = { name: name, type: type, }; if (inputs[index]) { - inputs[index] = input_column; + inputs[index] = column; } else { - inputs.push(input_column); + inputs.push(column); } - this.state['input_column'] = inputs; - this._auto_name(); + this.state['input_columns'] = inputs; + this._auto_column_name(); this.dispatchEvent(new CustomEvent('perspective-computed-column-update', { detail: { target, - input_column + column } })); } @@ -310,10 +328,9 @@ class ComputedColumn extends HTMLElement { this._computation_type.innerHTML = `${this.type_markers[return_type]}`; this.state['computation'] = computation; - this._clear_column_name(); + this._clear_column_name(); this._register_inputs(); - this._clear_error_messages(); } @@ -321,12 +338,9 @@ class ComputedColumn extends HTMLElement { _edit_computed_column(data) { this.state['computation'] = data.computation.name) this.state('column_name', data.column_name); - this.state['input_column'] = { - name: data.input_column, - type: data.input_type - }; + this.state['input_columns'] = data.input_columns, this.state['edit'] = true; - this.state['name_edited'] = data.column_name !== `${data.computation.name}(${data.input_column})`; + //this.state['name_edited'] = data.column_name !== `${data.computation.name}(${data.input_column})`; this._apply_state(); //this.classList.add('edit'); } @@ -349,7 +363,8 @@ class ComputedColumn extends HTMLElement { // save _save_computed_column() { if(!this._is_valid_state()) { - this._set_error_message('Missing parameters for computed column.', this._save_error_message); + this.state.errors.save = 'Missing parameters for computed column.'; + this._set_error_message('save', this._save_error_message); return; } diff --git a/packages/perspective-viewer/src/js/row.js b/packages/perspective-viewer/src/js/row.js index 6ab29ee747..d7410442b5 100644 --- a/packages/perspective-viewer/src/js/row.js +++ b/packages/perspective-viewer/src/js/row.js @@ -217,7 +217,7 @@ class Row extends HTMLElement { const data = JSON.parse(this.getAttribute('computed_column')); return { column_name: data.column_name, - input_column: data.input_column, + input_columns: data.input_columns, input_type: data.input_type, computation: data.computation, type: data.type, diff --git a/packages/perspective-viewer/src/js/view.js b/packages/perspective-viewer/src/js/view.js index bcf0f6f3c1..c3242550cb 100755 --- a/packages/perspective-viewer/src/js/view.js +++ b/packages/perspective-viewer/src/js/view.js @@ -319,7 +319,7 @@ function set_aggregate_attribute(aggs) { function _format_computed_data(cc) { return { column_name: cc[0], - input_column: cc[1].input_column, + input_columns: cc[1].input_columns, input_type: cc[1].input_type, computation: cc[1].computation, type: cc[1].type @@ -857,8 +857,8 @@ class ViewPrivate extends HTMLElement { _set_computed_column_input(event) { event.detail.target.appendChild( new_row.call(this, - event.detail.input_column.name, - event.detail.input_column.type)); + event.detail.column.name, + event.detail.column.type)); this._update_column_view(); } @@ -876,7 +876,7 @@ class ViewPrivate extends HTMLElement { computation: data.computation, column: computed_column_name, func: data.computation.func, - inputs: data.input_column.map(col => col.name), + inputs: data.input_columns.map(col => col.name), input_type: data.computation.input_type, type: data.computation.return_type, }]; diff --git a/packages/perspective-viewer/test/js/computed_column_tests.js b/packages/perspective-viewer/test/js/computed_column_tests.js index 672aaa2fce..1b33584f73 100644 --- a/packages/perspective-viewer/test/js/computed_column_tests.js +++ b/packages/perspective-viewer/test/js/computed_column_tests.js @@ -13,15 +13,9 @@ const add_computed_column = async (page) => { await page.evaluate(element => element.setAttribute('columns', '["Row ID","Quantity"]'), viewer); await page.click('#add-computed-column'); await page.$eval('perspective-computed-column', element => { - element._set_state('input_column', { - name: 'Order Date', - type: 'date' - }); - element._set_state('column_name', 'new_cc'); - element._set_state('name_edited', true); - element._apply_state(); + const columns = [{name: 'Order Date', type: 'date'}]; + element._apply_state(columns, element.computations['day_of_week'], 'new_cc'); }); - await page.select('#psp-cc-computation__select', 'day_of_week'); await page.click('#psp-cc-button-save'); await page.waitForSelector('perspective-viewer:not([updating])'); await page.evaluate(element => element.setAttribute('aggregates', '{"new_cc":"dominant"}'), viewer); @@ -58,33 +52,38 @@ exports.default = function() { await page.click('#add-computed-column'); await page.$eval('perspective-computed-column', element => { // call internal APIs to bypass drag/drop action - element._set_state('input_column', { - name: 'Order Date', - type: 'date', - }); - element._set_state('column_name', 'new_cc'); - element._set_state('name_edited', true); - element._apply_state(); + const columns = [{name: 'State', type: 'string'}]; + element._apply_state(columns, element.computations['lowercase'], 'new_cc'); + }); + }); + + test.capture("setting multiple column parameters should set input.", async page => { + await page.click('#config_button'); + const viewer = await page.$('perspective-viewer'); + await page.evaluate(element => element.setAttribute('columns', '["Row ID","Quantity"]'), viewer); + await page.$('perspective-viewer'); + await page.click('#add-computed-column'); + await page.$eval('perspective-computed-column', element => { + const columns = [ + {name: 'Quantity', type: 'integer'}, + {name: 'Row ID', type: 'integer'} + ]; + element._apply_state(columns, element.computations['add'], 'new_cc'); }); }); // computation - test.capture("computations of the same type should not clear input column.", async page => { + test.capture("computations should clear input column.", async page => { await page.click('#config_button'); const viewer = await page.$('perspective-viewer'); await page.evaluate(element => element.setAttribute('columns', '["Row ID","Quantity"]'), viewer); await page.$('perspective-viewer'); await page.click('#add-computed-column'); await page.$eval('perspective-computed-column', element => { - element._set_state('input_column', { - name: 'Order Date', - type: 'date', - }); - element._set_state('column_name', 'new_cc'); - element._set_state('name_edited', true); - element._apply_state(); + const columns = [{name: 'State', type: 'string'}]; + element._apply_state(columns, element.computations['lowercase'], 'new_cc'); }); - await page.select('#psp-cc-computation__select', 'day_of_week'); + await page.select('#psp-cc-computation__select', 'subtract'); }); // save diff --git a/packages/perspective-viewer/test/results/results.json b/packages/perspective-viewer/test/results/results.json index 92eb4d3089..90fbcbf4f9 100644 --- a/packages/perspective-viewer/test/results/results.json +++ b/packages/perspective-viewer/test/results/results.json @@ -10,19 +10,20 @@ "superstore.html/sorts by a hidden column.": "57bd2b3739ab342c71d6878722db2d95", "superstore.html/filters by a numeric column.": "dc1596fb2db82243f2200a0b9d8d35b8", "superstore.html/shows horizontal columns on small viewports.": "a182af50a17d94732c8fa6540fc56233", - "superstore.html/setting a valid column should set it as input.": "7ab0c6a33a4bfa826edf8d2a2f2fe5a2", - "superstore.html/computations of the same type should not clear input column.": "f608917490556d33c44d291960163aa4", - "superstore.html/saving a computed column should add it to inactive columns.": "60e618267929f77003dad9fd9f337577", + "superstore.html/setting a valid column should set it as input.": "7e9051e056a2723f886f7ecd340fd068", + "superstore.html/setting multiple column parameters should set input.": "162a2bd64e307b628c6e7423f55b85dc", + "superstore.html/computations should clear input column.": "894575c747a636c1de8c7616acb63357", + "superstore.html/saving a computed column should add it to inactive columns.": "a831f7659b423d5051af088cf2208e18", "superstore.html/aggregates by computed column.": "f9ba5b75dd267d3dddca65976fbd63f2", "superstore.html/pivots by computed column.": "d4467ad3612e90136fa391196e61189b", - "superstore.html/sorts by computed column.": "7a4c989e68159de9f2ed70a1e6380ef9", - "superstore.html/filters by computed column.": "12d31929677602612fdb26ee04958eef", "superstore.html/computed column aggregates should persist.": "0dbc9d15dc1a9d62d62e94b534447787", + "superstore.html/sorts by computed column.": "4daa6067842eecdddb9f3c06fbbb4326", + "superstore.html/filters by computed column.": "129c3afb5027f2e3454addcc212cd13d", "blank.html/Handles reloading with a schema.": "9cda0b27c92efb59599e11dffbad169e", "superstore.html/pivots by a column.": "71c2f17f6ade6513574aa0143a84c634", - "superstore.html/click on add computed column button opens the UI.": "4dd9778ed1f321d928570d7b3e60a473", + "superstore.html/click on add computed column button opens the UI.": "84bec558c77c5d54a608a135f3e7f9c2", "superstore.html/click on close button closes the UI.": "c10ffcce1cc9d93fdbcaeccfc12c7ca7", - "superstore.html/saving without parameters should show an error message.": "1d22204b783657c99cf03f2e862224ac", + "superstore.html/saving without parameters should show an error message.": "657c4319e6177c16880c38e8cccfaf30", "superstore.html/doesn't leak tables.": "f60686399c38b6154be75b3ebed74c83", "superstore.html/doesn't leak views when setting row pivots.": "9b51f44a534c4d49e4ad3876afacda0c", "superstore.html/doesn't leak views when setting filters.": "e67e550265c4256b52788e2a27d572b0" diff --git a/packages/perspective/src/js/perspective.js b/packages/perspective/src/js/perspective.js index 68ab91265d..6f916bc048 100644 --- a/packages/perspective/src/js/perspective.js +++ b/packages/perspective/src/js/perspective.js @@ -901,8 +901,6 @@ table.prototype._calculate_computed = function(tbl, computed_defs) { let inputs = coldef['inputs']; let type = coldef['type'] || 'string'; - console.log(coldef, name, func, inputs, type); - let dtype; switch (type) { case 'integer': @@ -1016,7 +1014,7 @@ table.prototype._computed_schema = function() { const column = {}; column.type = column_type; - column.input_column = computed[i].inputs; // edit to support multiple input columns + column.input_columns = computed[i].inputs; column.input_type = computed[i].input_type; column.computation = computed[i].computation; @@ -1454,7 +1452,7 @@ table.prototype._column_metadata = function () { if (computed_col !== undefined) { meta.computed = { - input_column: computed_col.input_column, + input_columns: computed_col.input_columns, input_type: computed_col.input_type, computation: computed_col.computation } @@ -1473,9 +1471,14 @@ table.prototype._column_metadata = function () { } /** - * Column metadata for this table. If the column is computed, the `computed` property - * is an Object containing `input_column`, `input_type`, and `computation`. Otherwise, - * `computed` is `undefined`. + * Column metadata for this table. + * + * If the column is computed, the `computed` property is an Object containing: + * - Array `input_columns` + * - String `input_type` + * - Object `computation`. + * + * Otherwise, `computed` is `undefined`. * * @async * diff --git a/packages/perspective/test/js/constructors.js b/packages/perspective/test/js/constructors.js index f692112017..46f1afd9fc 100644 --- a/packages/perspective/test/js/constructors.js +++ b/packages/perspective/test/js/constructors.js @@ -366,7 +366,7 @@ module.exports = (perspective) => { const result = await table2.computed_schema(); const expected = { "plus2": { - input_column: "x", + input_columns: ["x"], input_type: "integer", computation: computation, type: "integer" From 24fd4cf57d54e110395c20c427a787ba50a4ef8e Mon Sep 17 00:00:00 2001 From: Jun Tan Date: Mon, 27 Aug 2018 17:22:02 -0500 Subject: [PATCH 05/10] #200: refactor state to proper class instance, refactor out valid_state method --- .../src/js/computed_column.js | 39 ++----------------- .../src/js/computed_column/State.js | 26 ++++++++----- 2 files changed, 19 insertions(+), 46 deletions(-) diff --git a/packages/perspective-viewer/src/js/computed_column.js b/packages/perspective-viewer/src/js/computed_column.js index 7056cf0d95..05cc3c0dc8 100644 --- a/packages/perspective-viewer/src/js/computed_column.js +++ b/packages/perspective-viewer/src/js/computed_column.js @@ -7,7 +7,6 @@ * */ -import { values } from 'underscore'; import {polyfill} from "mobile-drag-drop"; import {bindTemplate} from './utils.js'; @@ -115,6 +114,8 @@ class ComputedColumn extends HTMLElement { this.state.input_columns = []; + // todo: replace html-loader with handlebars-loader + for (let i = 0; i < computation.num_params; i++) { this._input_columns.innerHTML += `
Date: Tue, 28 Aug 2018 16:54:45 -0500 Subject: [PATCH 06/10] #200: allow for swapping input columns --- .../src/js/computed_column.js | 75 ++++++++++++++----- .../src/js/computed_column/State.js | 1 + packages/perspective-viewer/src/js/view.js | 2 - 3 files changed, 58 insertions(+), 20 deletions(-) diff --git a/packages/perspective-viewer/src/js/computed_column.js b/packages/perspective-viewer/src/js/computed_column.js index 05cc3c0dc8..75ececf115 100644 --- a/packages/perspective-viewer/src/js/computed_column.js +++ b/packages/perspective-viewer/src/js/computed_column.js @@ -113,6 +113,7 @@ class ComputedColumn extends HTMLElement { const input_type = computation.input_type; this.state.input_columns = []; + this.state.swap_target = false; // todo: replace html-loader with handlebars-loader @@ -129,17 +130,36 @@ class ComputedColumn extends HTMLElement { } for (let column of this._input_columns.children) { - column.addEventListener('drop', this.drop_column.bind(this)); - column.addEventListener('dragend', this.remove_column.bind(this)); - column.addEventListener('dragover', this.hover_column.bind(this)); - column.addEventListener('dragleave', this.pass_column.bind(this)); + column.addEventListener('drop', this._drop_column.bind(this)); + column.addEventListener('dragstart', this._drag_column.bind(this)); + column.addEventListener('dragend', this._remove_column.bind(this)); + column.addEventListener('dragover', this._hover_column.bind(this)); + column.addEventListener('dragleave', this._pass_column.bind(this)); } this._clear_column_name(); } // Drag & Drop - hover_column(event) { + _parse_data_transfer(data) { + const column_data = JSON.parse(data); + if (!column_data) return; + + return { + column_name: column_data[0], + column_type: column_data[3], + } + } + + _drag_column(event) { + // called when columns are dragged from inside the UI + if (this.state.computation.num_params > 1) { + // if there is a chance of a swap happening, cache the swap target + this.state.swap_target = event.currentTarget; + } + } + + _hover_column(event) { event.preventDefault(); event.dataTransfer.dropEffect = 'move'; @@ -162,30 +182,43 @@ class ComputedColumn extends HTMLElement { } } - drop_column(event) { + _drop_column(event) { + const target = event.currentTarget; event.preventDefault(); - event.currentTarget.classList.remove('dropping'); + + target.classList.remove('dropping'); + + const is_swap = this.state.swap_target !== undefined && + target.innerHTML.indexOf('perspective-row') > -1; // column must match return type of computation - const data = JSON.parse(event.dataTransfer.getData('text')); + const data = this._parse_data_transfer(event.dataTransfer.getData('text')); if (!data) return; - const column_name = data[0]; - const column_type = data[3]; + if (is_swap) { + const current_column = target.children[0]; + const current_column_name = current_column.getAttribute('name'); + const current_column_type = current_column.getAttribute('type'); + event.swapTarget = this.state.swap_target; + + // take the column at the drop target, and set it to the column being swapped + this._set_input_column(event, current_column_name, current_column_type); - this._set_input_column(event, column_name, column_type); + // reset swap_target and currentTarget + this.state.swap_target = false; + delete event.swapTarget; + } + + this._set_input_column(event, data.column_name, data.column_type); } // Called when a column is dragged out of the computed column UI - remove_column(event) { - event.currentTarget.remove('dropping'); - const refresh = this._input_columns.children.length < this.state.computation.num_params; - if (refresh) - this._register_inputs(); + _remove_column(event) { + event.currentTarget.classList.remove('dropping'); } // Called when the column passes over and then leaves the drop target - pass_column(event) { + _pass_column(event) { const src = event.currentTarget; if (src !== null && src.nodeName !== 'SPAN') { const drop_target_hover = src.querySelector('.psp-cc-computation__drop-target-hover'); @@ -261,7 +294,13 @@ class ComputedColumn extends HTMLElement { const computation_type = computation.input_type; const inputs = this.state.input_columns; - const target = event.currentTarget; + let target; + if (event.swapTarget) { + target = event.swapTarget; + } else { + target = event.currentTarget; + } + const index = Number.parseInt(target.getAttribute('data-index')); if (type !== computation_type) { diff --git a/packages/perspective-viewer/src/js/computed_column/State.js b/packages/perspective-viewer/src/js/computed_column/State.js index 92f677c4ba..d4396b46bf 100644 --- a/packages/perspective-viewer/src/js/computed_column/State.js +++ b/packages/perspective-viewer/src/js/computed_column/State.js @@ -19,6 +19,7 @@ export default class State { this.column_name = undefined; this.computation = undefined; this.input_columns = []; + this.swap_target = false; this.name_edited = false; } diff --git a/packages/perspective-viewer/src/js/view.js b/packages/perspective-viewer/src/js/view.js index c3242550cb..2442dfbb54 100755 --- a/packages/perspective-viewer/src/js/view.js +++ b/packages/perspective-viewer/src/js/view.js @@ -431,7 +431,6 @@ async function loadTable(table) { let aggregate = aggregates .filter(a => a.column === cc_data.column_name) .map(a => a.op)[0]; - console.log(aggregate, cc_data); let row = new_row.call(this, cc_data.column_name, cc_data.type, aggregate, null, null, cc_data); this._inactive_columns.appendChild(row); } @@ -458,7 +457,6 @@ async function loadTable(table) { // fixme better approach please for (let cc of computed_cols) { let cc_data = _format_computed_data(cc); - console.log(aggregates); let aggregate = aggregates .filter(a => a.column === cc_data.column_name) .map(a => a.op)[0]; From 8dc3e460b5c5e1855ec5d0536b306cb8d1923f7c Mon Sep 17 00:00:00 2001 From: Andrew Stein Date: Sun, 2 Sep 2018 20:27:40 -0400 Subject: [PATCH 07/10] Switched default column path separator to '|', to avoid conflict with computed column names --- .gitignore | 1 + .../src/js/series.js | 19 +++++++-------- .../src/js/psp-to-hypergrid.js | 6 +++-- packages/perspective-viewer/src/js/view.js | 10 +------- packages/perspective/src/js/defaults.js | 2 ++ packages/perspective/src/js/perspective.js | 12 +++++----- .../perspective/src/js/view_formatters.js | 2 ++ packages/perspective/test/js/pivots.js | 24 +++++++++---------- 8 files changed, 36 insertions(+), 40 deletions(-) diff --git a/.gitignore b/.gitignore index bcab7c7d91..896bde24da 100644 --- a/.gitignore +++ b/.gitignore @@ -117,3 +117,4 @@ website/i18n/* .idea cmake-build-debug +.ipynb_checkpoints diff --git a/packages/perspective-viewer-highcharts/src/js/series.js b/packages/perspective-viewer-highcharts/src/js/series.js index 9630fd7715..c6ca08bc82 100644 --- a/packages/perspective-viewer-highcharts/src/js/series.js +++ b/packages/perspective-viewer-highcharts/src/js/series.js @@ -7,10 +7,7 @@ * */ -/****************************************************************************** - * - * Y - */ +import {COLUMN_SEPARATOR_STRING} from "@jpmorganchase/perspective/src/js/defaults.js"; function row_to_series(series, sname, gname) { let s; @@ -89,12 +86,12 @@ class ColumnsIterator { for (let row of this.rows) { if (this.columns === undefined) { this.columns = Object.keys(row).filter(prop => { - let cname = prop.split(','); + let cname = prop.split(COLUMN_SEPARATOR_STRING); cname = cname[cname.length - 1]; return prop !== "__ROW_PATH__" && this.hidden.indexOf(cname) === -1; }); this.is_stacked = this.columns.map(value => - value.substr(value.lastIndexOf(',') + 1, value.length) + value.substr(value.lastIndexOf(COLUMN_SEPARATOR_STRING) + 1, value.length) ).filter((value, index, self) => self.indexOf(value) === index ).length > 1; @@ -111,7 +108,7 @@ export function make_y_data(js, pivots, hidden) { for (let row of rows2) { for (let prop of rows2.columns) { - let sname = prop.split(','); + let sname = prop.split(COLUMN_SEPARATOR_STRING); let gname = sname[sname.length - 1]; if (rows2.is_stacked) { sname = sname.join(", ") || gname; @@ -226,12 +223,12 @@ export function make_xy_data(js, schema, columns, pivots, col_pivots, hidden) { } else { let prev, group = [], s; let cols = Object.keys(js[0]).filter(prop => { - let cname = prop.split(','); + let cname = prop.split(COLUMN_SEPARATOR_STRING); cname = cname[cname.length - 1]; return prop !== "__ROW_PATH__" && hidden.indexOf(cname) === -1; }); for (let prop of cols) { - let column_levels = prop.split(','); + let column_levels = prop.split(COLUMN_SEPARATOR_STRING); let group_name = column_levels.slice(0, column_levels.length - 1).join(",") || " "; if (prev === undefined) { prev = group_name; @@ -382,7 +379,7 @@ function make_levels(row_pivots) { function make_configs(series, levels) { let configs = []; for (let data of series) { - let title = data.name.split(','); + let title = data.name.split(COLUMN_SEPARATOR_STRING); configs.push({ layoutAlgorithm: 'squarified', allowDrillToNode: true, @@ -409,7 +406,7 @@ export function make_tree_data(js, row_pivots, hidden, aggregates, leaf_only) { for (let idx = 0; idx < rows2.columns.length; idx++) { let prop = rows2.columns[idx]; - let sname = prop.split(','); + let sname = prop.split(COLUMN_SEPARATOR_STRING); let gname = sname[sname.length - 1]; sname = sname.slice(0, sname.length - 1).join(", ") || " "; if (idx % aggregates.length === 0) { diff --git a/packages/perspective-viewer-hypergrid/src/js/psp-to-hypergrid.js b/packages/perspective-viewer-hypergrid/src/js/psp-to-hypergrid.js index 182bb01ed7..e95edcd915 100644 --- a/packages/perspective-viewer-hypergrid/src/js/psp-to-hypergrid.js +++ b/packages/perspective-viewer-hypergrid/src/js/psp-to-hypergrid.js @@ -7,11 +7,13 @@ * */ +import {COLUMN_SEPARATOR_STRING} from "@jpmorganchase/perspective/src/js/defaults.js"; + const TREE_COLUMN_INDEX = require('fin-hypergrid/src/behaviors/Behavior').prototype.treeColumnIndex; function filter_hidden(hidden, json) { for (let key of Object.keys(json)) { - const split_key = key.split(','); + const split_key = key.split(COLUMN_SEPARATOR_STRING); if (hidden.indexOf(split_key[split_key.length - 1].trim()) >= 0) { delete json[key]; } @@ -36,7 +38,7 @@ function psp2hypergrid(data, hidden, schema, tschema, row_pivots) { var is_tree = !!row_pivots.length; var flat_columns = Object.keys(data).filter(row => row !== '__ROW_PATH__'); - var columnPaths = flat_columns.map(row => row.split(',')); + var columnPaths = flat_columns.map(row => row.split(COLUMN_SEPARATOR_STRING)); let rows = []; diff --git a/packages/perspective-viewer/src/js/view.js b/packages/perspective-viewer/src/js/view.js index 2442dfbb54..4cc93489a4 100755 --- a/packages/perspective-viewer/src/js/view.js +++ b/packages/perspective-viewer/src/js/view.js @@ -51,16 +51,8 @@ function _register_debug_plugin() { global.registerPlugin('debug', { name: "Debug", create: async function(div) { - const json = await this._view.to_json(); + const csv = await this._view.to_csv({config: {delimiter: "|"}}); const timer = this._render_time(); - let csv = ""; - if (json.length > 0) { - let columns = Object.keys(json[0]); - csv += columns.join('|') + '\n'; - for (let row of json) { - csv += Object.values(row).join('|') + "\n"; - } - } div.innerHTML = `
${csv}
`; timer(); }, diff --git a/packages/perspective/src/js/defaults.js b/packages/perspective/src/js/defaults.js index e18d22f200..358a40337f 100644 --- a/packages/perspective/src/js/defaults.js +++ b/packages/perspective/src/js/defaults.js @@ -119,6 +119,8 @@ const DATE_FILTERS = [ "!=" ]; +export const COLUMN_SEPARATOR_STRING = "|"; + export const TYPE_FILTERS = { 'string': STRING_FILTERS, 'float': NUMBER_FILTERS, diff --git a/packages/perspective/src/js/perspective.js b/packages/perspective/src/js/perspective.js index 6f916bc048..6fc058a689 100644 --- a/packages/perspective/src/js/perspective.js +++ b/packages/perspective/src/js/perspective.js @@ -7,7 +7,7 @@ * */ -import {AGGREGATE_DEFAULTS, FILTER_DEFAULTS, SORT_ORDERS, TYPE_AGGREGATES, TYPE_FILTERS} from "./defaults.js"; +import {AGGREGATE_DEFAULTS, FILTER_DEFAULTS, SORT_ORDERS, TYPE_AGGREGATES, TYPE_FILTERS, COLUMN_SEPARATOR_STRING} from "./defaults.js"; import {DateParser, is_valid_date} from "./date_parser.js"; import {Precision} from "@apache-arrow/es5-esm/type"; @@ -25,7 +25,7 @@ if (typeof self !== "undefined" && self.performance === undefined) { self.performance = {now: Date.now}; } -const CHUNKED_THRESHOLD = 100000 +const CHUNKED_THRESHOLD = 100000; module.exports = function (Module) { let __MODULE__ = Module; @@ -484,7 +484,7 @@ view.prototype._column_names = function() { } col_name = col_name.reverse(); col_name.push(name); - col_name = col_name.join(","); + col_name = col_name.join(COLUMN_SEPARATOR_STRING); col_path.delete(); } col_names.push(col_name); @@ -518,7 +518,7 @@ view.prototype.schema = async function() { let new_schema = {}; let col_names = this._column_names(); for (let col_name of col_names) { - col_name = col_name.split(','); + col_name = col_name.split(COLUMN_SEPARATOR_STRING); col_name = col_name[col_name.length - 1]; if (types[col_name] === 1 || types[col_name] === 2) { new_schema[col_name] = "integer"; @@ -548,7 +548,7 @@ const map_aggregate_types = function(col_name, orig_type, aggregate) { for (let agg in aggregate) { let found_agg = aggregate[agg]; - if (found_agg.column.join(',') === col_name) { + if (found_agg.column.join(COLUMN_SEPARATOR_STRING) === col_name) { if (INTEGER_AGGS.includes(found_agg.op)) { return "integer"; } else if (FLOAT_AGGS.includes(found_agg.op)) { @@ -1187,7 +1187,7 @@ table.prototype.view = function(config) { throw `'${agg.op}' has incorrect arity ('${dep_length}') for column dependencies.`; } } - aggregates.push([agg.name || agg.column.join(","), agg_op, agg.column]); + aggregates.push([agg.name || agg.column.join(COLUMN_SEPARATOR_STRING), agg_op, agg.column]); } } else { let agg_op = __MODULE__.t_aggtype.AGGTYPE_DISTINCT_COUNT; diff --git a/packages/perspective/src/js/view_formatters.js b/packages/perspective/src/js/view_formatters.js index caadf8c570..cadb479898 100644 --- a/packages/perspective/src/js/view_formatters.js +++ b/packages/perspective/src/js/view_formatters.js @@ -21,6 +21,8 @@ const jsonFormatter = { }; const csvFormatter = Object.assign({}, jsonFormatter, { + addColumnValue: (data, row, colName, value) => row[colName.split('|').join(',')].unshift(value), + setColumnValue: (data, row, colName, value) => row[colName.split('|').join(',')] = value, formatData: (data, config) => papaparse.unparse(data, config) }); diff --git a/packages/perspective/test/js/pivots.js b/packages/perspective/test/js/pivots.js index 9bf162ab3e..18fabf2912 100644 --- a/packages/perspective/test/js/pivots.js +++ b/packages/perspective/test/js/pivots.js @@ -71,9 +71,9 @@ module.exports = (perspective) => { ], }); var answer = [ - {__ROW_PATH__: [], x: 2.5, "x,y": 2.8333333333333335}, - {__ROW_PATH__: [ false ], x: 3, "x,y": 3.3333333333333335}, - {__ROW_PATH__: [ true ], x: 2, "x,y": 2.3333333333333335}, + {__ROW_PATH__: [], x: 2.5, "x|y": 2.8333333333333335}, + {__ROW_PATH__: [ false ], x: 3, "x|y": 3.3333333333333335}, + {__ROW_PATH__: [ true ], x: 2, "x|y": 2.3333333333333335}, ]; let result = await view.to_json(); expect(answer).toEqual(result); @@ -480,10 +480,10 @@ module.exports = (perspective) => { column_pivot: ['y'] }); var answer = [ - {"a,x":1,"a,y":"a","a,z":true,"b,x":null,"b,y":null,"b,z":null,"c,x":null,"c,y":null,"c,z":null,"d,x":null,"d,y":null,"d,z":null}, - {"a,x":null,"a,y":null,"a,z":null,"b,x":2,"b,y":"b","b,z":false,"c,x":null,"c,y":null,"c,z":null,"d,x":null,"d,y":null,"d,z":null}, - {"a,x":null,"a,y":null,"a,z":null,"b,x":null,"b,y":null,"b,z":null,"c,x":3,"c,y":"c","c,z":true,"d,x":null,"d,y":null,"d,z":null}, - {"a,x":null,"a,y":null,"a,z":null,"b,x":null,"b,y":null,"b,z":null,"c,x":null,"c,y":null,"c,z":null,"d,x":4,"d,y":"d","d,z":false} + {"a|x":1,"a|y":"a","a|z":true,"b|x":null,"b|y":null,"b|z":null,"c|x":null,"c|y":null,"c|z":null,"d|x":null,"d|y":null,"d|z":null}, + {"a|x":null,"a|y":null,"a|z":null,"b|x":2,"b|y":"b","b|z":false,"c|x":null,"c|y":null,"c|z":null,"d|x":null,"d|y":null,"d|z":null}, + {"a|x":null,"a|y":null,"a|z":null,"b|x":null,"b|y":null,"b|z":null,"c|x":3,"c|y":"c","c|z":true,"d|x":null,"d|y":null,"d|z":null}, + {"a|x":null,"a|y":null,"a|z":null,"b|x":null,"b|y":null,"b|z":null,"c|x":null,"c|y":null,"c|z":null,"d|x":4,"d|y":"d","d|z":false} ]; let result2 = await view.to_json(); expect(answer).toEqual(result2); @@ -496,11 +496,11 @@ module.exports = (perspective) => { row_pivot: ['x'] }); var answer = [ - {"__ROW_PATH__":[],"a,x":1,"a,y":1,"a,z":1,"b,x":1,"b,y":1,"b,z":1,"c,x":1,"c,y":1,"c,z":1,"d,x":1,"d,y":1,"d,z":1}, - {"__ROW_PATH__":[1],"a,x":1,"a,y":1,"a,z":1,"b,x":null,"b,y":null,"b,z":null,"c,x":null,"c,y":null,"c,z":null,"d,x":null,"d,y":null,"d,z":null}, - {"__ROW_PATH__":[2],"a,x":null,"a,y":null,"a,z":null,"b,x":1,"b,y":1,"b,z":1,"c,x":null,"c,y":null,"c,z":null,"d,x":null,"d,y":null,"d,z":null}, - {"__ROW_PATH__":[3],"a,x":null,"a,y":null,"a,z":null,"b,x":null,"b,y":null,"b,z":null,"c,x":1,"c,y":1,"c,z":1,"d,x":null,"d,y":null,"d,z":null}, - {"__ROW_PATH__":[4],"a,x":null,"a,y":null,"a,z":null,"b,x":null,"b,y":null,"b,z":null,"c,x":null,"c,y":null,"c,z":null,"d,x":1,"d,y":1,"d,z":1} + {"__ROW_PATH__":[],"a|x":1,"a|y":1,"a|z":1,"b|x":1,"b|y":1,"b|z":1,"c|x":1,"c|y":1,"c|z":1,"d|x":1,"d|y":1,"d|z":1}, + {"__ROW_PATH__":[1],"a|x":1,"a|y":1,"a|z":1,"b|x":null,"b|y":null,"b|z":null,"c|x":null,"c|y":null,"c|z":null,"d|x":null,"d|y":null,"d|z":null}, + {"__ROW_PATH__":[2],"a|x":null,"a|y":null,"a|z":null,"b|x":1,"b|y":1,"b|z":1,"c|x":null,"c|y":null,"c|z":null,"d|x":null,"d|y":null,"d|z":null}, + {"__ROW_PATH__":[3],"a|x":null,"a|y":null,"a|z":null,"b|x":null,"b|y":null,"b|z":null,"c|x":1,"c|y":1,"c|z":1,"d|x":null,"d|y":null,"d|z":null}, + {"__ROW_PATH__":[4],"a|x":null,"a|y":null,"a|z":null,"b|x":null,"b|y":null,"b|z":null,"c|x":null,"c|y":null,"c|z":null,"d|x":1,"d|y":1,"d|z":1} ]; let result2 = await view.to_json(); expect(answer).toEqual(result2); From d2065eb1dfc010ff14056f29765a0db4c4d3b6f9 Mon Sep 17 00:00:00 2001 From: Andrew Stein Date: Sun, 2 Sep 2018 20:32:00 -0400 Subject: [PATCH 08/10] Integer can be promoted to float for computed columns --- .../perspective-viewer/src/js/computed_column.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/perspective-viewer/src/js/computed_column.js b/packages/perspective-viewer/src/js/computed_column.js index 75ececf115..10a91bc6d7 100644 --- a/packages/perspective-viewer/src/js/computed_column.js +++ b/packages/perspective-viewer/src/js/computed_column.js @@ -70,11 +70,11 @@ class ComputedColumn extends HTMLElement { 'uppercase': new Computation('uppercase', 'string', 'string', x => x.toUpperCase()), 'lowercase': new Computation('lowercase', 'string', 'string', x => x.toLowerCase()), 'length': new Computation('length', 'string', 'integer', x => x.length), - 'add': new Computation('add', 'integer', 'integer', (a, b) => a + b, 2), - 'subtract': new Computation('subtract', 'integer', 'integer', (a, b) => a - b, 2), - 'multiply': new Computation('multiply', 'integer', 'integer', (a, b) => a * b, 2), - 'divide': new Computation('divide', 'integer', 'integer', (a, b) => a / b, 2), - 'percent_a_of_b': new Computation('percent_a_of_b', 'integer', 'integer', (a, b) => ((a / b) * 100), 2), + 'add': new Computation('add', 'float', 'float', (a, b) => a + b, 2), + 'subtract': new Computation('subtract', 'float', 'float', (a, b) => a - b, 2), + 'multiply': new Computation('multiply', 'float', 'float', (a, b) => a * b, 2), + 'divide': new Computation('divide', 'float', 'float', (a, b) => a / b, 2), + 'percent_a_of_b': new Computation('percent_a_of_b', 'float', 'float', (a, b) => ((a / b) * 100), 2), 'concat_space': new Computation('concat_space', 'string', 'string', (a, b) => a + ' ' + b, 2), 'concat_comma': new Computation('concat_comma', 'string', 'string', (a, b) => a + ', ' + b, 2), }; @@ -303,7 +303,7 @@ class ComputedColumn extends HTMLElement { const index = Number.parseInt(target.getAttribute('data-index')); - if (type !== computation_type) { + if ((computation_type !== "float" && type !== computation_type) || (computation_type === "float" && type !== "float" && type !== "integer")) { this._register_inputs(); this.state.errors.input_column = `Input column type (${type}) must match computation input type (${computation_type}).`; this._set_error_message('input_column', this._input_column_error_message); From fdedf8a979f5500b3a6cb8c20317fc15a823ea90 Mon Sep 17 00:00:00 2001 From: Andrew Stein Date: Sun, 2 Sep 2018 20:48:42 -0400 Subject: [PATCH 09/10] Don't allow multiple-parameter computed columns with missing columns --- packages/perspective-viewer/src/js/computed_column.js | 4 +++- packages/perspective-viewer/src/js/computed_column/State.js | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/perspective-viewer/src/js/computed_column.js b/packages/perspective-viewer/src/js/computed_column.js index 10a91bc6d7..4ca0aecc7d 100644 --- a/packages/perspective-viewer/src/js/computed_column.js +++ b/packages/perspective-viewer/src/js/computed_column.js @@ -331,7 +331,9 @@ class ComputedColumn extends HTMLElement { } this.state['input_columns'] = inputs; - this._auto_column_name(); + if (inputs.length === computation.num_params) { + this._auto_column_name(); + } this.dispatchEvent(new CustomEvent('perspective-computed-column-update', { detail: { diff --git a/packages/perspective-viewer/src/js/computed_column/State.js b/packages/perspective-viewer/src/js/computed_column/State.js index d4396b46bf..8c249724a9 100644 --- a/packages/perspective-viewer/src/js/computed_column/State.js +++ b/packages/perspective-viewer/src/js/computed_column/State.js @@ -25,6 +25,6 @@ export default class State { is_valid() { const vals = values(this); - return !vals.includes(null) && !vals.includes(undefined) && !vals.includes(''); + return !vals.includes(null) && !vals.includes(undefined) && !vals.includes('') && this.input_columns.length === this.computation.num_params; } } \ No newline at end of file From a4baa33a879336bd46167b683224c07af75046ba Mon Sep 17 00:00:00 2001 From: Andrew Stein Date: Sun, 2 Sep 2018 22:35:00 -0400 Subject: [PATCH 10/10] Fixed computed column input deselect to only remove clicked column --- .../src/js/computed_column.js | 28 ++++++---- packages/perspective-viewer/src/js/view.js | 55 +++++++++++-------- 2 files changed, 47 insertions(+), 36 deletions(-) diff --git a/packages/perspective-viewer/src/js/computed_column.js b/packages/perspective-viewer/src/js/computed_column.js index 4ca0aecc7d..458b2163b1 100644 --- a/packages/perspective-viewer/src/js/computed_column.js +++ b/packages/perspective-viewer/src/js/computed_column.js @@ -212,6 +212,11 @@ class ComputedColumn extends HTMLElement { this._set_input_column(event, data.column_name, data.column_type); } + deselect_column(name) { + this.state.input_columns = this.state.input_columns.map(x => x && x.name === name ? undefined : x); + this._apply_state(this.state.input_columns, this.state.computation); + } + // Called when a column is dragged out of the computed column UI _remove_column(event) { event.currentTarget.classList.remove('dropping'); @@ -235,15 +240,18 @@ class ComputedColumn extends HTMLElement { const inputs = this._input_columns.children; for (let i = 0; i < this.state['input_columns'].length; i++) { - this._set_input_column( - { currentTarget: inputs[i] }, - this.state['input_columns'][i].name, - this.state['input_columns'][i].type); + if (this.state['input_columns'][i] !== undefined) { + this._set_input_column( + {currentTarget: inputs[i]}, + this.state['input_columns'][i].name, + this.state['input_columns'][i].type + ); + } } - this._column_name_input.innerText = name; + this._column_name_input.innerText = name || ""; this._set_column_name(); - this.state['name_edited'] = true; + this.state['name_edited'] = name !== undefined; } // error handling @@ -324,14 +332,10 @@ class ComputedColumn extends HTMLElement { type: type, }; - if (inputs[index]) { - inputs[index] = column; - } else { - inputs.push(column); - } + inputs[index] = column; this.state['input_columns'] = inputs; - if (inputs.length === computation.num_params) { + if (inputs.filter(x => x).length === computation.num_params) { this._auto_column_name(); } diff --git a/packages/perspective-viewer/src/js/view.js b/packages/perspective-viewer/src/js/view.js index 4cc93489a4..78f9c8e387 100755 --- a/packages/perspective-viewer/src/js/view.js +++ b/packages/perspective-viewer/src/js/view.js @@ -225,7 +225,8 @@ function column_visibility_clicked(ev) { } else { // check if we're manipulating computed column input if(ev.path[1].classList.contains('psp-cc-computation__input-column')) { - this._computed_column._register_inputs(); + // this._computed_column._register_inputs(); + this._computed_column.deselect_column(ev.currentTarget.getAttribute('name')); this._update_column_view(); return; } @@ -318,7 +319,7 @@ function _format_computed_data(cc) { }; } -async function loadTable(table) { +async function loadTable(table, redraw = true) { this.querySelector('#app').classList.add('hide_message'); this.setAttribute('updating', true); @@ -475,8 +476,7 @@ async function loadTable(table) { this.querySelector('#side_panel__actions').style.visibility = "visible"; this.filters = this.getAttribute('filters'); - - this._debounce_update(); + this._debounce_update(redraw); } function new_row(name, type, aggregate, filter, sort, computed) { @@ -576,7 +576,7 @@ class CancelTask { } -function update() { +function update(redraw = true) { if (!this._table) return; let row_pivots = this._view_columns('#row_pivots perspective-row'); let column_pivots = this._view_columns('#column_pivots perspective-row'); @@ -629,27 +629,34 @@ function update() { }); const timer = this._render_time(); - this._render_count = (this._render_count || 0) + 1; - if (this._task) { - this._task.cancel(); - } - const task = this._task = new CancelTask(() => { - this._render_count--; - }); - task.initial = true; - - this._plugin.create.call(this, this._datavis, this._view, task).catch(err => { - console.warn(err); - }).finally(() => { - if (!this.hasAttribute('render_time')) { - this.dispatchEvent(new Event('perspective-view-update')); + if (redraw) { + this._render_count = (this._render_count || 0) + 1; + if (this._task) { + this._task.cancel(); } + const task = this._task = new CancelTask(() => { + this._render_count--; + }); + task.initial = true; + + this._plugin.create.call(this, this._datavis, this._view, task).catch(err => { + console.warn(err); + }).finally(() => { + if (!this.hasAttribute('render_time')) { + this.dispatchEvent(new Event('perspective-view-update')); + } + timer(); + task.cancel(); + if (this._render_count === 0) { + this.removeAttribute('updating'); + } + }); + } else { timer(); - task.cancel(); if (this._render_count === 0) { this.removeAttribute('updating'); } - }); + } } /****************************************************************************** @@ -872,7 +879,7 @@ class ViewPrivate extends HTMLElement { }]; const table = this._table.add_computed(params); - loadTable.call(this, table).then(() => { + loadTable.call(this, table, false).then(() => { this._update_column_view(); //this.dispatchEvent(new Event('perspective-view-update')); this._computed_column._close_computed_column(); @@ -959,9 +966,9 @@ class ViewPrivate extends HTMLElement { _register_debounce_instance() { const _update = _.debounce(update.bind(this), 10); - this._debounce_update = () => { + this._debounce_update = (redraw) => { this.setAttribute('updating', true); - _update(); + _update(redraw); } } }