Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Vis: Default editor] EUIficate ranges param editor #38531

Merged
merged 9 commits into from
Jun 12, 2019
4 changes: 2 additions & 2 deletions src/legacy/ui/public/agg_types/buckets/range.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { BucketAggType } from './_bucket_agg_type';
import { createFilterRange } from './create_filter/range';
import { FieldFormat } from '../../../field_formats/field_format';
import { RangeKey } from './range_key';
import rangesTemplate from '../controls/ranges.html';
import { RangesParamEditor } from '../controls/ranges';
import { i18n } from '@kbn/i18n';

const keyCaches = new WeakMap();
Expand Down Expand Up @@ -91,7 +91,7 @@ export const rangeBucketAgg = new BucketAggType({
{ from: 0, to: 1000 },
{ from: 1000, to: 2000 }
],
editor: rangesTemplate,
editorComponent: RangesParamEditor,
write: function (aggConfig, output) {
output.params.ranges = aggConfig.params.ranges;
output.params.keyed = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/

import React from 'react';
import { EuiFieldText, EuiFlexItem } from '@elastic/eui';
import { EuiFieldText, EuiFlexItem, EuiIcon } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import Ipv4Address from '../../../utils/ipv4_address';
import { InputList, InputListConfig, InputModel, InputObject, InputItem } from './input_list';
Expand Down Expand Up @@ -95,6 +95,9 @@ function FromToList({ showValidation, onBlur, ...rest }: FromToListProps) {
onBlur={onBlur}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiIcon type="sortRight" color="subdued" />
</EuiFlexItem>
<EuiFlexItem>
<EuiFieldText
aria-label={i18n.translate('common.ui.aggTypes.ipRanges.ipRangeToAriaLabel', {
Expand Down
71 changes: 0 additions & 71 deletions src/legacy/ui/public/agg_types/controls/ranges.html

This file was deleted.

166 changes: 166 additions & 0 deletions src/legacy/ui/public/agg_types/controls/ranges.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
/*
* 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, { Fragment, useState, useEffect } from 'react';
import {
htmlIdGenerator,
EuiButtonIcon,
EuiFieldNumber,
EuiFlexItem,
EuiFlexGroup,
EuiIcon,
EuiSpacer,
EuiButtonEmpty,
} from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import { i18n } from '@kbn/i18n';
import { isEqual, omit } from 'lodash';
import { AggParamEditorProps } from 'ui/vis/editors/default';

const FROM_PLACEHOLDER = '\u2212\u221E';
const TO_PLACEHOLDER = '+\u221E';

const generateId = htmlIdGenerator();
const isEmpty = (value: any) => value === undefined || value === null;
Copy link
Contributor

Choose a reason for hiding this comment

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

As long as you're using lodash helpers, why not use the equivalent helper to this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure which lodash helper you are talking about, but if you mean isNil - it is only available from the version 4.0.0. (currently we are using 3.10.1)


interface RangeValues {
from?: number;
to?: number;
}

interface RangeValuesModel extends RangeValues {
id: string;
}

function RangesParamEditor({ agg, value = [], setValue }: AggParamEditorProps<RangeValues[]>) {
const [ranges, setRanges] = useState(() => value.map(range => ({ ...range, id: generateId() })));
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure you need to use a callback state here, couldn't you write: const [ranges, setRanges] = useState(value.map(...));

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the lazy init is used to prevent an id generation on every render.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@flash1293 you are completely right


// set up an initial range when there is no default range
useEffect(() => {
if (!value.length) {
onAddRange();
}
}, []);

useEffect(
() => {
// responsible for discarding changes
if (
value.length !== ranges.length ||
value.some((range, index) => !isEqual(range, omit(ranges[index], 'id')))
) {
setRanges(value.map(range => ({ ...range, id: generateId() })));
}
},
[value]
);

const updateRanges = (rangeValues: RangeValuesModel[]) => {
// do not set internal id parameter into saved object
setValue(rangeValues.map(range => omit(range, 'id')));
setRanges(rangeValues);
};
const onAddRange = () => updateRanges([...ranges, { id: generateId() }]);
const onRemoveRange = (id: string) => updateRanges(ranges.filter(range => range.id !== id));
const onChangeRange = (id: string, key: string, newValue: string) =>
updateRanges(
ranges.map(range =>
range.id === id
? {
...range,
[key]: newValue === '' ? undefined : parseFloat(newValue),
}
: range
)
);

return (
<>
{ranges.map(({ from, to, id }) => {
const deleteBtnTitle = i18n.translate(
'common.ui.aggTypes.ranges.removeRangeButtonAriaLabel',
{
defaultMessage: 'Remove the range of {from} to {to}',
values: {
from: isEmpty(from) ? FROM_PLACEHOLDER : from,
to: isEmpty(to) ? TO_PLACEHOLDER : to,
},
}
);

return (
<Fragment key={id}>
<EuiFlexGroup gutterSize="s" alignItems="center">
<EuiFlexItem>
<EuiFieldNumber
aria-label={i18n.translate('common.ui.aggTypes.ranges.fromLabel', {
defaultMessage: 'From',
})}
value={isEmpty(from) ? '' : from}
placeholder={FROM_PLACEHOLDER}
onChange={ev => onChangeRange(id, 'from', ev.target.value)}
fullWidth={true}
compressed={true}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiIcon type="sortRight" color="subdued" />
</EuiFlexItem>
<EuiFlexItem>
<EuiFieldNumber
aria-label={i18n.translate('common.ui.aggTypes.ranges.toLabel', {
defaultMessage: 'To',
})}
value={isEmpty(to) ? '' : to}
placeholder={TO_PLACEHOLDER}
onChange={ev => onChangeRange(id, 'to', ev.target.value)}
fullWidth={true}
compressed={true}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButtonIcon
title={deleteBtnTitle}
aria-label={deleteBtnTitle}
disabled={value.length === 1}
color="danger"
iconType="trash"
onClick={() => onRemoveRange(id)}
/>
</EuiFlexItem>
</EuiFlexGroup>
<EuiSpacer size="xs" />
</Fragment>
);
})}

<EuiSpacer size="s" />
<EuiFlexItem>
<EuiButtonEmpty iconType="plusInCircleFilled" onClick={onAddRange} size="xs">
<FormattedMessage
id="common.ui.aggTypes.ranges.addRangeButtonLabel"
defaultMessage="Add range"
/>
</EuiButtonEmpty>
</EuiFlexItem>
</>
);
}

export { RangesParamEditor };
6 changes: 0 additions & 6 deletions x-pack/plugins/translations/translations/ja-JP.json
Original file line number Diff line number Diff line change
Expand Up @@ -229,12 +229,6 @@
"common.ui.aggTypes.paramTypes.field.requiredFieldParameterErrorMessage": "{fieldParameter} は必須パラメーターです",
"common.ui.aggTypes.placeMarkersOffGridLabel": "グリッド外にマーカーを配置 (ジオセントロイドを使用)",
"common.ui.aggTypes.precisionLabel": "精度",
"common.ui.aggTypes.ranges.addRangeButtonLabel": "範囲を追加",
"common.ui.aggTypes.ranges.fromColumnLabel": "From",
"common.ui.aggTypes.ranges.removeRangeButtonAriaLabel": "この範囲を削除",
"common.ui.aggTypes.ranges.requiredRangeDescription": "範囲を最低 1 つ指定する必要があります。",
"common.ui.aggTypes.ranges.requiredRangeTitle": "必須:",
"common.ui.aggTypes.ranges.toColumnLabel": "To",
"common.ui.aggTypes.showEmptyBucketsLabel": "空のバケットを表示",
"common.ui.aggTypes.showEmptyBucketsTooltip": "結果のあるバケットだけでなくすべてのバケットを表示します",
"common.ui.aggTypes.sizeLabel": "サイズ",
Expand Down
5 changes: 0 additions & 5 deletions x-pack/plugins/translations/translations/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,6 @@
"common.ui.aggTypes.paramTypes.field.requiredFieldParameterErrorMessage": "{fieldParameter} 是必需字段",
"common.ui.aggTypes.placeMarkersOffGridLabel": "将标记置于网格外(使用 geocentroid)",
"common.ui.aggTypes.precisionLabel": "精确度",
"common.ui.aggTypes.ranges.addRangeButtonLabel": "添加范围",
"common.ui.aggTypes.ranges.fromColumnLabel": "从",
"common.ui.aggTypes.ranges.requiredRangeDescription": "必须指定至少一个范围。",
"common.ui.aggTypes.ranges.requiredRangeTitle": "必需:",
"common.ui.aggTypes.ranges.toColumnLabel": "到",
"common.ui.aggTypes.showEmptyBucketsLabel": "显示空存储桶",
"common.ui.aggTypes.showEmptyBucketsTooltip": "显示所有存储桶,不仅仅有结果的存储桶",
"common.ui.aggTypes.sizeLabel": "大小",
Expand Down