Refresh query when parameters update (#3737)

* Add touch state to parameters and autoupdate query * Use values change event instead of $watch * Remove getQueryResultDebounced * Add Apply button * Remove Input Number spinners for Parameters * Make Apply Button optional * Update share_embed_spec * Change debounce to the Parameters component * Remove unnecessary click on Execute query * Add apply button to the remaining places * Update dashboard_spec * Use onKeyUp for InputNumber * Simplify onParametersValuesChanged * Update DateTime onChange function * Don't apply when modifier key is pressed * Remove refresh Button from Parameters * Update apply button styling * Update apply right distance * Remove debounce for testing * Use data-dirty instead of classNames for styling * Make sure $apply runs before calling onChange
getredash · May 15, 2019 · c76955b · c76955b
1 parent 4f40237
commit c76955b
Show file tree

Hide file tree

Showing 13 changed files with 172 additions and 51 deletions.
diff --git a/client/app/components/DateTimeInput.jsx b/client/app/components/DateTimeInput.jsx
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { useState } from 'react';
 import PropTypes from 'prop-types';
 import { react2angular } from 'react2angular';
 import DatePicker from 'antd/lib/date-picker';
@@ -13,18 +13,26 @@ export function DateTimeInput({
 }) {
   const format = (clientConfig.dateFormat || 'YYYY-MM-DD') +
     (withSeconds ? ' HH:mm:ss' : ' HH:mm');
-  const additionalAttributes = {};
+  let defaultValue;
   if (value && value.isValid()) {
-    additionalAttributes.defaultValue = value;
+    defaultValue = value;
   }
+  const [currentValue, setCurrentValue] = useState(defaultValue);
   return (
     <DatePicker
       className={className}
       showTime
-      {...additionalAttributes}
+      value={currentValue}
       format={format}
       placeholder="Select Date and Time"
-      onChange={onSelect}
+      onChange={newValue => setCurrentValue(newValue)}
+      onOpenChange={(status) => {
+        if (!status) { // on close picker
+          if (currentValue && currentValue.isValid()) {
+            onSelect(currentValue);
+          }
+        }
+      }}
     />
   );
 }

diff --git a/client/app/components/DateTimeRangeInput.jsx b/client/app/components/DateTimeRangeInput.jsx
@@ -1,5 +1,5 @@
 import { isArray } from 'lodash';
-import React from 'react';
+import React, { useState } from 'react';
 import PropTypes from 'prop-types';
 import { react2angular } from 'react2angular';
 import DatePicker from 'antd/lib/date-picker';
@@ -16,17 +16,25 @@ export function DateTimeRangeInput({
 }) {
   const format = (clientConfig.dateFormat || 'YYYY-MM-DD') +
     (withSeconds ? ' HH:mm:ss' : ' HH:mm');
-  const additionalAttributes = {};
+  let defaultValue;
   if (isArray(value) && value[0].isValid() && value[1].isValid()) {
-    additionalAttributes.defaultValue = value;
+    defaultValue = value;
   }
+  const [currentValue, setCurrentValue] = useState(defaultValue);
   return (
     <RangePicker
       className={className}
       showTime
-      {...additionalAttributes}
+      value={currentValue}
       format={format}
-      onChange={onSelect}
+      onChange={newValue => setCurrentValue(newValue)}
+      onOpenChange={(status) => {
+        if (!status) { // on close picker
+          if (isArray(currentValue) && currentValue[0].isValid() && currentValue[1].isValid()) {
+            onSelect(currentValue);
+          }
+        }
+      }}
     />
   );
 }

diff --git a/client/app/components/ParameterValueInput.jsx b/client/app/components/ParameterValueInput.jsx
@@ -1,15 +1,19 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import { react2angular } from 'react2angular';
+import Button from 'antd/lib/button';
 import Select from 'antd/lib/select';
 import Input from 'antd/lib/input';
 import InputNumber from 'antd/lib/input-number';
+import { isFunction } from 'lodash';
 import { DateInput } from './DateInput';
 import { DateRangeInput } from './DateRangeInput';
 import { DateTimeInput } from './DateTimeInput';
 import { DateTimeRangeInput } from './DateTimeRangeInput';
 import { QueryBasedParameterInput } from './QueryBasedParameterInput';
 
+import './ParameterValueInput.less';
+
 const { Option } = Select;
 
 export class ParameterValueInput extends React.Component {
@@ -19,6 +23,7 @@ export class ParameterValueInput extends React.Component {
     enumOptions: PropTypes.string,
     queryId: PropTypes.number,
     parameter: PropTypes.any, // eslint-disable-line react/forbid-prop-types
+    applyButton: PropTypes.bool,
     onSelect: PropTypes.func,
     className: PropTypes.string,
   };
@@ -29,10 +34,31 @@ export class ParameterValueInput extends React.Component {
     enumOptions: '',
     queryId: null,
     parameter: null,
+    applyButton: false,
     onSelect: () => {},
     className: '',
   };
 
+  constructor(props) {
+    super(props);
+    this.state = { value: props.value };
+  }
+
+  renderApplyButton() {
+    const { onSelect } = this.props;
+    const { value } = this.state;
+    return (
+      <Button
+        className="parameter-apply-button"
+        type="primary"
+        size="small"
+        onClick={() => onSelect(value)}
+      >
+        Apply
+      </Button>
+    );
+  }
+
   renderDateTimeWithSecondsInput() {
     const { value, onSelect } = this.props;
     return (
@@ -132,25 +158,62 @@ export class ParameterValueInput extends React.Component {
   }
 
   renderNumberInput() {
-    const { value, onSelect, className } = this.props;
+    const { className, onSelect, applyButton } = this.props;
+    const { value } = this.state;
+    const showApplyButton = applyButton && value !== this.props.value;
+
+    const onChange = (newValue) => {
+      this.setState({ value: newValue });
+      if (!applyButton) {
+        onSelect(newValue);
+      }
+    };
+
     return (
-      <InputNumber
-        className={'form-control ' + className}
-        defaultValue={!isNaN(value) && value || 0}
-        onChange={onSelect}
-      />
+      <div className="parameter-input-number" data-dirty={showApplyButton || null}>
+        <InputNumber
+          className={className}
+          value={!isNaN(value) && value || 0}
+          onChange={onChange}
+          onKeyUp={showApplyButton ? (e) => {
+            const keyNumber = e.which || e.keyCode;
+            if (keyNumber === 13 && !e.ctrlKey && !e.metaKey) { // enter key
+              onSelect(value);
+            }
+          } : null}
+        />
+        {showApplyButton && this.renderApplyButton()}
+      </div>
     );
   }
 
   renderTextInput() {
-    const { value, onSelect, className } = this.props;
+    const { className, onSelect, applyButton } = this.props;
+    const { value } = this.state;
+    const showApplyButton = applyButton && value !== this.props.value;
+
+    const onChange = (event) => {
+      this.setState({ value: event.target.value });
+      if (!applyButton) {
+        onSelect(event.target.value);
+      }
+    };
+
     return (
-      <Input
-        className={'form-control ' + className}
-        defaultValue={value || ''}
-        data-test="TextParamInput"
-        onChange={event => onSelect(event.target.value)}
-      />
+      <div className="parameter-input" data-dirty={showApplyButton || null}>
+        <Input
+          className={className}
+          value={value || ''}
+          data-test="TextParamInput"
+          onChange={onChange}
+          onPressEnter={showApplyButton ? (e) => {
+            if (!e.ctrlKey && !e.metaKey) {
+              onSelect(value);
+            }
+          } : null}
+        />
+        {showApplyButton && this.renderApplyButton()}
+      </div>
     );
   }
 
@@ -181,15 +244,21 @@ export default function init(ngModule) {
         enum-options="$ctrl.param.enumOptions"
         query-id="$ctrl.param.queryId"
         on-select="$ctrl.setValue"
+        apply-button="$ctrl.applyButton"
       ></parameter-value-input-impl>
     `,
     bindings: {
       param: '<',
+      applyButton: '=?',
+      onChange: '=',
     },
     controller($scope) {
       this.setValue = (value) => {
         this.param.setValue(value);
-        $scope.$applyAsync();
+        $scope.$apply();
+        if (isFunction(this.onChange)) {
+          this.onChange();
+        }
       };
     },
   });

diff --git a/client/app/components/ParameterValueInput.less b/client/app/components/ParameterValueInput.less
@@ -0,0 +1,50 @@
+@import '~antd/lib/input-number/style/index'; // for ant @vars
+
+.parameter-input {
+  display: inline-block;
+  position: relative;
+  width: 195px;
+
+  &[data-dirty] {
+    .@{ant-prefix}-input {
+      padding-right: 58px !important;
+    }
+  }
+
+  .parameter-apply-button {
+    position: absolute !important;
+    height: 25px !important;
+    width: 50px;
+    top: 5px;
+    right: 5px;
+  }
+}
+
+.parameter-input-number {
+  display: inline-block;
+  position: relative;
+  width: 195px;
+
+  .@{ant-prefix}-input-number {
+    width: 100% !important;
+  }
+
+  &[data-dirty] {
+    .@{ant-prefix}-input-number {
+      padding-right: 68px !important;
+    }
+  }
+
+  // Make Input Number spinners always visible
+  .@{input-number-prefix-cls}-handler-wrap {
+    opacity: 1 !important;
+  }
+
+  .parameter-apply-button {
+    position: absolute !important;
+    height: 25px !important;
+    width: 50px;
+    top: 5px;
+    right: 27px;
+  }
+}
diff --git a/client/app/components/dashboards/widget.html b/client/app/components/dashboards/widget.html
@@ -38,7 +38,7 @@
         </div>
       </div>
       <div class="m-b-10" ng-if="$ctrl.localParametersDefs().length > 0">
-        <parameters parameters="$ctrl.localParametersDefs()"></parameters>
+        <parameters parameters="$ctrl.localParametersDefs()" on-values-change="$ctrl.refresh"></parameters>
       </div>
     </div>
 

diff --git a/client/app/components/parameters.html b/client/app/components/parameters.html
@@ -18,14 +18,6 @@
     >
       <i class="zmdi zmdi-settings"></i>
     </button>
-    <parameter-value-input param="param"></parameter-value-input>
+    <parameter-value-input param="param" on-change="onValuesChange" apply-button="true"></parameter-value-input>
   </div>
-  <button
-    class="m-t-20 btn btn-primary hidden-print"
-    ng-if="onRefresh"
-    ng-click="onRefresh()"
-    title="Refresh Dataset"
-  >
-    <span class="zmdi zmdi-play"></span>
-  </button>
 </div>
diff --git a/client/app/components/parameters.js b/client/app/components/parameters.js
@@ -12,7 +12,7 @@ function ParametersDirective($location) {
       editable: '=?',
       changed: '&onChange',
       onUpdated: '=',
-      onRefresh: '=',
+      onValuesChange: '=',
     },
     template,
     link(scope) {

diff --git a/client/app/components/queries/visualization-embed.html b/client/app/components/queries/visualization-embed.html
@@ -13,7 +13,7 @@ <h3>
 
   <div class="col-md-12 query__vis">
     <div class="p-t-15 p-b-5" ng-if="$ctrl.query.hasParameters()">
-      <parameters parameters="$ctrl.query.getParametersDefs()" on-refresh="$ctrl.refreshQueryResults"></parameters>
+      <parameters parameters="$ctrl.query.getParametersDefs()" on-values-change="$ctrl.refreshQueryResults"></parameters>
     </div>
 
     <div ng-if="$ctrl.error">

diff --git a/client/app/pages/dashboards/dashboard.html b/client/app/pages/dashboards/dashboard.html
@@ -94,7 +94,7 @@ <h3>
   </div>
 
   <div class="m-b-10 p-15 bg-white tiled" ng-if="$ctrl.globalParameters.length > 0">
-    <parameters parameters="$ctrl.globalParameters"></parameters>
+    <parameters parameters="$ctrl.globalParameters" on-values-change="$ctrl.refreshDashboard"></parameters>
   </div>
 
   <div class="m-b-10 p-15 bg-white tiled" ng-if="$ctrl.filters | notEmpty">

diff --git a/client/app/pages/queries/query.html b/client/app/pages/queries/query.html
@@ -191,7 +191,8 @@ <h3>
           <section class="flex-fill p-relative t-body query-visualizations-wrapper">
             <div class="d-flex flex-column p-b-15 p-absolute static-position__mobile" style="left: 0; top: 0; right: 0; bottom: 0;">
               <div class="p-t-15 p-b-5" ng-if="query.hasParameters()">
-                <parameters parameters="query.getParametersDefs()" sync-values="!query.isNew()" editable="sourceMode && canEdit" on-updated="onParametersUpdated"></parameters>
+                <parameters parameters="query.getParametersDefs()" sync-values="!query.isNew()" editable="sourceMode && canEdit"
+                  on-updated="onParametersUpdated" on-values-change="executeQuery"></parameters>
               </div>
               <!-- Query Execution Status -->
 

diff --git a/client/app/pages/queries/view.js b/client/app/pages/queries/view.js
@@ -49,7 +49,6 @@ function QueryViewCtrl(
     }
   }
 
-
   function getDataSourceId() {
     // Try to get the query's data source id
     let dataSourceId = $scope.query.data_source_id;

diff --git a/client/cypress/integration/dashboard/dashboard_spec.js b/client/cypress/integration/dashboard/dashboard_spec.js
@@ -444,14 +444,12 @@ describe('Dashboard', () => {
           cy.route('GET', 'api/query_results/*').as('FreshResults');
 
           // start with 1 table row
-          cy.get('@paramInput').clear().type('1');
-          cy.get('@refreshButton').click();
+          cy.get('@paramInput').clear().type('1{enter}');
           cy.wait('@FreshResults', { timeout: 10000 });
           cy.get('@widget').invoke('height').should('eq', 285);
 
           // add 4 table rows
-          cy.get('@paramInput').clear().type('5');
-          cy.get('@refreshButton').click();
+          cy.get('@paramInput').clear().type('5{enter}');
           cy.wait('@FreshResults', { timeout: 10000 });
 
           // expect to height to grow by 1 grid grow
@@ -466,8 +464,7 @@ describe('Dashboard', () => {
           editDashboard();
 
           // start with 1 table row
-          cy.get('@paramInput').clear().type('1');
-          cy.get('@refreshButton').click();
+          cy.get('@paramInput').clear().type('1{enter}');
           cy.wait('@FreshResults');
           cy.get('@widget').invoke('height').should('eq', 285);
 
@@ -476,8 +473,7 @@ describe('Dashboard', () => {
           cy.get('@widget').invoke('height').should('eq', 335);
 
           // add 4 table rows
-          cy.get('@paramInput').clear().type('5');
-          cy.get('@refreshButton').click();
+          cy.get('@paramInput').clear().type('5{enter}');
           cy.wait('@FreshResults');
 
           // expect height to stay unchanged (would have been 435)
-Original file line number
+Diff line change
@@ Expand Up / @@ -38,7 +38,7 @@ @@
             </div>
           </div>
           <div class="m-b-10" ng-if="$ctrl.localParametersDefs().length > 0">
-            <parameters parameters="$ctrl.localParametersDefs()"></parameters>
+            <parameters parameters="$ctrl.localParametersDefs()" on-values-change="$ctrl.refresh"></parameters>
           </div>
         </div>
@@ Expand Down @@