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

fix(queryHook): restore behaviour of queryHook #4202

Merged
merged 8 commits into from
Nov 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
"eslint-plugin-react": "7.16.0",
"eslint-plugin-react-hooks": "2.3.0",
"inquirer": "6.5.2",
"instantsearch-e2e-tests": "algolia/instantsearch-e2e-tests#v1.2.3",
"instantsearch-e2e-tests": "algolia/instantsearch-e2e-tests#1.2.4",
"jest": "24.9.0",
"jest-diff": "24.9.0",
"jest-environment-jsdom": "24.9.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,8 @@ Array [
className="input"
disabled={false}
maxLength={512}
onBlur={[Function]}
onFocus={[Function]}
onInput={[Function]}
placeholder="Search"
spellCheck={false}
Expand Down Expand Up @@ -568,6 +570,8 @@ Array [
className="input"
disabled={false}
maxLength={512}
onBlur={[Function]}
onFocus={[Function]}
onInput={[Function]}
placeholder="Search"
spellCheck={false}
Expand Down
45 changes: 32 additions & 13 deletions src/components/SearchBox/SearchBox.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ class SearchBox extends Component {
};

state = {
query: this.props.searchAsYouType ? '' : this.props.query,
query: this.props.query,
focused: false,
};

/**
Expand All @@ -72,13 +73,23 @@ class SearchBox extends Component {

if (searchAsYouType) {
refine(query);
} else {
this.setState({ query });
}
this.setState({ query });

onChange(event);
};

componentWillReceiveProps(nextProps) {
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
/**
* when the user is typing, we don't want to replace the query typed
* by the user (state.query) with the query exposed by the connector (props.query)
* see: https://github.com/algolia/instantsearch.js/issues/4141
*/
if (!this.state.focused && nextProps.query !== this.state.query) {
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
tkrugg marked this conversation as resolved.
Show resolved Hide resolved
this.setState({ query: nextProps.query });
}
}

