From b509952e2990835c060a7a442848d5445d2f135f Mon Sep 17 00:00:00 2001 From: Chrispine Lwekaza Date: Tue, 10 Aug 2021 09:24:57 -0400 Subject: [PATCH 1/4] incorporate review feedback --- .../src/components/query-bar/query-bar.jsx | 4 ++- .../components/query-bar/query-bar.spec.js | 36 +++++++++++++++++++ packages/compass-query-bar/src/plugin.spec.js | 36 +++++++++++++++++++ 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/packages/compass-query-bar/src/components/query-bar/query-bar.jsx b/packages/compass-query-bar/src/components/query-bar/query-bar.jsx index 5b3760793a0..7bdfa1a4b6f 100644 --- a/packages/compass-query-bar/src/components/query-bar/query-bar.jsx +++ b/packages/compass-query-bar/src/components/query-bar/query-bar.jsx @@ -109,6 +109,7 @@ class QueryBar extends Component { schemaFields: PropTypes.array, showQueryHistoryButton: PropTypes.bool, showExportToLanguageButton: PropTypes.bool, + sortOptionPlaceholder: PropTypes.string, }; static defaultProps = { @@ -206,7 +207,7 @@ class QueryBar extends Component { * @return {Component} the option component */ renderOption(option, id, hasToggle) { - const { filterValid, featureFlag, autoPopulated } = this.props; + const { filterValid, featureFlag, autoPopulated, sortOptionPlaceholder } = this.props; // for filter only, also validate feature flag directives const hasError = option === 'filter' @@ -219,6 +220,7 @@ class QueryBar extends Component { this.props[option] : this.props[`${option}String`]; const label = OPTION_DEFINITION[option].label || option; + const placeholder = option === 'sort' && sortOptionPlaceholder ? sortOptionPlaceholder : OPTION_DEFINITION[option].placeholder; return ( + ); + + expect(component.find('div[data-test-id="query-bar-options-toggle"]')).to.exist; + component.find('div[data-test-id="query-bar-options-toggle"]').simulate('click'); + expect(component.find('OptionEditor[label="sort"][placeholder="{ field: -1 } or [[\'field\', -1]]"]')).to.exist; + }); + + it('the query bar renders the specified sort option placeholder', function() { + const component = mount( + + ); + + expect(component.find('div[data-test-id="query-bar-options-toggle"]')).to.exist; + component.find('div[data-test-id="query-bar-options-toggle"]').simulate('click'); + expect(component.find('OptionEditor[label="sort"][placeholder="{ field: -1 } or [[\'field\', -1]]"]')).to.not.exist; + expect(component.find('OptionEditor[label="sort"][placeholder="{ field: -1 }"]')).to.exist; + }); + }); }); }); diff --git a/packages/compass-query-bar/src/plugin.spec.js b/packages/compass-query-bar/src/plugin.spec.js index f8eafbd4513..392dbe95db2 100644 --- a/packages/compass-query-bar/src/plugin.spec.js +++ b/packages/compass-query-bar/src/plugin.spec.js @@ -172,4 +172,40 @@ describe('QueryBar [Plugin]', () => { expect(component.find('#query-bar-menu-actions')).to.not.exist; }); }); + + describe('the correct sort placeholder is rendered when the component receives a custom sort placeholder', function() { + const layout = ['filter', 'project', 'sort']; + + it('square bracket sorts placeholder is rendered by default', function() { + component = mount( + + ); + + expect(component.find('div[data-test-id="query-bar-options-toggle"]')).to.exist; + component.find('div[data-test-id="query-bar-options-toggle"]').simulate('click'); + expect(component.find('OptionEditor[label="sort"][placeholder="{ field: -1 } or [[\'field\', -1]]"]')).to.exist; + }); + + it('the query bar renders the specified sort option placeholder', function() { + component = mount( + + ); + + expect(component.find('div[data-test-id="query-bar-options-toggle"]')).to.exist; + component.find('div[data-test-id="query-bar-options-toggle"]').simulate('click'); + expect(component.find('OptionEditor[label="sort"][placeholder="{ field: -1 } or [[\'field\', -1]]"]')).to.not.exist; + expect(component.find('OptionEditor[label="sort"][placeholder="{ field: -1 }"]')).to.exist; + }); + }); }); From ab03c83cf20182e459ffe68ce88f47a9f443fd9b Mon Sep 17 00:00:00 2001 From: Chrispine Lwekaza Date: Tue, 10 Aug 2021 11:19:52 -0400 Subject: [PATCH 2/4] Reference variables correctly --- .../compass-query-bar/src/components/query-bar/query-bar.jsx | 2 +- .../src/components/query-bar/query-bar.spec.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/compass-query-bar/src/components/query-bar/query-bar.jsx b/packages/compass-query-bar/src/components/query-bar/query-bar.jsx index 7bdfa1a4b6f..7519accc7e4 100644 --- a/packages/compass-query-bar/src/components/query-bar/query-bar.jsx +++ b/packages/compass-query-bar/src/components/query-bar/query-bar.jsx @@ -232,7 +232,7 @@ class QueryBar extends Component { key={`query-option-${id}`} value={value} actions={this.props.actions} - placeholder={OPTION_DEFINITION[option].placeholder} + placeholder={placeholder} link={OPTION_DEFINITION[option].link} inputType={OPTION_DEFINITION[option].type} onChange={this.onChange.bind(this, option)} diff --git a/packages/compass-query-bar/src/components/query-bar/query-bar.spec.js b/packages/compass-query-bar/src/components/query-bar/query-bar.spec.js index 8719fa33964..337c6e39f5e 100644 --- a/packages/compass-query-bar/src/components/query-bar/query-bar.spec.js +++ b/packages/compass-query-bar/src/components/query-bar/query-bar.spec.js @@ -1,5 +1,5 @@ import React from 'react'; -import { shallow } from 'enzyme'; +import { shallow, mount } from 'enzyme'; import QueryBar from '../query-bar'; import QueryOption from '../query-option'; From 2ca86d36b1649c5b3a28bf6e3026bda998062b66 Mon Sep 17 00:00:00 2001 From: Chrispine Lwekaza Date: Tue, 10 Aug 2021 15:13:23 -0400 Subject: [PATCH 3/4] improve readability of the two tests and trigger a github test action --- .../src/components/query-bar/query-bar.spec.js | 2 +- packages/compass-query-bar/src/plugin.spec.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/compass-query-bar/src/components/query-bar/query-bar.spec.js b/packages/compass-query-bar/src/components/query-bar/query-bar.spec.js index 337c6e39f5e..af31fcb125e 100644 --- a/packages/compass-query-bar/src/components/query-bar/query-bar.spec.js +++ b/packages/compass-query-bar/src/components/query-bar/query-bar.spec.js @@ -347,7 +347,7 @@ describe('QueryBar [Component]', function() { expect(component.find('OptionEditor[label="sort"][placeholder="{ field: -1 } or [[\'field\', -1]]"]')).to.exist; }); - it('the query bar renders the specified sort option placeholder', function() { + it('the query bar renders the specified sort option placeholder that is passed as a prop', function() { const component = mount( { expect(component.find('OptionEditor[label="sort"][placeholder="{ field: -1 } or [[\'field\', -1]]"]')).to.exist; }); - it('the query bar renders the specified sort option placeholder', function() { + it('the query bar renders the specified sort option placeholder that is passed as a prop', function() { component = mount( Date: Wed, 11 Aug 2021 11:31:58 -0400 Subject: [PATCH 4/4] Incorporate review feedback by making custom placeholders more streamlined --- .../src/components/query-bar/query-bar.jsx | 14 ++++++-- .../components/query-bar/query-bar.spec.js | 35 ++++++++++++++----- packages/compass-query-bar/src/plugin.spec.js | 35 ++++++++++++++----- 3 files changed, 65 insertions(+), 19 deletions(-) diff --git a/packages/compass-query-bar/src/components/query-bar/query-bar.jsx b/packages/compass-query-bar/src/components/query-bar/query-bar.jsx index 7519accc7e4..5d16745e7b8 100644 --- a/packages/compass-query-bar/src/components/query-bar/query-bar.jsx +++ b/packages/compass-query-bar/src/components/query-bar/query-bar.jsx @@ -97,6 +97,14 @@ class QueryBar extends Component { skipString: PropTypes.string, limitString: PropTypes.string, + filterPlaceholder: PropTypes.string, + projectPlaceholder: PropTypes.string, + collationPlaceholder: PropTypes.string, + sortPlaceholder: PropTypes.string, + skipPlaceholder: PropTypes.string, + limitPlaceholder: PropTypes.string, + maxTimeMSPlaceholder: PropTypes.string, + actions: PropTypes.object, buttonLabel: PropTypes.string, queryState: PropTypes.string, @@ -109,7 +117,6 @@ class QueryBar extends Component { schemaFields: PropTypes.array, showQueryHistoryButton: PropTypes.bool, showExportToLanguageButton: PropTypes.bool, - sortOptionPlaceholder: PropTypes.string, }; static defaultProps = { @@ -207,7 +214,7 @@ class QueryBar extends Component { * @return {Component} the option component */ renderOption(option, id, hasToggle) { - const { filterValid, featureFlag, autoPopulated, sortOptionPlaceholder } = this.props; + const { filterValid, featureFlag, autoPopulated } = this.props; // for filter only, also validate feature flag directives const hasError = option === 'filter' @@ -220,7 +227,8 @@ class QueryBar extends Component { this.props[option] : this.props[`${option}String`]; const label = OPTION_DEFINITION[option].label || option; - const placeholder = option === 'sort' && sortOptionPlaceholder ? sortOptionPlaceholder : OPTION_DEFINITION[option].placeholder; + const placeholder = this.props[`${option}Placeholder`] || OPTION_DEFINITION[option].placeholder; + return ( + serverVersion="3.4.0" + filterPlaceholder="{field: 'matchValue'}" + projectPlaceholder="{field: 1}" + collationPlaceholder="{locale: 'fr' }" + sortPlaceholder="{field: 1}" + skipPlaceholder="10" + limitPlaceholder="20" + maxTimeMSPlaceholder="50000" /> ); expect(component.find('div[data-test-id="query-bar-options-toggle"]')).to.exist; component.find('div[data-test-id="query-bar-options-toggle"]').simulate('click'); - expect(component.find('OptionEditor[label="sort"][placeholder="{ field: -1 } or [[\'field\', -1]]"]')).to.not.exist; - expect(component.find('OptionEditor[label="sort"][placeholder="{ field: -1 }"]')).to.exist; + expect(component.find('OptionEditor[label="filter"]').prop('placeholder')).to.equal("{field: 'matchValue'}"); + expect(component.find('OptionEditor[label="project"]').prop('placeholder')).to.equal('{field: 1}'); + expect(component.find('OptionEditor[label="collation"]').prop('placeholder')).to.equal("{locale: 'fr' }"); + expect(component.find('OptionEditor[label="sort"]').prop('placeholder')).to.equal('{field: 1}'); + expect(component.find('QueryOption[label="Max Time MS"]').prop('placeholder')).to.equal('50000'); + expect(component.find('QueryOption[label="skip"]').prop('placeholder')).to.equal('10'); + expect(component.find('QueryOption[label="limit"]').prop('placeholder')).to.equal('20'); }); }); }); diff --git a/packages/compass-query-bar/src/plugin.spec.js b/packages/compass-query-bar/src/plugin.spec.js index 24acf110844..06e8a0cfef1 100644 --- a/packages/compass-query-bar/src/plugin.spec.js +++ b/packages/compass-query-bar/src/plugin.spec.js @@ -173,10 +173,11 @@ describe('QueryBar [Plugin]', () => { }); }); - describe('the correct sort placeholder is rendered when the component receives a custom sort placeholder', function() { - const layout = ['filter', 'project', 'sort']; + describe('a user is able to provide custom placeholders for the input fields', function() { + const layout = ['filter', 'project', ['sort', 'maxTimeMS'], ['collation', 'skip', 'limit']]; - it('square bracket sorts placeholder is rendered by default', function() { + + it('the input fields have a placeholder by default', function() { component = mount( { expect(component.find('div[data-test-id="query-bar-options-toggle"]')).to.exist; component.find('div[data-test-id="query-bar-options-toggle"]').simulate('click'); - expect(component.find('OptionEditor[label="sort"][placeholder="{ field: -1 } or [[\'field\', -1]]"]')).to.exist; + expect(component.find('OptionEditor[label="filter"]').prop('placeholder')).to.not.be.empty; + expect(component.find('OptionEditor[label="project"]').prop('placeholder')).to.not.be.empty; + expect(component.find('OptionEditor[label="collation"]').prop('placeholder')).to.not.be.empty; + expect(component.find('OptionEditor[label="sort"]').prop('placeholder')).to.not.be.empty; + expect(component.find('QueryOption[label="Max Time MS"]').prop('placeholder')).to.not.be.empty; + expect(component.find('QueryOption[label="skip"]').prop('placeholder')).to.not.be.empty; + expect(component.find('QueryOption[label="limit"]').prop('placeholder')).to.not.be.empty; }); - it('the query bar renders the specified sort option placeholder that is passed as a prop', function() { + it('the input fields placeholders can be modified', function() { component = mount( { layout={layout} sortOptionPlaceholder="{ field: -1 }" expanded - serverVersion="3.4.0" /> + serverVersion="3.4.0" + filterPlaceholder="{field: 'matchValue'}" + projectPlaceholder="{field: 1}" + collationPlaceholder="{locale: 'fr' }" + sortPlaceholder="{field: 1}" + skipPlaceholder="10" + limitPlaceholder="20" + maxTimeMSPlaceholder="50000" /> ); expect(component.find('div[data-test-id="query-bar-options-toggle"]')).to.exist; component.find('div[data-test-id="query-bar-options-toggle"]').simulate('click'); - expect(component.find('OptionEditor[label="sort"][placeholder="{ field: -1 } or [[\'field\', -1]]"]')).to.not.exist; - expect(component.find('OptionEditor[label="sort"][placeholder="{ field: -1 }"]')).to.exist; + expect(component.find('OptionEditor[label="filter"]').prop('placeholder')).to.equal("{field: 'matchValue'}"); + expect(component.find('OptionEditor[label="project"]').prop('placeholder')).to.equal('{field: 1}'); + expect(component.find('OptionEditor[label="collation"]').prop('placeholder')).to.equal("{locale: 'fr' }"); + expect(component.find('OptionEditor[label="sort"]').prop('placeholder')).to.equal('{field: 1}'); + expect(component.find('QueryOption[label="Max Time MS"]').prop('placeholder')).to.equal('50000'); + expect(component.find('QueryOption[label="skip"]').prop('placeholder')).to.equal('10'); + expect(component.find('QueryOption[label="limit"]').prop('placeholder')).to.equal('20'); }); }); });