[Widget Params] Switched parameter list to table style (all parts) (#3332)

* [Widget Params] Split title and mapping editing

* [Widget Params] Restyled source editing

* [Widget Params] Switched parameter list to table style

* Displaying different labels and help phrases when changing type
Added link to knowledge base
Fixed issue with existing param default select value

client/app/assets/less/ant.less

@@ -13,6 +13,11 @@
 @import '~antd/lib/radio/style/index';
 @import '~antd/lib/time-picker/style/index';
 @import '~antd/lib/pagination/style/index';
+@import '~antd/lib/table/style/index';
+@import '~antd/lib/popover/style/index';
+@import '~antd/lib/icon/style/index';
+@import '~antd/lib/tag/style/index';
+@import '~antd/lib/grid/style/index';
 @import 'inc/ant-variables';
 
 // Remove bold in labels for Ant checkboxes and radio buttons

client/app/components/ParameterMappingInput.jsx

@@ -1,14 +1,28 @@
 /* eslint react/no-multi-comp: 0 */
 
-import { extend, map, includes, findIndex, find, fromPairs } from 'lodash';
-import React from 'react';
+import { extend, map, includes, findIndex, find, fromPairs, clone, isEmpty, replace } from 'lodash';
+import React, { Fragment } from 'react';
 import PropTypes from 'prop-types';
 import Select from 'antd/lib/select';
+import Table from 'antd/lib/table';
+import Popover from 'antd/lib/popover';
+import Button from 'antd/lib/button';
+import Icon from 'antd/lib/icon';
+import Tag from 'antd/lib/tag';
+import Input from 'antd/lib/input';
+import Radio from 'antd/lib/radio';
+import Form from 'antd/lib/form';
+import Tooltip from 'antd/lib/tooltip';
 import { ParameterValueInput } from '@/components/ParameterValueInput';
 import { ParameterMappingType } from '@/services/widget';
+import { Parameter } from '@/services/query';
+
+import './ParameterMappingInput.less';
 
 const { Option } = Select;
 
+const HELP_URL = 'https://redash.io/help/user-guide/querying/query-parameters?source={0}';
+
 export const MappingType = {
   DashboardAddNew: 'dashboard-add-new',
   DashboardMapToExisting: 'dashboard-map-to-existing',
@@ -80,6 +94,7 @@ export class ParameterMappingInput extends React.Component {
     onChange: PropTypes.func,
     clientConfig: PropTypes.any, // eslint-disable-line react/forbid-prop-types
     Query: PropTypes.any, // eslint-disable-line react/forbid-prop-types
+    inputError: PropTypes.string,
   };
 
   static defaultProps = {
@@ -88,138 +103,343 @@ export class ParameterMappingInput extends React.Component {
     onChange: () => {},
     clientConfig: null,
     Query: null,
+    inputError: null,
   };
 
-  updateParamMapping(mapping, updates) {
-    this.props.onChange(extend({}, mapping, updates));
+  constructor(props) {
+    super(props);
+
+    this.formItemProps = {
+      labelCol: { span: 5 },
+      wrapperCol: { span: 16 },
+      className: 'formItem',
+    };
+  }
+
+  updateSourceType = (type) => {
+    let { mapping: { mapTo } } = this.props;
+    const { existingParamNames } = this.props;
+
+    // if mapped name doesn't already exists
+    // default to first select option
+    if (
+      type === MappingType.DashboardMapToExisting &&
+      !includes(existingParamNames, mapTo)
+    ) {
+      mapTo = existingParamNames[0];
+    }
+
+    this.updateParamMapping({ type, mapTo });
+  }
+
+  updateParamMapping = (update) => {
+    const { onChange, mapping } = this.props;
+    const newMapping = extend({}, mapping, update);
+    onChange(newMapping);
   }
 
   renderMappingTypeSelector() {
-    const { mapping, existingParamNames } = this.props;
+    const noExisting = isEmpty(this.props.existingParamNames);
     return (
-      <div>
-        <Select
-          className="w-100"
-          defaultValue={mapping.type}
-          onChange={type => this.updateParamMapping(mapping, { type })}
-          dropdownClassName="ant-dropdown-in-bootstrap-modal"
+      <Radio.Group
+        value={this.props.mapping.type}
+        onChange={e => this.updateSourceType(e.target.value)}
+      >
+        <Radio className="radio" value={MappingType.DashboardAddNew}>
+          New dashboard parameter
+        </Radio>
+        <Radio
+          className="radio"
+          value={MappingType.DashboardMapToExisting}
+          disabled={noExisting}
         >
-          <Option value={MappingType.DashboardAddNew}>Add the parameter to the dashboard</Option>
-          {
-            (existingParamNames.length > 0) &&
-            <Option value={MappingType.DashboardMapToExisting}>Map to existing parameter</Option>
-          }
-          <Option value={MappingType.StaticValue}>Use static value for the parameter</Option>
-          <Option value={MappingType.WidgetLevel}>Keep the parameter at the widget level</Option>
-        </Select>
-      </div>
+          Existing dashboard parameter{' '}
+          {noExisting ? (
+            <Tooltip title="There are no dashboard parameters corresponding to this data type">
+              <Icon type="question-circle" theme="filled" />
+            </Tooltip>
+          ) : null }
+        </Radio>
+        <Radio className="radio" value={MappingType.WidgetLevel}>
+          Widget parameter
+        </Radio>
+        <Radio className="radio" value={MappingType.StaticValue}>
+          Static value
+        </Radio>
+      </Radio.Group>
     );
   }
 
   renderDashboardAddNew() {
-    const { mapping, existingParamNames } = this.props;
-    const alreadyExists = includes(existingParamNames, mapping.mapTo);
+    const { mapping: { mapTo } } = this.props;
     return (
-      <div className={'m-t-10' + (alreadyExists ? ' has-error' : '')}>
-        <input
-          type="text"
-          className="form-control"
-          value={mapping.mapTo}
-          onChange={event => this.updateParamMapping(mapping, { mapTo: event.target.value })}
-        />
-        { alreadyExists && (
-          <div className="help-block">
-            Dashboard parameter with this name already exists
-          </div>
-        )}
-      </div>
+      <Input
+        value={mapTo}
+        onChange={e => this.updateParamMapping({ mapTo: e.target.value })}
+      />
     );
   }
 
   renderDashboardMapToExisting() {
     const { mapping, existingParamNames } = this.props;
+
     return (
-      <div className="m-t-10">
-        <Select
-          className="w-100"
-          defaultValue={mapping.mapTo}
-          onChange={mapTo => this.updateParamMapping(mapping, { mapTo })}
-          disabled={existingParamNames.length === 0}
-          dropdownClassName="ant-dropdown-in-bootstrap-modal"
-        >
-          {map(existingParamNames, name => (
-            <Option value={name} key={name}>{ name }</Option>
-          ))}
-        </Select>
-      </div>
+      <Select
+        value={mapping.mapTo}
+        onChange={mapTo => this.updateParamMapping({ mapTo })}
+        dropdownMatchSelectWidth={false}
+      >
+        {map(existingParamNames, name => (
+          <Option value={name} key={name}>{ name }</Option>
+        ))}
+      </Select>
     );
   }
 
   renderStaticValue() {
     const { mapping } = this.props;
     return (
-      <div className="m-t-10">
-        <label htmlFor="parameter-value-input">Change parameter value:</label>
-        <ParameterValueInput
-          id="parameter-value-input"
-          className="w-100"
-          type={mapping.param.type}
-          value={mapping.param.normalizedValue}
-          enumOptions={mapping.param.enumOptions}
-          queryId={mapping.param.queryId}
-          onSelect={value => this.updateParamMapping(mapping, { value })}
-          clientConfig={this.props.clientConfig}
-          Query={this.props.Query}
-        />
-      </div>
+      <ParameterValueInput
+        type={mapping.param.type}
+        value={mapping.param.normalizedValue}
+        enumOptions={mapping.param.enumOptions}
+        queryId={mapping.param.queryId}
+        onSelect={value => this.updateParamMapping({ value })}
+        clientConfig={this.props.clientConfig}
+        Query={this.props.Query}
+      />
     );
   }
 
   renderInputBlock() {
     const { mapping } = this.props;
     switch (mapping.type) {
-      case MappingType.DashboardAddNew: return this.renderDashboardAddNew();
-      case MappingType.DashboardMapToExisting: return this.renderDashboardMapToExisting();
-      case MappingType.StaticValue: return this.renderStaticValue();
-      // no default
+      case MappingType.DashboardAddNew:
+        return [
+          'Key',
+          'Enter a new parameter keyword',
+          this.renderDashboardAddNew(),
+        ];
+      case MappingType.DashboardMapToExisting:
+        return [
+          'Key',
+          'Select from a list of existing parameters',
+          this.renderDashboardMapToExisting(),
+        ];
+      case MappingType.StaticValue:
+        return [
+          'Value',
+          null,
+          this.renderStaticValue(),
+        ];
+      default: return [];
     }
   }
 
-  renderTitleInput() {
-    const { mapping } = this.props;
-    if (mapping.type === MappingType.StaticValue) {
-      return null;
-    }
+  render() {
+    const { inputError } = this.props;
+    const [label, help, input] = this.renderInputBlock();
+
     return (
-      <div className="m-t-10">
-        <label htmlFor="parameter-title">Change parameter title (leave empty to use existing):</label>
-        <input
-          id="parameter-title"
-          type="text"
-          className="form-control"
-          value={mapping.title}
-          onChange={event => this.updateParamMapping(mapping, { title: event.target.value })}
-          placeholder={mapping.param.title}
+      <Form layout="horizontal">
+        <Form.Item label="Source" {...this.formItemProps}>
+          {this.renderMappingTypeSelector()}
+        </Form.Item>
+        <Form.Item
+          style={{ height: 60, visibility: input ? 'visible' : 'hidden' }}
+          label={label}
+          {...this.formItemProps}
+          validateStatus={inputError ? 'error' : ''}
+          help={inputError || help} // empty space so line doesn't collapse
+        >
+          {input}
+        </Form.Item>
+      </Form>
+    );
+  }
+}
+
+class EditMapping extends React.Component {
+  static propTypes = {
+    mapping: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types
+    existingParamNames: PropTypes.arrayOf(PropTypes.string).isRequired,
+    onChange: PropTypes.func.isRequired,
+    getContainerElement: PropTypes.func.isRequired,
+    clientConfig: PropTypes.any, // eslint-disable-line react/forbid-prop-types
+    Query: PropTypes.any, // eslint-disable-line react/forbid-prop-types
+  };
+
+  static defaultProps = {
+    clientConfig: null,
+    Query: null,
+  }
+
+  constructor(props) {
+    super(props);
+    this.state = {
+      visible: false,
+      mapping: clone(this.props.mapping),
+      inputError: null,
+    };
+  }
+
+  onVisibleChange = (visible) => {
+    if (visible) this.show(); else this.hide();
+  }
+
+  onChange = (mapping) => {
+    let inputError = null;
+
+    if (mapping.type === MappingType.DashboardAddNew) {
+      if (isEmpty(mapping.mapTo)) {
+        inputError = 'Keyword must have a value';
+      } else if (includes(this.props.existingParamNames, mapping.mapTo)) {
+        inputError = 'Parameter with this name already exists';
+      }
+    }
+
+    this.setState({ mapping, inputError });
+  }
+
+  get content() {
+    const { mapping, inputError } = this.state;
+    const { clientConfig, Query } = this.props;
+    const helpUrl = replace(HELP_URL, '{0}', 'edit_mapping');
+
+    return (
+      <div className="editMapping">
+        <header>
+          Edit Source and Value
+          {/* eslint-disable-next-line react/jsx-no-target-blank */}
+          <a href={helpUrl} target="_blank" rel="noopener">
+            <Tooltip title="Learn more about editing query paramaters (opens in a new window)">
+              <Icon type="question-circle" />
+            </Tooltip>
+          </a>
+        </header>
+        <ParameterMappingInput
+          mapping={mapping}
+          existingParamNames={this.props.existingParamNames}
+          onChange={this.onChange}
+          getContainerElement={() => this.wrapperRef.current}
+          clientConfig={clientConfig}
+          Query={Query}
+          inputError={inputError}
         />
+        <footer>
+          <Button onClick={this.hide}>Cancel</Button>
+          <Button onClick={this.save} disabled={!!inputError} type="primary">OK</Button>
+        </footer>
       </div>
     );
   }
 
+  save = () => {
+    this.props.onChange(this.props.mapping, this.state.mapping);
+    this.hide();
+  }
+
+  show = () => {
+    this.setState({
+      visible: true,
+      mapping: clone(this.props.mapping), // restore original state
+    });
+  }
+
+  hide = () => {
+    this.setState({ visible: false });
+  }
+
   render() {
-    const { mapping } = this.props;
     return (
-      <div key={mapping.name} className="row">
-        <div className="col-xs-5">
-          <div className="form-control-static">{'{{ ' + mapping.name + ' }}'}</div>
-        </div>
-        <div className="col-xs-7">
-          {this.renderMappingTypeSelector()}
-          {this.renderInputBlock()}
-          {this.renderTitleInput()}
-        </div>
+      <Popover
+        placement="left"
+        trigger="click"
+        content={this.content}
+        visible={this.state.visible}
+        onVisibleChange={this.onVisibleChange}
+        getPopupContainer={this.props.getContainerElement}
+      >
+        <Button size="small" type="dashed">
+          <Icon type="edit" />
+        </Button>
+      </Popover>
+    );
+  }
+}
+
+class EditTitle extends React.Component {
+  static propTypes = {
+    mapping: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types
+    onChange: PropTypes.func.isRequired,
+    getContainerElement: PropTypes.func.isRequired,
+  };
+
+  state = {
+    visible: false,
+    title: this.props.mapping.title,
+  }
+
+  onVisibleChange = (visible) => {
+    this.setState({
+      visible,
+      title: this.props.mapping.title, // reset title
+    });
+  }
+
+  onTitleChange = (event) => {
+    this.setState({ title: event.target.value });
+  }
+
+  get popover() {
+    const { param: { title: paramTitle } } = this.props.mapping;
+
+    return (
+      <div className="editTitle">
+        <Input
+          size="small"
+          value={this.state.title}
+          placeholder={paramTitle}
+          onChange={this.onTitleChange}
+          onPressEnter={this.save}
+          autoFocus
+        />
+        <Button size="small" type="dashed" onClick={this.hide}>
+          <Icon type="close" />
+        </Button>
+        <Button size="small" type="dashed" onClick={this.save}>
+          <Icon type="check" />
+        </Button>
       </div>
     );
   }
+
+  save = () => {
+    const newMapping = extend({}, this.props.mapping, { title: this.state.title });
+    this.props.onChange(newMapping);
+    this.hide();
+  }
+
+  hide = () => {
+    this.setState({ visible: false });
+  }
+
+  render() {
+    return (
+      <Popover
+        placement="right"
+        trigger="click"
+        content={this.popover}
+        visible={this.state.visible}
+        onVisibleChange={this.onVisibleChange}
+        getPopupContainer={this.props.getContainerElement}
+      >
+        <Button size="small" type="dashed">
+          <Icon type="edit" />
+        </Button>
+      </Popover>
+    );
+  }
 }
 
 export class ParameterMappingListInput extends React.Component {
@@ -242,6 +462,70 @@ export class ParameterMappingListInput extends React.Component {
     Query: null,
   };
 
+  constructor(props) {
+    super(props);
+    this.wrapperRef = React.createRef();
+  }
+
+  static getStringValue(value) {
+    // null
+    if (!value) {
+      return '';
+    }
+
+    // range
+    if (value instanceof Object && 'start' in value && 'end' in value) {
+      return `${value.start} ~ ${value.end}`;
+    }
+
+    // just to be safe, array or object
+    if (typeof value === 'object') {
+      return map(value, v => this.getStringValue(v)).join(', ');
+    }
+
+    // rest
+    return value.toString();
+  }
+
+  static getDefaultValue(mapping, existingParams) {
+    const { type, mapTo, name } = mapping;
+    let { param } = mapping;
+
+    // if mapped to another param, swap 'em
+    if (type === MappingType.DashboardMapToExisting && mapTo !== name) {
+      const mappedTo = find(existingParams, { name: mapTo });
+      if (mappedTo) { // just being safe
+        param = mappedTo;
+      }
+
+    // static type is different since it's fed param.normalizedValue
+    } else if (type === MappingType.StaticValue) {
+      param = param.clone().setValue(mapping.value);
+    }
+
+    const value = Parameter.getValue(param);
+    return this.getStringValue(value);
+  }
+
+  static getSourceTypeLabel({ type, mapTo }) {
+    switch (type) {
+      case MappingType.DashboardAddNew:
+      case MappingType.DashboardMapToExisting:
+        return (
+          <Fragment>
+            Dashboard{' '}
+            <Tag className="tag">{mapTo}</Tag>
+          </Fragment>
+        );
+      case MappingType.WidgetLevel:
+        return 'Widget parameter';
+      case MappingType.StaticValue:
+        return 'Static value';
+      default:
+        return ''; // won't happen (typescript-ftw)
+    }
+  }
+
   updateParamMapping(oldMapping, newMapping) {
     const mappings = [...this.props.mappings];
     const index = findIndex(mappings, oldMapping);
@@ -255,28 +539,75 @@ export class ParameterMappingListInput extends React.Component {
   }
 
   render() {
-    const clientConfig = this.props.clientConfig; // eslint-disable-line react/prop-types
-    const Query = this.props.Query; // eslint-disable-line react/prop-types
+    const { clientConfig, Query, existingParams } = this.props; // eslint-disable-line react/prop-types
+    const dataSource = this.props.mappings.map(mapping => ({ mapping }));
 
     return (
-      <div>
-        {this.props.mappings.map((mapping, index) => {
-          const existingParamsNames = this.props.existingParams
-            .filter(({ type }) => type === mapping.param.type) // exclude mismatching param types
-            .map(({ name }) => name); // keep names only
+      <div ref={this.wrapperRef} className="paramMappingList">
+        <Table
+          dataSource={dataSource}
+          size="middle"
+          pagination={false}
+          rowKey={(record, idx) => `row${idx}`}
+        >
+          <Table.Column
+            title="Title"
+            dataIndex="mapping"
+            key="title"
+            render={(mapping) => {
+              const { title, param: { title: paramTitle } } = mapping;
+              return (
+                <Fragment>
+                  {title || paramTitle}{' '}
+                  <EditTitle
+                    mapping={mapping}
+                    onChange={newMapping => this.updateParamMapping(mapping, newMapping)}
+                    getContainerElement={() => this.wrapperRef.current}
+                  />
+                </Fragment>
+              );
+            }}
+          />
+          <Table.Column
+            title="Keyword"
+            dataIndex="mapping"
+            key="keyword"
+            className="keyword"
+            render={mapping => <code>{`{{ ${mapping.name} }}`}</code>}
+          />
+          <Table.Column
+            title="Default Value"
+            dataIndex="mapping"
+            key="value"
+            render={mapping => (
+              this.constructor.getDefaultValue(mapping, this.props.existingParams)
+            )}
+          />
+          <Table.Column
+            title="Value Source"
+            dataIndex="mapping"
+            key="source"
+            render={(mapping) => {
+              const existingParamsNames = existingParams
+                .filter(({ type }) => type === mapping.param.type) // exclude mismatching param types
+                .map(({ name }) => name); // keep names only
 
-          return (
-            <div key={mapping.name} className={(index === 0 ? '' : ' m-t-15')}>
-              <ParameterMappingInput
-                mapping={mapping}
-                existingParamNames={existingParamsNames}
-                onChange={newMapping => this.updateParamMapping(mapping, newMapping)}
-                clientConfig={clientConfig}
-                Query={Query}
-              />
-            </div>
-          );
-        })}
+              return (
+                <Fragment>
+                  {this.constructor.getSourceTypeLabel(mapping)}{' '}
+                  <EditMapping
+                    mapping={mapping}
+                    existingParamNames={existingParamsNames}
+                    onChange={(oldMapping, newMapping) => this.updateParamMapping(oldMapping, newMapping)}
+                    getContainerElement={() => this.wrapperRef.current}
+                    clientConfig={clientConfig}
+                    Query={Query}
+                  />
+                </Fragment>
+              );
+            }}
+          />
+        </Table>
       </div>
     );
   }

client/app/components/ParameterMappingInput.less

@@ -0,0 +1,69 @@
+@import '~antd/lib/modal/style/index'; // for ant @vars
+
+.paramMappingList {
+  .keyword {
+    max-width: 100px;
+    overflow: hidden;
+    text-overflow: ellipsis;
+
+    code {
+      white-space: nowrap; // so the curly braces don't line break
+    }
+  }
+
+  // Ant <Tag> overrides
+  .tag {
+    margin: 0;
+    pointer-events: none; // unclickable
+
+    &:empty {
+      display: none;
+    }
+  }
+}
+
+.editMapping {
+  width: 390px;
+
+  .radio {
+    display: block;
+    height: 30px;
+    line-height: 30px;
+  }
+
+  .formItem {
+    margin-bottom: 10px;
+  }
+
+  header {
+    padding: 0 16px 10px;
+    margin: 0 -16px 20px;
+    border-bottom: @border-width-base @border-style-base @border-color-split;
+    font-size: @font-size-lg;
+    font-weight: 500;
+    color: @heading-color;
+    display: flex;
+    justify-content: space-between;
+  }
+
+  footer {
+    border-top: @border-width-base @border-style-base @border-color-split;
+    padding: 10px 16px 0;
+    margin: 0 -16px;
+    text-align: right;
+
+    button {
+      margin-left: 8px;
+    }
+  }
+}
+
+.editTitle {
+  input {
+    width: 100px;
+  }
+
+  button {
+    margin-left: 2px;
+  }
+}

client/app/components/dashboards/AddWidgetDialog.jsx

@@ -308,6 +308,7 @@ class AddWidgetDialog extends React.Component {
         }}
         okText="Add to Dashboard"
         onCancel={this.close}
+        width={700}
       >
         {this.renderQueryInput()}
         {!this.state.selectedQuery && this.renderSearchQueryResults()}

client/app/components/dashboards/EditParameterMappingsDialog.jsx

@@ -65,11 +65,6 @@ class EditParameterMappingsDialog extends React.Component {
   }
 
   render() {
-    const existingParams = map(
-      this.props.dashboard.getParametersDefs(),
-      ({ name, type }) => ({ name, type }),
-    );
-
     return (
       <Modal
         visible={this.state.showModal}
@@ -78,11 +73,12 @@ class EditParameterMappingsDialog extends React.Component {
         onOk={() => this.saveWidget()}
         okButtonProps={{ loading: this.state.saveInProgress }}
         onCancel={this.close}
+        width={700}
       >
         {(this.state.parameterMappings.length > 0) && (
           <ParameterMappingListInput
             mappings={this.state.parameterMappings}
-            existingParams={existingParams}
+            existingParams={this.props.dashboard.getParametersDefs()}
             onChange={mappings => this.updateParamMappings(mappings)}
           />
         )}

client/app/services/query.js

@@ -49,7 +49,7 @@ function isDateRangeParameter(paramType) {
   return includes(['date-range', 'datetime-range', 'datetime-range-with-seconds'], paramType);
 }
 
-class Parameter {
+export class Parameter {
   constructor(parameter) {
     this.title = parameter.title;
     this.name = parameter.name;
@@ -78,20 +78,25 @@ class Parameter {
   }
 
   getValue() {
-    const isEmptyValue = isNull(this.value) || isUndefined(this.value) || (this.value === '');
+    return this.constructor.getValue(this);
+  }
+
+  static getValue(param) {
+    const { value, type, useCurrentDateTime } = param;
+    const isEmptyValue = isNull(value) || isUndefined(value) || (value === '');
     if (isEmptyValue) {
       if (
-        includes(['date', 'datetime-local', 'datetime-with-seconds'], this.type) &&
-        this.useCurrentDateTime
+        includes(['date', 'datetime-local', 'datetime-with-seconds'], type) &&
+        useCurrentDateTime
       ) {
-        return moment().format(DATETIME_FORMATS[this.type]);
+        return moment().format(DATETIME_FORMATS[type]);
       }
       return null; // normalize empty value
     }
-    if (this.type === 'number') {
-      return normalizeNumericValue(this.value, null); // normalize empty value
+    if (type === 'number') {
+      return normalizeNumericValue(value, null); // normalize empty value
     }
-    return this.value;
+    return value;
   }
 
   setValue(value) {
@@ -135,6 +140,8 @@ class Parameter {
         local.setValue(this.value);
       });
     }
+
+    return this;
   }
 
   get normalizedValue() {