Skip to content

Commit

Permalink
[Vis: Default editor] EUIficate string control (elastic#32881)
Browse files Browse the repository at this point in the history
* EUIficate string control

* Fix tests

* Update unit test

* Add data-test-subj for input

* Remove unused props

* Refactoring

* Add value prop to be able to discard changes on form

* Refactoring

* Set ng-model

* Remove unnecessary check and specify model name

* Set full width to be consistent with other controls from the form
  • Loading branch information
maryia-lapata committed Mar 18, 2019
1 parent 663a7ca commit cd0a0f4
Show file tree
Hide file tree
Showing 14 changed files with 247 additions and 34 deletions.
26 changes: 26 additions & 0 deletions src/legacy/ui/public/agg_types/agg_param.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

interface AggParam {
type: string;
name: string;
displayName?: string;
}

export { AggParam };
2 changes: 2 additions & 0 deletions src/legacy/ui/public/agg_types/agg_type.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import _ from 'lodash';
import { AggParams } from './agg_params';
import { fieldFormats } from '../registry/field_formats';
import { i18n } from '@kbn/i18n';

/**
* Generic AggType Constructor
Expand Down Expand Up @@ -118,6 +119,7 @@ function AggType(config) {
if (config.customLabels !== false) {
this.params.push({
name: 'customLabel',
displayName: i18n.translate('common.ui.aggTypes.string.customLabel', { defaultMessage: 'Custom label' }),
type: 'string',
write: _.noop
});
Expand Down
2 changes: 2 additions & 0 deletions src/legacy/ui/public/agg_types/buckets/terms.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,13 +311,15 @@ export const termsBucketAgg = new BucketAggType({
},
{
name: 'exclude',
displayName: i18n.translate('common.ui.aggTypes.buckets.terms.excludeLabel', { defaultMessage: 'Exclude' }),
type: 'string',
advanced: true,
disabled: isNotType('string'),
...migrateIncludeExcludeFormat
},
{
name: 'include',
displayName: i18n.translate('common.ui.aggTypes.buckets.terms.includeLabel', { defaultMessage: 'Include' }),
type: 'string',
advanced: true,
disabled: isNotType('string'),
Expand Down
13 changes: 0 additions & 13 deletions src/legacy/ui/public/agg_types/controls/string.html

This file was deleted.

42 changes: 42 additions & 0 deletions src/legacy/ui/public/agg_types/controls/string.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import React from 'react';

import { EuiFieldText, EuiFormRow } from '@elastic/eui';
import { AggParamEditorProps } from '../../vis/editors/default';

function StringParamEditor({ agg, aggParam, value, setValue }: AggParamEditorProps<string>) {
return (
<EuiFormRow
label={aggParam.displayName || aggParam.name}
className="form-group"
fullWidth={true}
>
<EuiFieldText
value={value || ''}
data-test-subj={`visEditorStringInput${agg.id}${aggParam.name}`}
onChange={ev => setValue(ev.target.value)}
fullWidth={true}
/>
</EuiFormRow>
);
}

export { StringParamEditor };
1 change: 1 addition & 0 deletions src/legacy/ui/public/agg_types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@
* under the License.
*/

export { AggParam } from './agg_param';
export { AggType } from './agg_type';
7 changes: 4 additions & 3 deletions src/legacy/ui/public/agg_types/param_types/string.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,16 @@
* under the License.
*/

import editorHtml from '../controls/string.html';
import { BaseParamType } from './base';
import { createLegacyClass } from '../../utils/legacy_class';
import { StringParamEditor } from '../controls/string';
import { BaseParamType } from './base';

createLegacyClass(StringParamType).inherits(BaseParamType);
function StringParamType(config) {
StringParamType.Super.call(this, config);
}

StringParamType.prototype.editor = editorHtml;
StringParamType.prototype.editorComponent = StringParamEditor;

/**
* Write the aggregation parameter.
Expand All @@ -45,3 +45,4 @@ StringParamType.prototype.write = function (aggConfig, output) {
};

export { StringParamType };

Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ describe('Vis-Editor-Agg-Params plugin directive', function () {
aggFilter: aggFilter
});

const customLabelElement = $elem.find('label:contains("Custom Label")');
const customLabelElement = $elem.find('label:contains("Custom label")');
expect(customLabelElement.length).to.be(1);
});

Expand All @@ -117,7 +117,7 @@ describe('Vis-Editor-Agg-Params plugin directive', function () {
aggFilter: aggFilter
});

const customLabelElement = $elem.find('label:contains("Custom Label")');
const customLabelElement = $elem.find('label:contains("Custom label")');
expect(customLabelElement.length).to.be(0);
});
});
56 changes: 52 additions & 4 deletions src/legacy/ui/public/vis/editors/default/agg_param.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,32 +17,80 @@
* under the License.
*/

import _ from 'lodash';
import { isFunction } from 'lodash';
import { wrapInI18nContext } from 'ui/i18n';
import { uiModules } from '../../../modules';
import { AggParamReactWrapper } from './agg_param_react_wrapper';

uiModules
.get('app/visualize')
.directive('visAggParamReactWrapper', reactDirective => reactDirective(wrapInI18nContext(AggParamReactWrapper), [
['agg', { watchDepth: 'collection' }],
['aggParam', { watchDepth: 'reference' }],
['paramEditor', { wrapApply: false }],
['onChange', { watchDepth: 'reference' }],
'value',
]))
.directive('visAggParamEditor', function (config) {
return {
restrict: 'E',
// We can't use scope binding here yet, since quiet a lot of child directives arbitrary access
// parent scope values right now. So we cannot easy change this, until we remove the whole directive.
scope: true,
template: function ($el) {
require: '?^ngModel',
template: function ($el, attrs) {
if (attrs.editorComponent) {
// Why do we need the `ng-if` here?
// Short answer: Preventing black magic
// Longer answer: The way this component is mounted in agg_params.js (by manually compiling)
// and adding to some array, once you switch an aggregation type, this component will once
// render once with a "broken state" (something like new aggParam, but still old template),
// before agg_params.js actually removes it from the DOM and create a correct version with
// the correct template. That ng-if check prevents us from crashing during that broken render.
return `<vis-agg-param-react-wrapper
ng-if="editorComponent"
param-editor="editorComponent"
agg="agg"
agg-param="aggParam"
on-change="onChange"
value="paramValue"
></vis-agg-param-react-wrapper>`;
}

return $el.html();
},
link: {
pre: function ($scope, $el, attr) {
$scope.$bind('aggParam', attr.aggParam);
$scope.$bind('agg', attr.agg);
$scope.$bind('editorComponent', attr.editorComponent);
},
post: function ($scope) {
post: function ($scope, $el, attr, ngModelCtrl) {
$scope.config = config;

$scope.optionEnabled = function (option) {
if (option && _.isFunction(option.enabled)) {
if (option && isFunction(option.enabled)) {
return option.enabled($scope.agg);
}

return true;
};

$scope.$watch('agg.params[aggParam.name]', (value) => {
// Whenever the value of the parameter changed (e.g. by a reset or actually by calling)
// we store the new value in $scope.paramValue, which will be passed as a new value to the react component.
$scope.paramValue = value;
}, true);

$scope.onChange = (value) => {
// This is obviously not a good code quality, but without using scope binding (which we can't see above)
// to bind function values, this is right now the best temporary fix, until all of this will be gone.
$scope.$parent.onParamChange($scope.agg, $scope.aggParam.name, value);

if(ngModelCtrl) {
ngModelCtrl.$setDirty();
}
};
}
}
};
Expand Down
30 changes: 30 additions & 0 deletions src/legacy/ui/public/vis/editors/default/agg_param_editor_props.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { AggParam } from '../../../agg_types';
import { AggConfig } from '../../agg_config';

interface AggParamEditorProps<T> {
agg: AggConfig;
aggParam: AggParam;
value: T;
setValue(value: T): void;
}

export { AggParamEditorProps };
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import React from 'react';

import { AggParam } from '../../../agg_types';
import { AggConfig } from '../../agg_config';
import { AggParamEditorProps } from './agg_param_editor_props';

interface AggParamReactWrapperProps<T> {
agg: AggConfig;
aggParam: AggParam;
paramEditor: React.FunctionComponent<AggParamEditorProps<T>>;
value: T;
onChange(value: T): void;
}

function AggParamReactWrapper<T>(props: AggParamReactWrapperProps<T>) {
const { agg, aggParam, paramEditor: ParamEditor, onChange, value } = props;
return <ParamEditor value={value} setValue={onChange} aggParam={aggParam} agg={agg} />;
}

export { AggParamReactWrapper };
37 changes: 26 additions & 11 deletions src/legacy/ui/public/vis/editors/default/agg_params.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@
*/

import $ from 'jquery';
import { has, get } from 'lodash';
import aggSelectHtml from './agg_select.html';
import advancedToggleHtml from './advanced_toggle.html';
import '../../../filters/match_any';
import './agg_param';
import { get, has } from 'lodash';
import { aggTypes } from '../../../agg_types';
import { uiModules } from '../../../modules';
import { documentationLinks } from '../../../documentation_links/documentation_links';
import aggParamsTemplate from './agg_params.html';
import { aggTypeFilters } from '../../../agg_types/filter';
import { editorConfigProviders } from '../config/editor_config_providers';
import { aggTypeFieldFilters } from '../../../agg_types/param_types/filter';
import { documentationLinks } from '../../../documentation_links/documentation_links';
import '../../../filters/match_any';
import { uiModules } from '../../../modules';
import { editorConfigProviders } from '../config/editor_config_providers';
import advancedToggleHtml from './advanced_toggle.html';
import './agg_param';
import aggParamsTemplate from './agg_params.html';
import aggSelectHtml from './agg_select.html';

uiModules
.get('app/visualize')
Expand All @@ -56,6 +56,12 @@ uiModules
updateEditorConfig('default');
});

$scope.onParamChange = (agg, paramName, value) => {
if(agg.params[paramName] !== value) {
agg.params[paramName] = value;
}
};

function updateEditorConfig(property = 'fixedValue') {
$scope.editorConfig = editorConfigProviders.getConfigForAgg(
aggTypes.byType[$scope.groupName],
Expand Down Expand Up @@ -185,18 +191,27 @@ uiModules
// build HTML editor given an aggParam and index
function getAggParamHTML(param, idx) {
// don't show params without an editor
if (!param.editor) {
if (!param.editor && !param.editorComponent) {
return;
}

const attrs = {
'agg-param': 'agg.type.params[' + idx + ']'
'agg-param': 'agg.type.params[' + idx + ']',
'agg': 'agg',
};

if (param.advanced) {
attrs['ng-show'] = 'advancedToggled';
}

if (param.editorComponent) {
attrs['editor-component'] = `agg.type.params[${idx}].editorComponent`;
// The form should interact with reactified components as well.
// So we set the ng-model (using a random ng-model variable) to have the method to set dirty
// inside the agg_param.js directive, which can get access to the ngModelController to manipulate it.
attrs['ng-model'] = `_internalNgModelState${$scope.agg.id}${param.name}`;
}

return $('<vis-agg-param-editor>')
.attr(attrs)
.append(param.editor)
Expand Down
Loading

0 comments on commit cd0a0f4

Please sign in to comment.