Skip to content

Controls Visualization - Dates shown in milliseconds AFTER dates selection #25654

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.

Sign up for GitHub

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

Merged
nreese merged 5 commits into from
Nov 23, 2018
Merged

Controls Visualization - Dates shown in milliseconds AFTER dates selection #25654

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
24407f0
use field format to create label
nreese Nov 14, 2018
03086f4
Merge branch 'master' of github.com:elastic/kibana into issue-25111
nreese Nov 14, 2018
ca49927
make sure labels are strings so sort does not throw exceptions
nreese Nov 14, 2018
90c2518
Merge branch 'master' of github.com:elastic/kibana into issue-25111
nreese Nov 23, 2018
1c2ffc2
move map out of jsx block
nreese Nov 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 10 additions & 24 deletions ...gins/input_control_vis/public/components/vis/__snapshots__/input_control_vis.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,14 @@ exports[`Apply and Cancel change btns enabled when there are changes 1`] = `
controlIndex={0}
disableMsg={null}
fetchOptions={[Function]}
formatOptionLabel={[Function]}
id="mock-list-control"
label="list control"
multiselect={true}
options={
Array [
Object {
"label": "choice1",
"value": "choice1",
},
Object {
"label": "choice2",
"value": "choice2",
},
"choice1",
"choice2",
]
}
selectedOptions={Array []}
Expand Down Expand Up @@ -167,19 +162,14 @@ exports[`Clear btns enabled when there are values 1`] = `
controlIndex={0}
disableMsg={null}
fetchOptions={[Function]}
formatOptionLabel={[Function]}
id="mock-list-control"
label="list control"
multiselect={true}
options={
Array [
Object {
"label": "choice1",
"value": "choice1",
},
Object {
"label": "choice2",
"value": "choice2",
},
"choice1",
"choice2",
]
}
selectedOptions={Array []}
Expand Down Expand Up @@ -306,19 +296,14 @@ exports[`Renders list control 1`] = `
controlIndex={0}
disableMsg={null}
fetchOptions={[Function]}
formatOptionLabel={[Function]}
id="mock-list-control"
label="list control"
multiselect={true}
options={
Array [
Object {
"label": "choice1",
"value": "choice1",
},
Object {
"label": "choice2",
"value": "choice2",
},
"choice1",
"choice2",
]
}
selectedOptions={Array []}
Expand Down Expand Up @@ -444,6 +429,7 @@ exports[`Renders range control 1`] = `
<InjectIntl(RangeControlUi)
control={
Object {
"format": [Function],
"id": "mock-range-control",
"isEnabled": [Function],
"label": "range control",
Expand Down
4 changes: 2 additions & 2 deletions ...e_plugins/input_control_vis/public/components/vis/__snapshots__/list_control.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ exports[`renders ListControl 1`] = `
Array [
Object {
"data-test-subj": "option_choice1",
"label": "choice1",
"label": "choice1 + formatting",
"value": "choice1",
},
Object {
"data-test-subj": "option_choice2",
"label": "choice2",
"label": "choice2 + formatting",
"value": "choice2",
},
]
Expand Down
1 change: 1 addition & 0 deletions src/core_plugins/input_control_vis/public/components/vis/input_control_vis.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export class InputControlVis extends Component {
label={control.label}
options={control.selectOptions}
selectedOptions={control.value}
formatOptionLabel={control.format}
disableMsg={control.isEnabled() ? null : control.disabledReason}
multiselect={control.options.multiselect}
dynamicOptions={control.options.dynamicOptions}
Expand Down
9 changes: 4 additions & 5 deletions src/core_plugins/input_control_vis/public/components/vis/input_control_vis.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,8 @@ const mockListControl = {
type: 'list',
label: 'list control',
value: [],
selectOptions: [
{ label: 'choice1', value: 'choice1' },
{ label: 'choice2', value: 'choice2' }
]
selectOptions: ['choice1', 'choice2'],
format: (value) => { return value; },
};
const mockRangeControl = {
id: 'mock-range-control',
Expand All @@ -53,7 +51,8 @@ const mockRangeControl = {
label: 'range control',
value: { min: 0, max: 0 },
min: 0,
max: 100
max: 100,
format: (value) => { return value; }
};
const updateFiltersOnChange = false;

Expand Down
42 changes: 24 additions & 18 deletions src/core_plugins/input_control_vis/public/components/vis/list_control.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ class ListControlUi extends Component {
}

handleOnChange = (selectedOptions) => {
this.props.stageFilter(this.props.controlIndex, selectedOptions);
const selectedValues = selectedOptions.map(({ value }) => {
return value;
});
this.props.stageFilter(this.props.controlIndex, selectedValues);
}

debouncedFetch = _.debounce(async (searchValue) => {
Expand Down Expand Up @@ -77,25 +80,32 @@ class ListControlUi extends Component {
);
}

const options = this.props.options.map(option => {
return {
label: option.label,
value: option.value,
['data-test-subj']: `option_${option.value.replace(' ', '_')}`
};
});

return (
<EuiComboBox
placeholder={intl.formatMessage({
id: 'inputControl.vis.listControl.selectPlaceholder',
defaultMessage: 'Select...'
})}
options={options}
options={this.props.options.map(option => {
return {
label: this.props.formatOptionLabel(option).toString(),
value: option,
['data-test-subj']: `option_${option.toString().replace(' ', '_')}`
};
})
.sort((a, b) => {
return a.label.toLowerCase().localeCompare(b.label.toLowerCase());
})
}

@markov00 markov00 Nov 23, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Why you have moved this map in the JSX? Don't you think it loose a bit of readability if written here?

isLoading={this.state.isLoading}
async={this.props.dynamicOptions}
onSearchChange={this.props.dynamicOptions ? this.onSearchChange : undefined}
selectedOptions={this.props.selectedOptions}
selectedOptions={this.props.selectedOptions.map(selectedOption => {
return {
label: this.props.formatOptionLabel(selectedOption).toString(),
value: selectedOption,
};
})}

@markov00 markov00 Nov 23, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The same as the comment above. You an also write an helper function like:

function formatOptions(optionList, formatter, dataTestSubjPrefix, sorted) {
 const formattedOptionList = optionList.map(value => {
    const formattedOption = {
      label: formatter(value).toString(),
      value,
     };
     if (dataTestSubjPrefix ) {
        formattedOption['data-test-subj'] = `${dataTestSubjPrefix}_${value.toString().replace(' ', '_')}`;
     }
     return formattedOption;
  });
  if (sorted) {
    return formattedOptionList.sort((a, b) => {
      return a.label.toLowerCase().localeCompare(b.label.toLowerCase());
    })
   }
   return formattedOptionList;
}


const options = formatOptions(this.props.options, this.props.formatOptionLabel, 'option', true)
const selectedOptions = formatOptions(this.props.selectedOptions, this.props.formatOptionLabel)

onChange={this.handleOnChange}
singleSelection={!this.props.multiselect}
data-test-subj={`listControlSelect${this.props.controlIndex}`}
Expand All @@ -117,16 +127,12 @@ class ListControlUi extends Component {
}
}

const comboBoxOptionShape = PropTypes.shape({
label: PropTypes.string.isRequired,
value: PropTypes.string.isRequired,
});

ListControlUi.propTypes = {
id: PropTypes.string.isRequired,
label: PropTypes.string.isRequired,
selectedOptions: PropTypes.arrayOf(comboBoxOptionShape).isRequired,
options: PropTypes.arrayOf(comboBoxOptionShape),
selectedOptions: PropTypes.array.isRequired,
options: PropTypes.array,
formatOptionLabel: PropTypes.func.isRequired,
disableMsg: PropTypes.string,
multiselect: PropTypes.bool,
dynamicOptions: PropTypes.bool,
Expand Down
11 changes: 7 additions & 4 deletions src/core_plugins/input_control_vis/public/components/vis/list_control.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@ import {
ListControl,
} from './list_control';

const options = [
{ label: 'choice1', value: 'choice1' },
{ label: 'choice2', value: 'choice2' }
];
const options = ['choice1', 'choice2'];

const formatOptionLabel = (value) => {
return `${value} + formatting`;
};

let stageFilter;

Expand All @@ -45,6 +46,7 @@ test('renders ListControl', () => {
multiselect={true}
controlIndex={0}
stageFilter={stageFilter}
formatOptionLabel={formatOptionLabel}
/>);
expect(component).toMatchSnapshot(); // eslint-disable-line
});
Expand All @@ -56,6 +58,7 @@ test('disableMsg', () => {
multiselect={true}
controlIndex={0}
stageFilter={stageFilter}
formatOptionLabel={formatOptionLabel}
disableMsg={'control is disabled to test rendering when disabled'}
/>);
expect(component).toMatchSnapshot(); // eslint-disable-line
Expand Down
2 changes: 1 addition & 1 deletion src/core_plugins/input_control_vis/public/control/control.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export class Control {
throw new Error('fetch method not defined, subclass are required to implement');
}

format(value) {
format = (value) => {
const field = this.filterManager.getField();
if (field) {
return field.format.convert(value);
Expand Down
12 changes: 3 additions & 9 deletions src/core_plugins/input_control_vis/public/control/filter_manager/phrase_filter_manager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,12 @@ export class PhraseFilterManager extends FilterManager {
/**
* Convert phrases into filter
*
* @param {array}
* @param {array} phrases
* @return {object} query filter
* single phrase: match query
* multiple phrases: bool query with should containing list of match_phrase queries
*/
createFilter(selectedOptions) {
const phrases = selectedOptions.map(phrase => {
return phrase.value;
});
createFilter(phrases) {
let newFilter;
if (phrases.length === 1) {
newFilter = buildPhraseFilter(
Expand Down Expand Up @@ -74,10 +71,7 @@ export class PhraseFilterManager extends FilterManager {
return values
.reduce((accumulator, currentValue) => {
return accumulator.concat(currentValue);
}, [])
.map(value => {
return { value, label: value };
});
}, []);
}

/**
Expand Down
10 changes: 5 additions & 5 deletions ...ore_plugins/input_control_vis/public/control/filter_manager/phrase_filter_manager.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('PhraseFilterManager', function () {
});

test('should create match phrase filter from single value', function () {
const newFilter = filterManager.createFilter([{ value: 'ios' }]);
const newFilter = filterManager.createFilter(['ios']);
expect(newFilter).to.have.property('meta');
expect(newFilter.meta.index).to.be(indexPatternId);
expect(newFilter.meta.controlledBy).to.be(controlId);
Expand All @@ -56,7 +56,7 @@ describe('PhraseFilterManager', function () {
});

test('should create bool filter from multiple values', function () {
const newFilter = filterManager.createFilter([{ value: 'ios' }, { value: 'win xp' }]);
const newFilter = filterManager.createFilter(['ios', 'win xp']);
expect(newFilter).to.have.property('meta');
expect(newFilter.meta.index).to.be(indexPatternId);
expect(newFilter.meta.controlledBy).to.be(controlId);
Expand Down Expand Up @@ -102,7 +102,7 @@ describe('PhraseFilterManager', function () {
}
}
]);
expect(filterManager.getValueFromFilterBar()).to.eql([{ value: 'ios', label: 'ios' }]);
expect(filterManager.getValueFromFilterBar()).to.eql(['ios']);
});

test('should extract value from multiple filters', function () {
Expand All @@ -128,7 +128,7 @@ describe('PhraseFilterManager', function () {
}
},
]);
expect(filterManager.getValueFromFilterBar()).to.eql([{ value: 'ios', label: 'ios' }, { value: 'win xp', label: 'win xp' }]);
expect(filterManager.getValueFromFilterBar()).to.eql(['ios', 'win xp']);
});

test('should extract value from bool filter', function () {
Expand All @@ -152,7 +152,7 @@ describe('PhraseFilterManager', function () {
}
}
]);
expect(filterManager.getValueFromFilterBar()).to.eql([{ value: 'ios', label: 'ios' }, { value: 'win xp', label: 'win xp' }]);
expect(filterManager.getValueFromFilterBar()).to.eql(['ios', 'win xp']);
});

test('should return undefined when filter value can not be extracted from Kibana filter', function () {
Expand Down
4 changes: 1 addition & 3 deletions src/core_plugins/input_control_vis/public/control/list_control_factory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,7 @@ class ListControl extends Control {
}

const selectOptions = _.get(resp, 'aggregations.termsAgg.buckets', []).map((bucket) => {
return { label: this.format(bucket.key), value: bucket.key.toString() };
}).sort((a, b) => {
return a.label.toLowerCase().localeCompare(b.label.toLowerCase());
return bucket.key;
});

if(selectOptions.length === 0 && !query) {
Expand Down
11 changes: 1 addition & 10 deletions src/core_plugins/input_control_vis/public/control/list_control_factory.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,16 +119,7 @@ describe('fetch', () => {

test('should set selectOptions to results of terms aggregation', async () => {
await listControl.fetch();
expect(listControl.selectOptions).toEqual([
{
'label': 'Xi an Xianyang International Airport',
'value': 'Xi an Xianyang International Airport',
},
{
'label': 'Zurich Airport',
'value': 'Zurich Airport',
}
]);
expect(listControl.selectOptions).toEqual(['Zurich Airport', 'Xi an Xianyang International Airport']);
});
});

Expand Down