onSubmit = event => {
const { searchAsYouType, refine, onSubmit } = this.props;

Expand All @@ -96,20 +107,25 @@ class SearchBox extends Component {
};

onReset = event => {
const { searchAsYouType, refine, onReset } = this.props;
const { refine, onReset } = this.props;
const query = '';

this.input.focus();

refine(query);

if (!searchAsYouType) {
this.setState({ query });
}
this.setState({ query });

onReset(event);
};

onBlur = () => {
this.setState({ focused: false });
};

onFocus = () => {
this.setState({ focused: true });
};

render() {
const {
cssClasses,
Expand All @@ -120,11 +136,8 @@ class SearchBox extends Component {
showLoadingIndicator,
templates,
isSearchStalled,
searchAsYouType,
} = this.props;

const query = searchAsYouType ? this.props.query : this.state.query;

return (
<div className={cssClasses.root}>
<form
Expand All @@ -137,7 +150,7 @@ class SearchBox extends Component {
>
<input
ref={inputRef => (this.input = inputRef)}
value={query}
value={this.state.query}
disabled={this.props.disabled}
className={cssClasses.input}
type="search"
Expand All @@ -149,6 +162,8 @@ class SearchBox extends Component {
spellCheck={false}
maxLength={512}
onInput={this.onInput}
onBlur={this.onBlur}
onFocus={this.onFocus}
/>

<Template
Expand All @@ -171,7 +186,11 @@ class SearchBox extends Component {
className: cssClasses.reset,
type: 'reset',
title: 'Clear the search query.',
hidden: !(showReset && query.trim() && !isSearchStalled),
hidden: !(
showReset &&
this.state.query.trim() &&
!isSearchStalled
),
}}
templates={templates}
data={{ cssClasses }}
Expand Down
181 changes: 137 additions & 44 deletions src/components/SearchBox/__tests__/SearchBox-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,40 @@ describe('SearchBox', () => {
});

describe('Events', () => {
describe('focus/blur', () => {
tkrugg marked this conversation as resolved.
Show resolved Hide resolved
test('does not derive value from prop when focused', () => {
// This makes sure we don't override the user's input while they're typing.
// This issue is more obvious when using queryHook to add debouncing.

const props = {
...defaultProps,
query: 'Initial query',
};
const { container, rerender } = render(<SearchBox {...props} />);
const input = container.querySelector('input');
expect(input.value).toEqual('Initial query');

fireEvent.focus(input);
rerender(<SearchBox {...props} query={'Query updated through prop'} />);

expect(input.value).toEqual('Initial query');
});

test('derives value from prop when not focused', () => {
const props = {
...defaultProps,
query: 'Initial query',
};
const { container, rerender } = render(<SearchBox {...props} />);
const input = container.querySelector('input');
expect(input.value).toEqual('Initial query');

fireEvent.blur(input);
rerender(<SearchBox {...props} query={'Query updated through prop'} />);

expect(input.value).toEqual('Query updated through prop');
});
});
describe('searchAsYouType to true', () => {
test('refines input value on input', () => {
const refine = jest.fn();
Expand Down Expand Up @@ -281,64 +315,123 @@ describe('SearchBox', () => {
});
});

// @TODO This test suite will pass once we upgrade Jest to 25.x.
// See https://github.com/algolia/instantsearch.js/issues/4160
// eslint-disable-next-line jest/no-disabled-tests
describe.skip('onReset', () => {
test('resets the input value with searchAsYouType to true', () => {
const props = {
...defaultProps,
searchAsYouType: true,
};
const { container } = render(<SearchBox {...props} />);
const input = container.querySelector('input');
const resetButton = container.querySelector('button[type="reset"]');
describe('onReset', () => {
// @TODO This test suite will pass once we upgrade Jest to 25.x.
// See https://github.com/algolia/instantsearch.js/issues/4160
// eslint-disable-next-line jest/no-disabled-tests
describe.skip('with button click', () => {
test('resets the input value with searchAsYouType to true', () => {
const props = {
...defaultProps,
searchAsYouType: true,
};
const { container } = render(<SearchBox {...props} />);
const input = container.querySelector('input');
const resetButton = container.querySelector('button[type="reset"]');

fireEvent.change(input, {
target: { value: 'hello' },
});

expect(input.value).toEqual('hello');

fireEvent.click(resetButton);

expect(input.value).toEqual('');
expect(input).toHaveFocus();
});

fireEvent.change(input, {
target: { value: 'hello' },
test('resets the input value with searchAsYouType to false', () => {
const props = {
...defaultProps,
searchAsYouType: false,
};
const { container } = render(<SearchBox {...props} />);
const form = container.querySelector('form');
const input = container.querySelector('input');
const resetButton = container.querySelector('button[type="reset"]');

fireEvent.input(input, { target: { value: 'hello' } });
fireEvent.submit(form);

expect(input.value).toEqual('hello');

fireEvent.click(resetButton);

expect(input.value).toEqual('');
expect(input).toHaveFocus();
});

expect(input.value).toEqual('hello');
test('calls custom onReset', () => {
const onReset = jest.fn();
const props = {
...defaultProps,
onReset,
};
const { container } = render(<SearchBox {...props} />);
const resetButton = container.querySelector('button[type="reset"]');

fireEvent.click(resetButton);
fireEvent.click(resetButton);

expect(input.value).toEqual('');
expect(input).toHaveFocus();
expect(onReset).toHaveBeenCalledTimes(1);
});
});

test('resets the input value with searchAsYouType to false', () => {
const props = {
...defaultProps,
searchAsYouType: false,
};
const { container } = render(<SearchBox {...props} />);
const form = container.querySelector('form');
const input = container.querySelector('input');
const resetButton = container.querySelector('button[type="reset"]');
describe('when form is reset programmatically', () => {
test('resets the input value with searchAsYouType to true', () => {
const props = {
...defaultProps,
searchAsYouType: true,
};
const { container } = render(<SearchBox {...props} />);
const input = container.querySelector('input');
const form = container.querySelector('form');

fireEvent.input(input, { target: { value: 'hello' } });
fireEvent.submit(form);
fireEvent.change(input, {
target: { value: 'hello' },
});

expect(input.value).toEqual('hello');
expect(input.value).toEqual('hello');

fireEvent.click(resetButton);
fireEvent.reset(form);

expect(input.value).toEqual('');
expect(input).toHaveFocus();
});
expect(input.value).toEqual('');
expect(input).toHaveFocus();
});

test('calls custom onReset', () => {
const onReset = jest.fn();
const props = {
...defaultProps,
onReset,
};
const { container } = render(<SearchBox {...props} />);
const resetButton = container.querySelector('button[type="reset"]');
test('resets the input value with searchAsYouType to false', () => {
const props = {
...defaultProps,
searchAsYouType: false,
};
const { container } = render(<SearchBox {...props} />);
const form = container.querySelector('form');
const input = container.querySelector('input');

fireEvent.input(input, { target: { value: 'hello' } });
fireEvent.submit(form);

fireEvent.click(resetButton);
expect(input.value).toEqual('hello');

expect(onReset).toHaveBeenCalledTimes(1);
fireEvent.reset(form);

expect(input.value).toEqual('');
expect(input).toHaveFocus();
});

test('calls custom onReset', () => {
const onReset = jest.fn();
const props = {
...defaultProps,
onReset,
};
const { container } = render(<SearchBox {...props} />);
const form = container.querySelector('form');

fireEvent.reset(form);

expect(onReset).toHaveBeenCalledTimes(1);
});
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ Array [
className="input"
disabled={false}
maxLength={512}
onBlur={[Function]}
onFocus={[Function]}
onInput={[Function]}
placeholder=""
spellCheck={false}
Expand Down Expand Up @@ -90,6 +92,8 @@ Array [
className="input"
disabled={false}
maxLength={512}
onBlur={[Function]}
onFocus={[Function]}
onInput={[Function]}
placeholder=""
spellCheck={false}
Expand Down
17 changes: 17 additions & 0 deletions stories/search-box.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,21 @@ storiesOf('Basics|SearchBox', module)
}),
]);
})
)
.add(
'with debounced queryHook',
withHits(({ search, container, instantsearch }) => {
let timerId;
search.addWidgets([
instantsearch.widgets.searchBox({
container,
queryHook(query, refine) {
clearTimeout(timerId);
timerId = setTimeout(() => {
refine(query);
}, 100);
},
}),
]);
})
);
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7606,9 +7606,9 @@ insert-css@^2.0.0:
resolved "https://registry.yarnpkg.com/insert-css/-/insert-css-2.0.0.tgz#eb5d1097b7542f4c79ea3060d3aee07d053880f4"
integrity sha1-610Ql7dUL0x56jBg067gfQU4gPQ=

instantsearch-e2e-tests@algolia/instantsearch-e2e-tests#v1.2.3:
version "1.2.3"
resolved "https://codeload.github.com/algolia/instantsearch-e2e-tests/tar.gz/856d5d788ff25cfeeab43d3aa7dbd10ab98b0182"
instantsearch-e2e-tests@algolia/instantsearch-e2e-tests#1.2.4:
version "1.2.4"
resolved "https://codeload.github.com/algolia/instantsearch-e2e-tests/tar.gz/da70c1c4c0d1d0f3b6db2bbce1491e3103cf44fd"
dependencies:
dotenv "^8.0.0"
ts-node "^8.3.0"
Expand Down