Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
flash1293 committed May 27, 2021
1 parent 026371f commit 6891e8b
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { EuiComboBox } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import { uniq } from 'lodash';
import { i18n } from '@kbn/i18n';
import React, { useEffect, useState } from 'react';
import React, { useEffect, useRef, useState } from 'react';
import { Query } from 'src/plugins/data/public';
import { search } from '../../../../../../src/plugins/data/public';
import { parseTimeShift } from '../../../../../../src/plugins/data/common';
Expand Down Expand Up @@ -146,6 +146,7 @@ export function TimeShift({
activeData: IndexPatternDimensionEditorProps['activeData'];
layerId: string;
}) {
const focusSetRef = useRef(false);
const [localValue, setLocalValue] = useState(selectedColumn.timeShift);
useEffect(() => {
setLocalValue(selectedColumn.timeShift);
Expand Down Expand Up @@ -211,7 +212,8 @@ export function TimeShift({
ref={(r) => {
if (r && isFocused) {
const timeShiftInput = r.querySelector('[data-test-subj="comboBoxSearchInput"]');
if (timeShiftInput instanceof HTMLInputElement) {
if (!focusSetRef.current && timeShiftInput instanceof HTMLInputElement) {
focusSetRef.current = true;
timeShiftInput.focus();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function getDisallowedTermsMessage(
return {
message: i18n.translate('xpack.lens.indexPattern.termsWithMultipleShifts', {
defaultMessage:
'In a single layer, you are unable to combine multiple time shifts and dynamic top values. Use the same value for all time shifts, or use filters instead of top values.',
'In a single layer, you are unable to combine metrics with different time shifts and dynamic top values. Use the same time shift value for all metrics, or use filters instead of top values.',
}),
fixAction: {
label: i18n.translate('xpack.lens.indexPattern.termsWithMultipleShiftsFixActionLabel', {
Expand All @@ -84,12 +84,15 @@ function getDisallowedTermsMessage(
newState: async (core: CoreStart, frame: FramePublicAPI, layerId: string) => {
const currentColumn = layer.columns[columnId] as TermsIndexPatternColumn;
const fieldName = currentColumn.sourceField;
const activeDataFieldNameMatch =
frame.activeData?.[layerId].columns.find(({ id }) => id === columnId)?.meta.field ===
fieldName;
let currentTerms = uniq(
frame.activeData?.[layerId].rows
.map((row) => row[columnId] as string)
.filter((term) => typeof term === 'string' && term !== '__other__') || []
);
if (currentTerms.length === 0) {
if (!activeDataFieldNameMatch || currentTerms.length === 0) {
const response: FieldStatsResponse<string | number> = await core.http.post(
`/api/lens/index_stats/${indexPattern.id}/field`,
{
Expand All @@ -115,7 +118,7 @@ function getDisallowedTermsMessage(
...layer.columns,
[columnId]: {
label: i18n.translate('xpack.lens.indexPattern.pinnedTopValuesLabel', {
defaultMessage: 'Pinned top values of {field}',
defaultMessage: 'Filters of {field}',
values: {
field: fieldName,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1037,7 +1037,7 @@ describe('terms', () => {
expect(termsOperation.getErrorMessage!(layer, 'col1', indexPattern)).toEqual([
expect.objectContaining({
message:
'In a single layer, you are unable to combine multiple time shifts and dynamic top values. Use the same value for all time shifts, or use filters instead of top values.',
'In a single layer, you are unable to combine metrics with different time shifts and dynamic top values. Use the same time shift value for all metrics, or use filters instead of top values.',
}),
]);
});
Expand Down

0 comments on commit 6891e8b

Please sign in to comment.